[trimmer] per-RID linked corlib assembly contains foreign platform intrinsics classes

[grendello]

This happens on xamarin-android/main (.NET8), but I suppose it will apply to other platforms as well.

I noticed that System.Private.CoreLib.dll output by the linker for arm64 RID contains X86 and WASM intrinsics types:

549: System.Runtime.Intrinsics.Wasm.PackedSimd (flist=2253, mlist=6116, flags=0x100081, extends=0xa0)
554: System.Runtime.Intrinsics.X86.X86Base (flist=2253, mlist=6151, flags=0x100081, extends=0xa0)
555: System.Runtime.Intrinsics.X86.Sse (flist=2253, mlist=6151, flags=0x100081, extends=0x8a8)
556: System.Runtime.Intrinsics.X86.Sse2 (flist=2253, mlist=6151, flags=0x100081, extends=0x8ac)
557: System.Runtime.Intrinsics.X86.Sse3 (flist=2253, mlist=6154, flags=0x100081, extends=0x8b0)
558: System.Runtime.Intrinsics.X86.Sse41 (flist=2253, mlist=6154, flags=0x100081, extends=0x8bc)
559: System.Runtime.Intrinsics.X86.Ssse3 (flist=2253, mlist=6155, flags=0x100081, extends=0x8b4)

It appears some of the x86 classes are removed, but not all. For comparison, this is a list of types from the same assembly but linked for x84:

558: System.Runtime.Intrinsics.X86.X86Base (flist=2255, mlist=6330, flags=0x100081, extends=0xa0)
559: System.Runtime.Intrinsics.X86.X86Base/X64 (flist=2255, mlist=6333, flags=0x100082, extends=0xa0)
560: System.Runtime.Intrinsics.X86.Avx (flist=2255, mlist=6336, flags=0x100081, extends=0x90c)
561: System.Runtime.Intrinsics.X86.Avx2 (flist=2255, mlist=6338, flags=0x100081, extends=0x8c0)
562: System.Runtime.Intrinsics.X86.Avx512BW (flist=2255, mlist=6344, flags=0x100081, extends=0x8cc)
563: System.Runtime.Intrinsics.X86.Avx512F (flist=2255, mlist=6346, flags=0x100081, extends=0x8c4)
564: System.Runtime.Intrinsics.X86.Bmi1 (flist=2255, mlist=6347, flags=0x100081, extends=0x8b8)
565: System.Runtime.Intrinsics.X86.Bmi1/X64 (flist=2255, mlist=6349, flags=0x100082, extends=0x8bc)
566: System.Runtime.Intrinsics.X86.Bmi2 (flist=2255, mlist=6352, flags=0x100081, extends=0x8b8)
567: System.Runtime.Intrinsics.X86.Bmi2/X64 (flist=2255, mlist=6352, flags=0x100082, extends=0x8bc)
568: System.Runtime.Intrinsics.X86.Lzcnt (flist=2255, mlist=6354, flags=0x100081, extends=0x8b8)
569: System.Runtime.Intrinsics.X86.Lzcnt/X64 (flist=2255, mlist=6356, flags=0x100082, extends=0x8bc)
570: System.Runtime.Intrinsics.X86.Popcnt (flist=2255, mlist=6358, flags=0x100081, extends=0x90c)
571: System.Runtime.Intrinsics.X86.Sse (flist=2255, mlist=6360, flags=0x100081, extends=0x8b8)
572: System.Runtime.Intrinsics.X86.Sse/X64 (flist=2255, mlist=6360, flags=0x100082, extends=0x8bc)
573: System.Runtime.Intrinsics.X86.Sse2 (flist=2255, mlist=6360, flags=0x100081, extends=0x8ec)
574: System.Runtime.Intrinsics.X86.Sse2/X64 (flist=2255, mlist=6373, flags=0x100082, extends=0x8f0)
575: System.Runtime.Intrinsics.X86.Sse3 (flist=2255, mlist=6376, flags=0x100081, extends=0x8f4)
576: System.Runtime.Intrinsics.X86.Sse3/X64 (flist=2255, mlist=6376, flags=0x100082, extends=0x8f8)
577: System.Runtime.Intrinsics.X86.Sse41 (flist=2255, mlist=6376, flags=0x100081, extends=0x910)
578: System.Runtime.Intrinsics.X86.Sse41/X64 (flist=2255, mlist=6379, flags=0x100082, extends=0x914)
579: System.Runtime.Intrinsics.X86.Sse42 (flist=2255, mlist=6380, flags=0x100081, extends=0x904)
580: System.Runtime.Intrinsics.X86.Ssse3 (flist=2255, mlist=6380, flags=0x100081, extends=0x8fc)
581: System.Runtime.Intrinsics.X86.Ssse3/X64 (flist=2255, mlist=6382, flags=0x100082, extends=0x900)

