[tools] Convert IsARM64CallingConvention optimization to a trimmer feature switch Fixes #26105#26141
Conversation
…ature switch Replace the hand-written IL rewrite that inlined ObjCRuntime.Runtime.IsARM64CallingConvention with an ILLink trimmer feature switch (ObjCRuntime.Runtime.IsARM64CallingConvention), mirroring the DynamicRegistrationSupported / IsManagedStaticRegistrar approach. - Keep ObjCRuntime.Runtime.IsARM64CallingConvention as a public field (changing it to a property would be a binary breaking change); Runtime.cs is unchanged. - Substitute the field's value (and stub the GetIsARM64CallingConvention helper) via the 'ObjCRuntime.Runtime.IsARM64CallingConvention' feature switch, so the trimmer inlines reads of the field into constants and removes the native architecture detection - exactly replicating the previous hand-written IL optimization. The expected preservedapis files are therefore unchanged. - Add the substitution entries to the four ILLink.Substitutions.*.xml files. - Xamarin.Shared.Sdk.targets: set the feature switch based on whether the RuntimeIdentifier ends with "-arm64". - Remove the old optimizer plumbing (OptimizeGeneratedCode.cs, Target.cs, CoreOptimizeGeneratedCode.cs, OptimizeGeneratedCodeStep.cs) and disable the inline-is-arm64-calling-convention optimization flag (kept for compatibility), mirroring inline-intptr-size. - Keep the IL-checking in the linker test (it verifies the trimmer actually inlines the field value); the only change is that reads are now inlined regardless of [BindingImpl (BindingImplOptions.Optimizable)], since the trimmer (not the old custom optimizer) does the inlining. - Update the docs. Fixes #26105 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the ObjCRuntime.Runtime.IsARM64CallingConvention inlining optimization from the custom OptimizeGeneratedCode IL rewrite to an ILLink trimmer feature switch, aligning it with existing feature-switch based optimizations (e.g., dynamic registration support). This reduces custom linker plumbing while still ensuring the field read is folded into a constant for architecture-specific builds.
Changes:
- Removed the custom linker optimizer logic that rewrote
Runtime.IsARM64CallingConventionandGetIsARM64CallingConventioninto constants. - Added ILLink substitutions for
Runtime.IsARM64CallingConvention(field) andRuntime.GetIsARM64CallingConvention(method) gated on theObjCRuntime.Runtime.IsARM64CallingConventionfeature switch. - Set the new feature switch from MSBuild based on whether
$(RuntimeIdentifier)ends with-arm64, and updated linker tests + documentation accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/linker/OptimizeGeneratedCode.cs | Removes the old custom IL rewrite for IsARM64CallingConvention. |
| tools/linker/CoreOptimizeGeneratedCode.cs | Stops passing the old per-ABI inlining value into the optimizer. |
| tools/dotnet-linker/OptimizeGeneratedCodeStep.cs | Stops passing the old per-ABI inlining value into the optimizer. |
| tools/common/Target.cs | Removes the ABI-based computation for the old inlining optimization. |
| tools/common/Optimizations.cs | Keeps the optimization flag for compatibility, but disables platform applicability and removes “enabled by default” behavior. |
| tests/linker/BaseOptimizeGeneratedCodeTest.cs | Updates the test expectations: field reads should now be constant-folded regardless of [BindingImpl(Optimizable)]. |
| src/ILLink.Substitutions.iOS.xml | Adds field + method substitutions driven by the new feature switch. |
| src/ILLink.Substitutions.tvOS.xml | Adds field + method substitutions driven by the new feature switch. |
| src/ILLink.Substitutions.macOS.xml | Adds field + method substitutions driven by the new feature switch. |
| src/ILLink.Substitutions.MacCatalyst.xml | Adds field + method substitutions driven by the new feature switch. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Introduces MSBuild logic to set ObjCRuntime.Runtime.IsARM64CallingConvention based on the RID architecture. |
| docs/website/optimizations.md | Documents the switch-based approach and updates the legacy --optimize option behavior. |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🔥 [CI Build #81a8fc6] Test results 🔥Test results❌ Tests failed on VSTS: test results 3 tests crashed, 18 tests failed, 181 tests passed. Failures❌ dotnettests tests (iOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_ios (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - dotnettests_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ framework tests🔥 Failed catastrophically on VSTS: test results - framework (no summary found). Html Report (VSDrops) Download ❌ linker tests (iOS)3 tests failed, 12 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (MacCatalyst)3 tests failed, 12 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (macOS)4 tests failed, 17 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (tvOS)3 tests failed, 12 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Ventura (13) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS testsLinux Build VerificationPipeline on Agent |
…operty Addresses a PR review comment: IsARM64CallingConvention is a public field, not a property. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the hand-written IL rewrite that inlined ObjCRuntime.Runtime.IsARM64CallingConvention with an ILLink trimmer feature switch (ObjCRuntime.Runtime.IsARM64CallingConvention), mirroring the DynamicRegistrationSupported / IsManagedStaticRegistrar approach.
Fixes #26105
🤖 Pull request created by Copilot