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

Jump to bottom

NativeAOT: Disable AggressiveAttributeTrimming with ILLink #18545

Merged
merged 5 commits into from
Jul 17, 2023
Merged

NativeAOT: Disable AggressiveAttributeTrimming with ILLink #18545

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f8a499
Disable AggressiveAttributeTrimming with ILLink
ivanpovazan Jul 10, 2023
05e7922
Merge remote-tracking branch 'upstream/net8.0' into naot-trim-enable
ivanpovazan Jul 11, 2023
706da1a
Properly include platform assembly
ivanpovazan Jul 11, 2023
0b4a9be
Merge remote-tracking branch 'upstream/net8.0' into naot-trim-enable
ivanpovazan Jul 12, 2023
9b31fbb
Merge branch 'net8.0' into naot-trim-enable
dalexsoto Jul 13, 2023
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
31 changes: 26 additions & 5 deletions dotnet/targets/Xamarin.Shared.Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@

<ProjectCapability Condition="'$(_KeepLaunchProfiles)' != 'true'" Remove="LaunchProfiles" />
</ItemGroup>


<!-- Default item includes (globs and implicit references) -->
<Import Project="Xamarin.Shared.Sdk.DefaultItems.targets" />

<PropertyGroup>
<!-- Add a property that specifies the name of the platform assembly for each platform -->
<_PlatformAssemblyName>Microsoft.$(_PlatformName)</_PlatformAssemblyName>
Expand Down Expand Up @@ -132,7 +135,6 @@
<UseSystemResourceKeys Condition="'$(UseSystemResourceKeys)' == '' And '$(_BundlerDebug)' == 'true'">false</UseSystemResourceKeys>
<UseNativeHttpHandler Condition="'$(_PlatformName)' != 'macOS' And '$(UseNativeHttpHandler)' == ''">true</UseNativeHttpHandler>
<!-- AutoreleasePoolSupport needs to be set earlier, so that illink doesn't override it - https://github.com/dotnet/runtime/pull/86753 - so it's set in Xamarin.Shared.Sdk.props -->
<_AggressiveAttributeTrimming Condition="'$(_AggressiveAttributeTrimming)' == ''">true</_AggressiveAttributeTrimming>
<NullabilityInfoContextSupport Condition="'$(NullabilityInfoContextSupport)' == ''">false</NullabilityInfoContextSupport>
<BuiltInComInteropSupport Condition="'$(BuiltInComInteropSupport)' == ''">false</BuiltInComInteropSupport>
<!-- Verify DI trimmability at development-time, but turn the validation off for production/trimmed builds. -->
Expand All @@ -141,6 +143,15 @@

<!-- We don't need to generate reference assemblies for apps or app extensions -->
<ProduceReferenceAssembly Condition="'$(ProduceReferenceAssembly)' == '' And ('$(OutputType)' == 'Exe' Or '$(IsAppExtension)' == 'true')">false</ProduceReferenceAssembly>

<!--
With NativeAOT we want to prevent ILLink from removing attributes like `IsTrimmable` so further trimming can be done by the NativeAOT toolchain.
For this reason, in case of NativeAOT, we set _AggressiveAttributeTrimming to false by default and store the overwriten default in
_OriginalAggressiveAttributeTrimming property, which is later used to properly configure NativeAOT trimming.
-->
<_OriginalAggressiveAttributeTrimming>$(_AggressiveAttributeTrimming)</_OriginalAggressiveAttributeTrimming>
<_AggressiveAttributeTrimming Condition="'$(_UseNativeAot)' == 'true'">false</_AggressiveAttributeTrimming>
<_AggressiveAttributeTrimming Condition="'$(_AggressiveAttributeTrimming)' == ''">true</_AggressiveAttributeTrimming>
</PropertyGroup>

<PropertyGroup>
Expand All @@ -152,9 +163,6 @@
<TargetPlatformSupported Condition=" '$(TargetPlatformIdentifier)' == '$(_PlatformName)' ">true</TargetPlatformSupported>
</PropertyGroup>

<!-- Default item includes (globs and implicit references) -->
<Import Project="Xamarin.Shared.Sdk.DefaultItems.targets" />

<PropertyGroup Condition="'$(_GlobalizationDataFileLocation)' == ''">
<_GlobalizationDataFileLocation Condition="'$(_UseNativeAot)' == 'true'">Resource</_GlobalizationDataFileLocation>
<_GlobalizationDataFileLocation Condition="'$(_UseNativeAot)' != 'true'">Assembly</_GlobalizationDataFileLocation>
Expand Down Expand Up @@ -1216,6 +1224,19 @@

<!-- Link in the output from ILC -->
<_NativeExecutableObjectFiles Include="$(NativeObject)" />

<!-- By default perform aggressive attribute trimming, except when explicitly disabled -->
<_TrimmerFeatureSettings Condition="'$(_OriginalAggressiveAttributeTrimming)' != 'false' and '%(Identity)' == 'System.AggressiveAttributeTrimming'">
<Value>true</Value>
<Trim>true</Trim>
</_TrimmerFeatureSettings>
<RuntimeHostConfigurationOption Condition="'$(_OriginalAggressiveAttributeTrimming)' != 'false' and '%(Identity)' == 'System.AggressiveAttributeTrimming'">
<Value>true</Value>
<Trim>true</Trim>
</RuntimeHostConfigurationOption>

<!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 -->
<TrimmerRootAssembly Include="Microsoft.iOS" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<TrimmerRootAssembly Include="Microsoft.iOS" />
<TrimmerRootAssembly Include="$(_PlatformAssemblyName)" />

Copy link
Contributor

Choose a reason for hiding this comment

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

The name is different for each platform, so this needs to be adjusted. Aside from that, is there a longer description why is this necessary? I suppose with #18519 and #18529 this would no longer be necessary, correct?

Copy link
Contributor Author

@ivanpovazan ivanpovazan Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the correction, it was an oversight. I corrected it.

I suppose with #18519 and #18529 this would no longer be necessary, correct?

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]. The issue I noted in the comment should track/document all these incompatibilities.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I am not mistaken, there is still an issue with ILC not knowing about [Preserve].

I am almost sure that [Preserve] in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.

</ItemGroup>

<ItemGroup>
Expand Down