The x64 version also contains some ARM intrinsics, but not all:

553: System.Runtime.Intrinsics.Wasm.PackedSimd (flist=2255, mlist=6322, flags=0x100081, extends=0xa0)
554: System.Runtime.Intrinsics.Arm.AdvSimd (flist=2255, mlist=6325, flags=0x100081, extends=0x8b0)
555: System.Runtime.Intrinsics.Arm.AdvSimd/Arm64 (flist=2255, mlist=6326, flags=0x100082, extends=0x8b4)
556: System.Runtime.Intrinsics.Arm.ArmBase (flist=2255, mlist=6330, flags=0x100081, extends=0xa0)
557: System.Runtime.Intrinsics.Arm.ArmBase/Arm64 (flist=2255, mlist=6330, flags=0x100082, extends=0xa0)

Would it be possible to remove all of them? They are essentially dead weight for the target RID.

Attached is a zip with both assemblies mentioned above. The app being built was the dotnet new android template for NET8 corlib.zip

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics See info in area-owners.md if you want to be subscribed.

Issue Details

---

This happens on xamarin-android/main (.NET8), but I suppose it will apply to other platforms as well. I noticed that `System.Private.CoreLib.dll` output by the linker for arm64 RID contains X86 and WASM intrinsics types: ``` 549: System.Runtime.Intrinsics.Wasm.PackedSimd (flist=2253, mlist=6116, flags=0x100081, extends=0xa0) 554: System.Runtime.Intrinsics.X86.X86Base (flist=2253, mlist=6151, flags=0x100081, extends=0xa0) 555: System.Runtime.Intrinsics.X86.Sse (flist=2253, mlist=6151, flags=0x100081, extends=0x8a8) 556: System.Runtime.Intrinsics.X86.Sse2 (flist=2253, mlist=6151, flags=0x100081, extends=0x8ac) 557: System.Runtime.Intrinsics.X86.Sse3 (flist=2253, mlist=6154, flags=0x100081, extends=0x8b0) 558: System.Runtime.Intrinsics.X86.Sse41 (flist=2253, mlist=6154, flags=0x100081, extends=0x8bc) 559: System.Runtime.Intrinsics.X86.Ssse3 (flist=2253, mlist=6155, flags=0x100081, extends=0x8b4) ``` It appears some of the x86 classes are removed, but not all. For comparison, this is a list of types from the same assembly but linked for `x84`: ``` 558: System.Runtime.Intrinsics.X86.X86Base (flist=2255, mlist=6330, flags=0x100081, extends=0xa0) 559: System.Runtime.Intrinsics.X86.X86Base/X64 (flist=2255, mlist=6333, flags=0x100082, extends=0xa0) 560: System.Runtime.Intrinsics.X86.Avx (flist=2255, mlist=6336, flags=0x100081, extends=0x90c) 561: System.Runtime.Intrinsics.X86.Avx2 (flist=2255, mlist=6338, flags=0x100081, extends=0x8c0) 562: System.Runtime.Intrinsics.X86.Avx512BW (flist=2255, mlist=6344, flags=0x100081, extends=0x8cc) 563: System.Runtime.Intrinsics.X86.Avx512F (flist=2255, mlist=6346, flags=0x100081, extends=0x8c4) 564: System.Runtime.Intrinsics.X86.Bmi1 (flist=2255, mlist=6347, flags=0x100081, extends=0x8b8) 565: System.Runtime.Intrinsics.X86.Bmi1/X64 (flist=2255, mlist=6349, flags=0x100082, extends=0x8bc) 566: System.Runtime.Intrinsics.X86.Bmi2 (flist=2255, mlist=6352, flags=0x100081, extends=0x8b8) 567: System.Runtime.Intrinsics.X86.Bmi2/X64 (flist=2255, mlist=6352, flags=0x100082, extends=0x8bc) 568: System.Runtime.Intrinsics.X86.Lzcnt (flist=2255, mlist=6354, flags=0x100081, extends=0x8b8) 569: System.Runtime.Intrinsics.X86.Lzcnt/X64 (flist=2255, mlist=6356, flags=0x100082, extends=0x8bc) 570: System.Runtime.Intrinsics.X86.Popcnt (flist=2255, mlist=6358, flags=0x100081, extends=0x90c) 571: System.Runtime.Intrinsics.X86.Sse (flist=2255, mlist=6360, flags=0x100081, extends=0x8b8) 572: System.Runtime.Intrinsics.X86.Sse/X64 (flist=2255, mlist=6360, flags=0x100082, extends=0x8bc) 573: System.Runtime.Intrinsics.X86.Sse2 (flist=2255, mlist=6360, flags=0x100081, extends=0x8ec) 574: System.Runtime.Intrinsics.X86.Sse2/X64 (flist=2255, mlist=6373, flags=0x100082, extends=0x8f0) 575: System.Runtime.Intrinsics.X86.Sse3 (flist=2255, mlist=6376, flags=0x100081, extends=0x8f4) 576: System.Runtime.Intrinsics.X86.Sse3/X64 (flist=2255, mlist=6376, flags=0x100082, extends=0x8f8) 577: System.Runtime.Intrinsics.X86.Sse41 (flist=2255, mlist=6376, flags=0x100081, extends=0x910) 578: System.Runtime.Intrinsics.X86.Sse41/X64 (flist=2255, mlist=6379, flags=0x100082, extends=0x914) 579: System.Runtime.Intrinsics.X86.Sse42 (flist=2255, mlist=6380, flags=0x100081, extends=0x904) 580: System.Runtime.Intrinsics.X86.Ssse3 (flist=2255, mlist=6380, flags=0x100081, extends=0x8fc) 581: System.Runtime.Intrinsics.X86.Ssse3/X64 (flist=2255, mlist=6382, flags=0x100082, extends=0x900) ``` The `x64` version also contains some ARM intrinsics, but not all: ``` 553: System.Runtime.Intrinsics.Wasm.PackedSimd (flist=2255, mlist=6322, flags=0x100081, extends=0xa0) 554: System.Runtime.Intrinsics.Arm.AdvSimd (flist=2255, mlist=6325, flags=0x100081, extends=0x8b0) 555: System.Runtime.Intrinsics.Arm.AdvSimd/Arm64 (flist=2255, mlist=6326, flags=0x100082, extends=0x8b4) 556: System.Runtime.Intrinsics.Arm.ArmBase (flist=2255, mlist=6330, flags=0x100081, extends=0xa0) 557: System.Runtime.Intrinsics.Arm.ArmBase/Arm64 (flist=2255, mlist=6330, flags=0x100082, extends=0xa0) ``` Would it be possible to remove all of them? They are essentially dead weight for the target RID.

