Consider refactoring the System.Runtime.Intrinsics

[tannergooding]

Currently System.Runtime.Intrinsics types have two implementations:

• Isa.cs which is recursive
• Isa.PlatformNotSupported.cs which is just throw new PlatformNotSupportedException()

When you are talking about pure IL size, this is a win as all "unsupported" intrinsics can have their method bodies shared.
However, in practice (I believe due to our usage of crossgen) we actually end up increasing the size of the assembly (If this is due to crossgen, then the "unsupported" methods are each 28 bytes in size to contain the logic for throwing an exception; compared to only 7 bytes for a non-expanded recursive call).

A local experiment shows we can reduce the size of the x64 System.Private.Corelib.dll by 98kb (from 8990kb to 8892kb) by refactoring all of the methods to no longer be recursive and instead to call into a shared throw helper:

       [DoesNotReturn]
       internal static void ThrowPlatformNotSupportedException()
       {
           throw new PlatformNotSupportedException();
       }

       [DoesNotReturn]
       internal static T ThrowPlatformNotSupportedException<T>()
       {
           throw new PlatformNotSupportedException();
       }

The downside to this approach is that the calls are no longer recursive and so the existing logic in the JIT to support invoking these via reflection will be broken.
However, we do support changing the IL for method bodies in the VM and do such with System.Runtime.CompilerServices.Unsafe and a few others, so we could theoretically do the same with the HWIntrinsics and given that this is only required during recursive calls we might even be able to make it "pay for play"

An indirect benefit of doing this is that dev's could actually see hwintrinsic codegen in the altjit which may assist with situations where ARM64 hardware is not readily available: #41518

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[tannergooding]

CC. @jkotas, @echesakovMSFT

[ghost]

Tagging subscribers to this area: @tannergooding
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently System.Runtime.Intrinsics types have two implementations:

• Isa.cs which is recursive
• Isa.PlatformNotSupported.cs which is just throw new PlatformNotSupportedException()

When you are talking about pure IL size, this is a win as all "unsupported" intrinsics can have their method bodies shared.
However, in practice (I believe due to our usage of crossgen) we actually end up increasing the size of the assembly (If this is due to crossgen, then the "unsupported" methods are each 28 bytes in size to contain the logic for throwing an exception; compared to only 7 bytes for a non-expanded recursive call).

A local experiment shows we can reduce the size of the x64 System.Private.Corelib.dll by 98kb (from 8990kb to 8892kb) by refactoring all of the methods to no longer be recursive and instead to call into a shared throw helper:

       [DoesNotReturn]
       internal static void ThrowPlatformNotSupportedException()
       {
           throw new PlatformNotSupportedException();
       }

       [DoesNotReturn]
       internal static T ThrowPlatformNotSupportedException<T>()
       {
           throw new PlatformNotSupportedException();
       }

The downside to this approach is that the calls are no longer recursive and so the existing logic in the JIT to support invoking these via reflection will be broken.
However, we do support changing the IL for method bodies in the VM and do such with System.Runtime.CompilerServices.Unsafe and a few others, so we could theoretically do the same with the HWIntrinsics and given that this is only required during recursive calls we might even be able to make it "pay for play"

An indirect benefit of doing this is that dev's could actually see hwintrinsic codegen in the altjit which may assist with situations where ARM64 hardware is not readily available: #41518

Author:	tannergooding
Assignees:	-
Labels:	area-System.Runtime.Intrinsics, untriaged
Milestone:	-

[jkotas]

reduce the size of the x64 System.Private.Corelib.dll

I think a better way to get even better size reduction would be to stop AOTing methods that always throw for R2R. Throwing exceptions is rare and slow, so it is ok to fallback to JIT for these cases.

we do support changing the IL for method bodies in the VM and do such with System.Runtime.CompilerServices.Unsafe and a few others

I see this as a hack to get equivalent of inlined IL. I do not think we should be growing it beyond that. Ideally, I would love to see this replaced by build-time step.

dev's could actually see hwintrinsic codegen in the altjit

Isn't the actual problem with altjit that arm intrinsics are not marked as intrinsics in x64 corelib?

Can the altjit problem be fixed by assuming that anything is intrinsic when the JIT is running in the altjit mode? The intrinsic attribute is a throughput optimization to avoid looking up and matching the method names for every method. It should be ok to not have this optimization in altjit mode.

[tannergooding]

I think a better way to get even better size reduction would be to stop AOTing methods that always throw for R2R. Throwing exceptions is rare and slow, so it is ok to fallback to JIT for these cases.

That's fair and also reasonable. I had just noticed the size difference when doing other investigations so wanted to raise its visibility.
If we can save size by just not AOTing these methods, that's also good.

I see this as a hack to get equivalent of inlined IL. I do not think we should be growing it beyond that. Ideally, I would love to see this replaced by build-time step.

If we don't go this route, it might still be worth seeing if the JIT can detect these methods as "supported intrinsics" in a cheapish way. We are currently taking up 6 bytes of IL per recursive hwintrinsic.
x86 has ~1297 unique method bodies while ARM has ~2307 (approx as I only looked at methods that only throw PNSE), so just from the perspective of IL we are spending ~7.6kb on x86 and ~13.5kb on ARM to support reflection and indirect invocation.

We currently look to see if the method calls itself to detect this and then just force expand in this scenario. If we could instead detect, for example, that we are in a method marked intrinsic that is just "newobj; throw", we might be able to expand anyways; but I'm not sure how expensive that would be.

Isn't the actual problem with altjit that arm intrinsics are not marked as intrinsics in x64 corelib?

Right, we only check for the type level attribute in the "supported" namespace for the VM layer; so the JIT never sees the methods as intrinsic otherwise.

[tannergooding]

Going to close this given the above conversation.