Convert some old style intrinsics to NamedIntrinsic

[am11]

Converted:

• CORINFO_INTRINSIC_RTH_GetValueInternal -> NI_System_RuntimeTypeHandle_GetValueInternal
• CORINFO_INTRINSIC_Object_GetType -> NI_System_Object_GetType
• CORINFO_INTRINSIC_StubHelpers_GetStubContext -> NI_System_StubHelpers_GetStubContext
• CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr -> NI_System_StubHelpers_GetStubContext
• CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress -> NI_System_StubHelpers_GetStubContext

[ghost]

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

Issue Details

---

Converted:

• CORINFO_INTRINSIC_RTH_GetValueInternal -> NI_System_RuntimeTypeHandle_GetValueInternal
• CORINFO_INTRINSIC_Object_GetType -> NI_System_Object_GetType
• CORINFO_INTRINSIC_StubHelpers_GetStubContext -> NI_System_StubHelpers_GetStubContext
• CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr -> NI_System_StubHelpers_GetStubContext
• CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress -> NI_System_StubHelpers_GetStubContext

Author:	am11
Assignees:	-
Labels:	area-CodeGen-coreclr, community-contribution
Milestone:	-

[EgorBo]

Thanks for doing this, I recall there was some problem with GetType one in crossgen2 but I don't remember the details - let's see what CI thinks about it first 🙂

[am11]

cc @EgorBo, @jkotas, PTAL.

I have added a TODO in interpreter, since named intrinsics are currently accessible only within the JIT. We can add a separate lookup table for interpreter, once someone will start working on fixing the interpreter.

[EgorBo]

@am11 LGTM, but could you please make sure it doesn't produce any diffs? because I'm a bit worried about the GetType
Unfortunately you can't check it via superpmi and the only option is jit-utils https://github.com/dotnet/jitutils

1. build two runtimes on the same commits (e.g. C:\prj\runtime-base and C:\prj\runtime-diff)
   runtimes should be built like this:

build.cmd Clr+Libs -c Release
build.cmd Clr -c Checked
cd src\tests
build.cmd Checked generatelayoutonly
build.cmd Release generatelayoutonly
cd ..\..

2. apply your changes to runtime-diff.
3. run

jit-diff diff --output C:\prj\jit-diffs -f --core_root C:\prj\runtime-base\artifacts\tests\coreclr\windows.x64.Release\Tests\Core_Root --base C:\prj\runtime-base\artifacts\bin\coreclr\windows.x64.Checked --crossgen C:\prj\runtime-base\artifacts\tests\coreclr\windows.x64.Release\Tests\Core_Root\crossgen.exe

jit-diff diff --output C:\prj\jit-diffs -f --core_root C:\prj\runtime-diff\artifacts\tests\coreclr\windows.x64.Release\Tests\Core_Root --base C:\prj\runtime-diff\artifacts\bin\coreclr\windows.x64.Checked --crossgen C:\prj\runtime-diff\artifacts\tests\coreclr\windows.x64.Release\Tests\Core_Root\crossgen.exe

jit-analyze -b C:\prj\jit-diffs\dasmdiffs0\base -d C:\prj\jit-diffs\dasmdiffs1\base -r -c 100

it's a lot of steps but unfortunately for this specific case where we want to test changes in vm instead of jit it's the only option

[am11]

it's a lot of steps but unfortunately for this specific case where we want to test changes in vm instead of jit it's the only option

Yup, I remember, not my first time though dotnet/jitutils#275 ;)

[am11]

No diffs; only textual diffs are found.

Found 276 files with textual diffs.

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 36335206
Total bytes of diff: 36335206
Total bytes of delta: 0 (0.00 % of base)
Total relative delta: NaN
    relative diff is a regression.


0 total files with Code Size differences (0 improved, 0 regressed), 273 unchanged.

0 total methods with Code Size differences (0 improved, 0 regressed), 244797 unchanged.

@EgorBo, some paths are changed in latest tooling (e,g dasmdiffs0 and dasmdiffs1 are now dasmset_1 and dasmset_2; Core_Root/crossgen is Core_Root/crossgen2/crossgen2). I have written an on-demand GH workflow to make it easier for future https://github.com/am11/CrossRepoCITesting/blob/02e887a/.github/workflows/jit-diff.yml. e.g. for this run https://github.com/am11/CrossRepoCITesting/actions/runs/1538330009, it was dispatched with these parameters:

[EgorBo]

LGTM, Thanks!

[am11]

@EgorBo, if there are no further comments, can this be merged? Thanks!