Author:	grendello
Assignees:	-
Labels:	`area-System.Runtime.Intrinsics`
Milestone:	-

[akoeplinger]

They're basically preserved because of this reference:

This is coming from here: https://github.com/dotnet/runtime/blob/f20509b9ea563da18af963976b7db0e523e6837e/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Transcoding.cs#L969-L978

I'm assuming the linker isn't able to figure out that this else is not possible, I think we could add else if (Sse41.X64.IsSupported) instead there.

[grendello]

I noticed the following after disassembling the linker binary:

.method /*06000800*/ private hidebysig static 
          valuetype System.ValueTuple`3/*02000105*/<valuetype System.Runtime.Intrinsics.Vector128`1/*02000220*/<uint8>,valuetype System.Runtime.Intrinsics.Vector128`1/*02000220*/<uint8>,valuetype System.Runtime.Intrinsics.Vector128`1/*02000220*/<uint8>> 
          FormatGuidVector128Utf8(valuetype System.Guid/*02000096*/ A_0,
                                  bool A_1) cil managed aggressiveinlining
  // SIG: 00 02 15 11 84 14 03 15 11 88 80 01 05 15 11 88 80 01 05 15 11 88 80 01 05 11 82 58 02
  {
    .custom /*0C0001CD:06001894*/ instance void System.Runtime.CompilerServices.CompExactlyDependsOnAttribute/*02000255*/::.ctor(class System.Type/*02000038*/) /* 06001894 */ = ( 01 00 23 53 79 73 74 65 6D 2E 52 75 6E 74 69 6D   // ..#System.Runtim
                                                                                                                                                                                   65 2E 49 6E 74 72 69 6E 73 69 63 73 2E 58 38 36   // e.Intrinsics.X86
                                                                                                                                                                                   2E 53 73 73 65 33 00 00 )                         // .Ssse3..
    .custom /*0C0001CE:06001894*/ instance void System.Runtime.CompilerServices.CompExactlyDependsOnAttribute/*02000255*/::.ctor(class System.Type/*02000038*/) /* 06001894 */ = ( 01 00 2B 53 79 73 74 65 6D 2E 52 75 6E 74 69 6D   // ..+System.Runtim
                                                                                                                                                                                   65 2E 49 6E 74 72 69 6E 73 69 63 73 2E 41 72 6D   // e.Intrinsics.Arm
                                                                                                                                                                                   2E 41 64 76 53 69 6D 64 2B 41 72 6D 36 34 00 00 ) // .AdvSimd+Arm64..

.method /*06000F96*/ private hidebysig static 
          void  EncodeToUtf16_Vector128(valuetype System.ReadOnlySpan`1/*020000D7*/<uint8> A_0,
                                        valuetype System.Span`1/*020000DB*/<char> A_1,
                                        valuetype System.HexConverter/*0200010C*//Casing/*0200010D*/ A_2) cil managed
  // SIG: 00 03 01 15 11 83 5C 01 05 15 11 83 6C 01 03 11 84 34
  {
    .custom /*0C000228:06001894*/ instance void System.Runtime.CompilerServices.CompExactlyDependsOnAttribute/*02000255*/::.ctor(class System.Type/*02000038*/) /* 06001894 */ = ( 01 00 23 53 79 73 74 65 6D 2E 52 75 6E 74 69 6D   // ..#System.Runtim
                                                                                                                                                                                   65 2E 49 6E 74 72 69 6E 73 69 63 73 2E 58 38 36   // e.Intrinsics.X86
                                                                                                                                                                                   2E 53 73 73 65 33 00 00 )                         // .Ssse3..

[tannergooding]

@akoeplinger

I think we could add else if (Sse41.X64.IsSupported) instead there.

It's not clear why this would be needed.

L941 is if (Sse41.X64.IsSupported || (AdvSimd.Arm64.IsSupported && BitConverter.IsLittleEndian)). On x64 it might be true || (false && ...) while on Arm64 it might be false || (true && ...), and any other platform it should resolve to false || (false && ...). In all of these cases, the logic is trivially constant true or constant false and therefore the block from L941-1067 should be either kept or preserved.

Correspondingly the inner if (AdvSimd.Arm64.IsSupported) on L956 should make the else on L969-L978 "dead" for Arm64 and therefore only live on x64 when Sse41.X64.IsSupported.

So this seems rather like the trimmer is leaving code in for otherwise simple patterns, where there isn't really anything complex that should prevent it from understanding it can be kept alive or treated as dead

[akoeplinger]

@tannergooding I looked into this again and when building for arm64 the AdvSimd.Arm64.IsSupported is not substituted as true because we don't have ILLink.Substitutions.xml for that so the linker can't remove that check and thus also keeps the else alive. That's why adding else if (Sse41.X64.IsSupported) would "fix" this too since we substitute that to false via ILLink.Substitutions.NoX86Intrinsics.xml

afaik these simd instructions are part of arm64 right? so we could add the substitutions for arm64.

[tannergooding]

we don't have ILLink.Substitutions.xml for that

@akoeplinger, is Mono using different ILLink files than RyuJIT?

We have https://github.com/tannergooding/runtime/blob/main/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.NoArmIntrinsics.xml (with equivalents for x86/wasm) and those should be getting included automatically: https://github.com/tannergooding/runtime/blob/80dbd6929c00bf4d97dba93f7fb3efe2672951d5/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems#L58-L60

I know that there is then additional CoreCLR (https://github.com/dotnet/runtime/tree/main/src/coreclr/System.Private.CoreLib/src/ILLink) and Mono (https://github.com/dotnet/runtime/tree/main/src/mono/System.Private.CoreLib/src/ILLink) specific files, but I wouldn't expect those to conflict or cause the above to not be included

[akoeplinger]

right, that works for removing AdvSimd on x86/x64, but on arm64 we don't have anything that makes IsSupported=true

[tannergooding]

Ah right and it can’t be constant true for the IL because the target platform might not support it.

The JIT or AOT needs to be folding it to constant true

So we would need to update this and other paths to be explicit about the negative isa check for the linker to be able to trim it

[tannergooding]

That is, the linker isn’t smart enough to do the rewriting itself and understand that the else is dead because the outer if itself became if (AdvSimd.IsSupported) and therefore the inner if (AdvSimd.IsSupported) is the only path that can be taken

[akoeplinger]

Ah right and it can’t be constant true for the IL because the target platform might not support it.

for arm64 it could be constant true: https://github.com/dotnet/runtime/blob/c88c31b3dff2e0f027379d947414fbced8c4b911/src/mono/mono/mini/mini.c#L4543-L4546

[tannergooding]

I don't think we want to rely on that, in case it changes in the future or Mono mirrors other core features that RyuJIT has (like supporting the DOTNET_EnableAdvSimd=0 environment variable).

I'd guess we also want the trimming to work for RyuJIT, so I think the best short term fix here is to identify places like the above and update them to explicitly have the checks the trimmer does support (that is ensure we do else if (Sse41.IsSupported) { } instead of just else { })

Long term, the "better" fix would be if the trimmer could identify cases like:

if (boolExpr1 || boolExpr2)
{
    if (boolExpr2)
    {
    }
    else
    {
    }
}

and understand that if boolExpr1 is constant false that it means the interior boolExpr2 must be constant true, because it can only be reached if the outermost evaluated boolExpr2 to true at first

[tannergooding]

Have a PR up here that should resolve most of this: https://github.com/dotnet/runtime/pull/106777

Needs review if anyone has time

[kasperk81]

That is, the linker isn’t smart enough to do the rewriting itself and understand that the else is dead because the outer if itself became if (AdvSimd.IsSupported) and therefore the inner if (AdvSimd.IsSupported) is the only path that can be taken

@sbomer @jtschuster, @vitek-karas

C = link-time constants X = non-constants

1. if (C1 or C2) { if(C1) { $c1-code } else { $c2-code }}

   • If C1 is true: Replace the entire block with $c1-code.
   • If C1 is false and C2 is true: Replace the entire block with $c2-code.

2. if (C1 and C2) { if(C1) { $c1-code } else { $c2-code }}

   • If C1 is true: Replace with if(C1) { $c1-code }.
   • If C1 is false: Remove the entire block.

3. if (C1 or X1) { if(C1) { $c1-code } else { $x1-code }}

   • If C1 is true: Replace the entire block with $c1-code.
   • If C1 is false and X1 is true: Replace the entire block with $x1-code.

4. if (C1 and X1) { if(C1) { $c1-code } else { $x1-code }}

   • If C1 is true: Replace with if(C1) { $c1-code }.
   • If C1 is false: Remove the entire block.

[vitek-karas]

The problem is not that it's hard to figure out any specific case, the problem is that there are lot of cases to cover. The trimmer sees the IL, which look quite a bit less clean and to make things worse C# compiler will produce different IL between debug and release. The trimmer pattern matching code for this is already relatively complicated. That's actually why we intentionally written down the set of patterns which we support and we explicitly state that anything else is NOT supported (although it might work sometimes). We have test coverage for the supported patterns, so we can guarantee that behavior. https://github.com/dotnet/runtime/blob/main/docs/design/tools/illink/feature-checks.md

I think what we're missing is additional diagnostics around this. I'll leave that up to the trimmer team, but one idea I had was to add logic into the analyzer (because it sees the C# which "looks nice") to detect supported cases, and for everything else it would produce a warning - this would only work for conditions which use the new feature switch attributed properties, since then the analyzer knows that it's a feature check and would produce the warning. It should flag the above cases as problematic.

Obviously, we could expand the set of supported patterns, but I think we should be careful about doing that - the logic is currently implemented 3 times (Differently) so it's relatively costly to add anything new.

[jeffhandley]

I'm punting this out to 10.0.0. If folks feel strongly about backporting this to .NET 9 after a fix is merged, it can be reviewed and discussed.