Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NativeAOT-LLVM] Add WasmExternalDwarf property to create an external_debug_info section #2786

Open
wants to merge 6 commits into
base: feature/NativeAOT-LLVM
Choose a base branch
from
Open

[NativeAOT-LLVM] Add WasmExternalDwarf property to create an external_debug_info section #2786

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
804111f
add WasmExternalDwarf to create an external_debug_info section and pl…
yowl Nov 18, 2024
dd84ce5
Use StripSymbols which is true by default
yowl Nov 20, 2024
e8fd31f
move gseparate-dwarf to browser/emcc ItemGroup
yowl Nov 20, 2024
27e99ef
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Nativ…
yowl Nov 21, 2024
b12b969
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Nativ…
yowl Nov 21, 2024
5cffb73
feedback, drop DotNetJsApi condition
yowl Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@

<Delete Files="$(PublishDir)\$(TargetName).wasm" Condition="'$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'"/>
<Copy SourceFiles="$(NativeOutputPath)$(TargetName).wasm" DestinationFolder="$(PublishDir)" Condition="'$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'"/>


<Delete Files="$(PublishDir)\$(TargetName).wasm.debug.wasm" Condition="'$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'"/>
<Copy SourceFiles="$(NativeOutputPath)$(TargetName).wasm.debug.wasm" DestinationFolder="$(PublishDir)" Condition="'$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'"/>

<ItemGroup>
<_FilesToCopyToNative Include="$(NativeOutputPath)\dotnet.native.js" />
<_FilesToCopyToNative Include="$(NativeOutputPath)\dotnet.native.wasm" />
Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<IlcTreatWarningsAsErrors Condition="'$(IlcTreatWarningsAsErrors)' == ''">$(TreatWarningsAsErrors)</IlcTreatWarningsAsErrors>
<_IsiOSLikePlatform Condition="'$(_targetOS)' == 'maccatalyst' or $(_targetOS.StartsWith('ios')) or $(_targetOS.StartsWith('tvos'))">true</_IsiOSLikePlatform>
<_IsApplePlatform Condition="'$(_targetOS)' == 'osx' or '$(_IsiOSLikePlatform)' == 'true'">true</_IsApplePlatform>
<WasmExternalDwarf Condition="'$(WasmExternalDwarf)' == ''and '$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'">true</WasmExternalDwarf>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a documented property of this in the NAOT toolchain: StripSymbols. We can use it.

Also, why disable for DotNetJsApi? I think it makes sense for DotNetJsApi as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, I didn't really know, and just copied what was in the publish file for the .wasm file. Will remove it.

</PropertyGroup>

<!-- Set up default feature switches -->
Expand Down Expand Up @@ -100,6 +101,7 @@ The .NET Foundation licenses this file to you under the MIT license.

<NativeObject>$(NativeIntermediateOutputPath)$(TargetName)$(NativeObjectExt)</NativeObject>
<NativeBinary Condition="'$(NativeBinary)' == ''">$(NativeOutputPath)$(TargetName)$(NativeBinaryExt)</NativeBinary>
<WasmBinary Condition="'$(_targetArchitecture)' == 'wasm'">$(NativeOutputPath)$(TargetName).wasm</WasmBinary>
yowl marked this conversation as resolved.
Show resolved Hide resolved
<IlcExportUnmanagedEntrypoints Condition="'$(IlcExportUnmanagedEntrypoints)' == '' and '$(NativeLib)' == 'Shared'">true</IlcExportUnmanagedEntrypoints>
<ExportsFile Condition="$(ExportsFile) == '' and '$(BuildingFrameworkLibrary)' != 'true'">$(NativeIntermediateOutputPath)$(TargetName)$(ExportsFileExt)</ExportsFile>

Expand Down Expand Up @@ -390,9 +392,11 @@ The .NET Foundation licenses this file to you under the MIT license.
<ItemGroup>
<LlvmObjects Include="@(_IlcProducedFiles)" Condition="'%(Extension)' == '$(LlvmObjectExt)'">
<NativeObject>%(RelativeDir)%(Filename)$(NativeObjectExt)</NativeObject>
<DwarfObject>&quot;%(RelativeDir)%(Filename).dwo&quot;</DwarfObject>
</LlvmObjects>
<NativeObjects Include="@(_IlcProducedFiles)" Condition="'%(Extension)' == '$(NativeObjectExt)'" />
<NativeObjects Include="@(LlvmObjects->'%(NativeObject)')" />
<DwarfObjects Include="@(LlvmObjects->'%(DwarfObject)')" />
yowl marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
</Target>

Expand Down Expand Up @@ -439,12 +443,14 @@ The .NET Foundation licenses this file to you under the MIT license.
<WasmCompilerPath Condition="'$(EMSDK)' == ''">&quot;$(EmscriptenSdkToolsPath)bin/clang++&quot;</WasmCompilerPath>
<WasmLinkerPath Condition="'$(EMSDK)' != ''">&quot;$(EMSDK)/upstream/emscripten/emcc$(EmccScriptExt)&quot;</WasmLinkerPath>
<WasmLinkerPath Condition="'$(EMSDK)' == ''">&quot;$(EmscriptenSdkToolsPath)emscripten/emcc$(EmccScriptExt)&quot;</WasmLinkerPath>
<WasmDwarfLinkerPath Condition="'$(EMSDK)' != ''">&quot;$(EMSDK)/upstream/bin/llvm-dwp$(ExeExt)&quot;</WasmDwarfLinkerPath>
<WasmDwarfLinkerPath Condition="'$(EMSDK)' == ''">&quot;$(EmscriptenSdkToolsPath)/bin/llvm-dwp$(ExeExt)&quot;</WasmDwarfLinkerPath>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like leftovers from DWP (here and above with DwarfObjects).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

</PropertyGroup>

<PropertyGroup Condition="'$(_targetOS)' == 'wasi'">
<WasmCompilerPath>&quot;$(WASI_SDK_PATH)/bin/clang++&quot;</WasmCompilerPath>
<WasmLinkerPath>&quot;$(WASI_SDK_PATH)/bin/clang&quot;</WasmLinkerPath>
</PropertyGroup>
</PropertyGroup>

<!-- Invoke the compilers in parallel. -->
<ItemGroup>
Expand Down Expand Up @@ -575,6 +581,8 @@ The .NET Foundation licenses this file to you under the MIT license.
<!-- see https://github.com/emscripten-core/emscripten/issues/16836 for the issue that means we have to force the linker to include these symbols before LTO -->
<!--<CustomLinkerArg Condition="'$(Optimize)' != 'true'" Include="$(WasmOptimizationSetting) -flto" /> -->
<CustomLinkerArg Condition="'$(IlcLlvmTarget)' != ''" Include="-target $(IlcLlvmTarget)" />

<CustomLinkerArg Condition="'$(WasmExternalDwarf)' == 'true'" Include="-gseparate-dwarf" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<CustomLinkerArg Condition="'$(WasmExternalDwarf)' == 'true'" Include="-gseparate-dwarf" />
<CustomLinkerArg Condition="'$(WasmExternalDwarf)' == 'true'" Include="-gseparate-dwarf" />

I think we should pass something like GetFilenameWithoutExtension($(NativeBinary)).debug.wasm here to avoid the odd .wasm.debug.wasm thing.

</ItemGroup>

<ItemGroup Condition="'$(_targetOS)' == 'browser'" >
Expand Down