[jkotas]

I think the pStubArg also needs to be deleted at internal static extern IntPtr GetDelegateTarget(Delegate pThis, ref IntPtr pStubArg); in StubHelpers.cs

[jkotas]

Thank you!

I will be happy to help you to work through the issues that prevent getting rid of the few remaining old-style intrinsics.

[am11]

@jkotas, thank you! I have pushed the w.i.p commit to the same branch main...am11:feature/jit/namedintrinsic. Currently, it only compiles the native code, but failing on runtime.

$ artifacts/bin/testhost/net7.0-Linux-Debug-x64/shared/Microsoft.NETCore.App/7.0.0/corerun \
     $(pwd)/../hwapp/bin/Debug/net6.0/hwapp.dll

Stack overflow.
Repeat 261416 times:
--------------------------------
   at System.Runtime.Intrinsics.X86.Sse2.get_IsSupported()
--------------------------------
   at System.SpanHelpers.IndexOf(Char ByRef, Char, Int32)
   at System.String.wcslen(Char*)
   at System.String.Ctor(Char*)
   at System.AppContext.Setup(Char**, Char**, Int32)

---

However, first on the main branch; I am still trying to figure out a C# code sample which will make the importer detect CORINFO_INTRINSIC_ARRAY_* class of instrinsics. This line

runtime/src/coreclr/jit/importer.cpp

Line 3855 in 1d4b5f6

JITDUMP("Intrinsic %s Recognized\n", impGetIntrinsicName(intrinsicID));

prints the other intrinsics in debug build (with DOTNET_JitDump=* ./corerun ...), but I am yet to see any of the array ones. 🤔

array like intrinsics which are recognized on hello-world start startup:

# runtime main branch
$ DOTNET_JitDump=* artifacts/bin/testhost/net7.0-Linux-Debug-x64/shared/Microsoft.NETCore.App/7.0.0/corerun \
     $(pwd)/../hwapp/bin/Debug/net6.0/hwapp.dll | grep -i intrinsic.*recog | sort -u | grep -i array
     
Named Intrinsic System.Array.GetLowerBound: Recognized
Named Intrinsic System.Runtime.CompilerServices.RuntimeHelpers.InitializeArray: Recognized
Named Intrinsic System.Runtime.InteropServices.MemoryMarshal.GetArrayDataReference: Not recognized

and these are the only old style intrinsics recognized:

$ DOTNET_JitDump=* artifacts/bin/testhost/net7.0-Linux-Debug-x64/shared/Microsoft.NETCore.App/7.0.0/corerun \
    $(pwd)/../hwapp/bin/Debug/net6.0/hwapp.dll | grep ^Intrinsic.*Recog | sort -u

Intrinsic CORINFO_INTRINSIC_ByReference_Ctor Recognized
Intrinsic CORINFO_INTRINSIC_ByReference_Value Recognized

[jkotas]

failing on runtime

You have some mix up between CORINFO_FLG_JIT_INTRINSIC and CORINFO_FLG_INTRINSIC flags. I think you may want to delete CORINFO_FLG_JIT_INTRINSIC and just use CORINFO_FLG_INTRINSIC everywhere. It should fix the crash as side-effect.

I am still trying to figure out a C# code sample which will make the importer detect CORINFO_INTRINSIC_ARRAY_* class of instrinsics

These intrinsics are generated for multi-dimensional arrays.

[am11]

Thanks! All three of them are detected with this program.

var x = new int[2,2];
x[0,0] = 2;
x[1,0] = 1;

unsafe
{
    fixed(int *y = &x[0,0])
    {
        Console.WriteLine("{0}, {1}, {2}, {3}", x.GetLowerBound(0), x.GetUpperBound(1), x[1,0], (IntPtr)y);
    }
}

(dotnet build -p:AllowUnsafeBlocks=true)

[am11]

I've pushed the array intrinsic implementation to the branch. Currently, corerun is getting a sigsegv.
clrstack: http://sprunge.us/cT7pzU (croerun built from main branch on the same system successfully executes this app).
(aside: interestingly export DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1 has no effect on this initialization path, it always seem to execute)

[am11]

Also, the last line just before the crash in jit dumps is related to stub helper:

****** DONE compiling ILStubClass:IL_STUB_PInvoke(long,long,int):int

[jkotas]

I do not see anything obvious that can explain this crash by just looking at the code.

Could you please open draft PR with the change to make it easy to comment on it? There are some minor issue that I would like to comment on.

[am11]

Opened #62639.