Configure illink task to trim unreachable platform-specific code

[jkotas]

IL linker is capable of constant propagation and dead code removal (see dotnet/linker#848).

We should configure linking for the runtime package to eliminate platform-specific unreachable code that is under conditions like:

• IntPtr.Size, sizeof(IntPtr)
• BitConverter.IsLittleEndian
• hardware intrinsics IsSupported methods
• System.Runtime.InteropService.OSPlatform

[jkotas]

Related #31712

[am11]

hardware intrinsics IsSupported methods

If the build machine has different hardware capabilities than that of consumers', wouldn't it produce unexpected result? For instance, if software fallback is removed by this optimization, and some consumer required it.

[Gnbrkm41]

Aren't HWIntrinsics configurable by environment variable before the execution of the runtime? https://github.com/dotnet/runtime/blob/master/src/coreclr/src/jit/jitconfigvalues.h#L223-L245
cc @tannergooding

[MichalStrehovsky]

Aren't HWIntrinsics configurable by environment variable before the execution of the runtime?

I think this is about e.g. all x64 intrinsics never returning IsSupported == true on ARM.

[tannergooding]

Aren't HWIntrinsics configurable by environment variable before the execution of the runtime?

I don't believe we currently support this for the R2R code. We would have to determine if it is appropriate to support for AOT code in general.

[ericstj]

My understanding is that this issue is calling for us to add configuration such as the following to our ILLink invokation.

    <type fullname="System.IntPtr">
      <method signature="System.Int32 get_Size()" body="stub" value="4" />
    </type>

And similar for each case @jkotas mentions above, and maintain a different set of values for each architecture and platform we build for.

Do we really think this is the right way to control this? Can we instead do so with metadata on the methods? That way anyone running the linker can benefit with a simple flag (eg: --honor-platform-specific-metadata-for-platform:plat-arch or something)

[marek-safar]

What metadata would you add the the method? The configuration for each RID which will be used during libraries build will be different and for some platforms (e.g. webassembly) much longer than what Jan listed in initial set. Perhaps a better example would be to use RuntimeInformation.OSArchitecture.

We could also embed such configuration file into System.Private.CoreLib for linker to use the data also when linking user apps for specific RID.

[ericstj]

What metadata would you add the the method

I was thinking something like

LinkAssumeReturnValue("xyz")

or

LinkAssumeReturnValueForRuntime("xyz", "rid")

I think it makes the most sense for this sort of information to be maintained parallel to the API which implements it (ideally in the same source file). Also we should have tests that make sure that whatever information is checked in agrees with the actual API behavior on that platform.

[tannergooding]

Some of the metadata, such as hardware intrinsic IsSupported checks, won't be doable in that manner as they are dependent on the underlying or target hardware.

[marek-safar]

Some of the metadata, such as hardware intrinsic IsSupported checks

It still might be useful, for example for platforms where the intrinsics set is restricted (e.g. wasm)

I was thinking something like

LinkAssumeReturnValueForRuntime("xyz", "rid")

How would that work for e.g. IntPtr.Size. Would you list every possible 32 bit rid in the values? Perhaps that could work for our build.

Note, the xml file/syntax will still be used to let users control features like System.Globalization.Invariant

[eerhardt]

I've opened #37615 which covers:

• IntPtr.Size, sizeof(IntPtr)
• BitConverter.IsLittleEndian
• hardware intrinsics IsSupported methods

sizeof(IntPtr) is supported by setting IntPtr.Size given this linker code:

https://github.com/mono/linker/blob/29fea6aa0778e7b4e2af0e276c18f968f407a050/src/linker/Linker.Steps/RemoveUnreachableBlocksStep.cs#L203-L223

The original proposed constants above that I don't know how to support with the ILLinker are:

• System.Runtime.InteropService.OSPlatform

The OSPlatform API is tricky because you always pass a parameter into the method, and I don't think the ILLinker supports trimming branches on methods that have parameters:

https://github.com/mono/linker/blob/29fea6aa0778e7b4e2af0e276c18f968f407a050/src/linker/Linker.Steps/RemoveUnreachableBlocksStep.cs#L181-L182

[tannergooding]

sizeof(IntPtr) is supported by setting IntPtr.Size given this linker code:

Should this also cover sizeof(void*) (and pointers in general)? Does this catch Unsafe.SizeOf<T> (which internally uses sizeof, iirc)?

[marek-safar]

Should this also cover sizeof(void*)

I don't think it should. It's rare to see this code used in conditional comparison

[ViktorHofer]

@eerhardt assigning you. Is the tracking work already done or is there something else necessary to be done for 5.0?

[eerhardt]

It is tracking filling out this TODO:

runtime/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

Line 20 in c652058

<!-- TODO: Enable these substitutions files for more platforms once https://github.com/mono/linker/issues/1260 is fixed -->

Which is blocked on the underlying linker issue.

[eerhardt]

This was addressed in #42578.