JIT: Use metadata names for SIMD intrinsic method recognition

[jakobbotsch]

eeGetMethodName is only expected to be used for diagnostics.

In practice this does not cause issues, but some odd situations could occur in the cases where eeGetMethodName returns a placeholder name during SPMI replay.

Also avoid unnecessary EE call in impSIMDIntrinsic in the common case of no SIMD intrinsic.

[ghost]

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

Issue Details

---

eeGetMethodName is only expected to be used for diagnostics.

Author:	jakobbotsch
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

If I am reading the code correctly, we are always going to ask for getMethodNameFromMetadata twice for SIMD methods. First time during attempt to import non-SIMD intrinsic and second time here. Would it be worth it to avoid these duplicate calls?

[jkotas]

If it is too complex refactoring, we should add if (isIntrinsic) condition around the following in importer.cpp:

    {
        call = impSIMDIntrinsic(opcode, newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken->token);
        if (call != nullptr)
        {
            bIntrinsicImported = true;
            goto DONE_CALL;
        }

and flip the same check in impSIMDIntrinsic to assert. It will avoid the more expensive isSIMDClass checks for nearly all methods calls.

[jakobbotsch]

It's a good point. The refactoring does look a little complicated to do, but impImportCall is long overdue for a refactor so maybe I can spend some time on that. I also noticed that we needlessly query the signature for all hardware intrinsics even though we already have it from the call info.

[jakobbotsch]

@jkotas I started with a larger refactoring but as you mentioned it does end up being a more major operation, so for this PR I went with your second suggestion instead.

[jakobbotsch]

This logic seems a bit odd to me in terms of the namespaceResult, IIUC it will work only for one level of nesting.

[jkotas]

This is designed to handle just enough for the JIT to identify intrinsics. We only need one level of nesting for that.

[tannergooding]

Should this just be part of the slightly higher check that already exists?

There isn't any real reason it should be separate or not handled in impIntrinsic...

[jakobbotsch]

I initially tried refactoring this and moving it in to be part of impIntrinsic, but it turned out to be a bit more work than I wanted here. Also wasn't totally sure with the alt-jit check if I could just move it under the existing if.

I am planning to do some more refactoring for this (starting with #76792) so I think I will leave the further clean up to a future PR.

[jakobbotsch]

Failure is known according to build analysis.