Runtime: JIT needs to handle all IsSupported hardware intrinsics

[CarolEidt]

All IsSupported intrinsics should be handled such that the appropriate path can be eliminated in the JIT.

[sdmaclea]

I don't remember this being broken. Is this a recent regression? Is it just that the code is not being properly eliminated?

[CarolEidt]

I'm not sure whether it's recent, but it doesn't seem to handle it as an intrinsic at all, so it winds up calling the IL method. I observed this with both the altjit and on arm64/windows but haven't tried it on native Linux.

[CarolEidt]

This is an issue for all platforms. If, for example, you run JIT\HardwareIntrinsics\Arm64\Simd\Simd.exe on an x86 platform, you will see a call to System.Runtime.Intrinsics.Arm.Arm64.Simd:get_IsSupported():bool. And if you run just about any test on arm64 (or using the arm64 altjit) you will see various calls to get_IsSupported for x86 intrinsics that should be treated as a constant false.

[tannergooding]

@CarolEidt, even if this isn't handled as intrinsic, why isn't the JIT inlining the method and treating it as a constant (given that the IL only ever returns false)?

[CarolEidt]

why isn't the JIT inlining the method and treating it as a constant (given that the IL only ever returns false)?

I haven't analyzed that, though I suspect that in some cases it's running into limits.

[BruceForstall]

I see the following when running PMI for arm32 altjit on an x86 Checked Windows VM:

JIT must expand the intrinsic!###System.Runtime.Intrinsics.X86.Aes:get_IsSupported():bool
JIT must expand the intrinsic!###System.Runtime.Intrinsics.X86.Aes:DecryptLast(struct,struct):struct
JIT must expand the intrinsic!###System.Runtime.Intrinsics.X86.Aes:EncryptLast(struct,struct):struct
JIT must expand the intrinsic!###System.Runtime.Intrinsics.X86.Aes:KeygenAssist(struct,ubyte):struct
JIT must expand the intrinsic!###System.Runtime.Intrinsics.X86.Avx:TestNotZAndNotC(struct,struct):bool
JIT must expand the intrinsic!###System.Runtime.Intrinsics.X86.Avx:TestNotZAndNotC(struct,struct):bool

How is the altjit scenario supposed to work here?

Does the arm32 JIT need to implement at least the IsSupported intrinsics?

[sdmaclea]

Does the arm32 JIT need to implement at least the IsSupported intrinsics?

Yes

[CarolEidt]

Does the arm32 JIT need to implement at least the IsSupported intrinsics?

Yes, all the JITs should implement all of them as intrinsics. It would reduce the messiness of having it generate code for those dead paths.

How is the altjit scenario supposed to work here?

I believe that, if the arm32 JIT supported the IsSupported intrinsics, this issue would go away for this case. That said, it seems like kind of an unfortunate situation, and I don't believe it's limited to the altjit (i.e. I don't think the VM is checking the ISA dependencies). I'm not sure how it would behave in a release compiler, but let's say a developer hadn't guarded their code with a call to IsSupported, but somehow knew that the path would not be hit for Arm. Then it could hit this weirdness.

Perhaps the JIT needs a "fallback" path for any "mustExpand" intrinsic that it doesn't know about, that simply generates a throw to the platform-not-supported exception.

[CarolEidt]

As discussed in dotnet/coreclr#23673, we should also unify the JITDUMP output for recognizing hardware intrinsics, to make it easier to track down issues.

[tannergooding]

I can take a look at this after https://github.com/dotnet/coreclr/pull/23673 and https://github.com/dotnet/coreclr/pull/23674 are merged.

[RussKeldorph]

@tannergooding @CarolEidt Is this fixed by dotnet/coreclr#23751?

[tannergooding]

Yes. I forgot to comment resolved by in the top post.