Add new Unmanaged MD calling convention value

[AaronRobinsonMSFT]

Currently throws "Not Supported".

Contributes to #38133
Fixes #38135

/cc @lambdageek @jkotas @tannergooding @333fred @jkoritzinsky @elinor-fung @davidwrighton

[Dotnet-GitSync-Bot]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[BruceForstall]

Technically, I think we need a new JIT/EE interface GUID

[BruceForstall]

It's really worrisome to change ilasm and esp. the ilasm parser since we currently don't have automated testing (or any testing). Maybe you could figure out a way to test it by replacing the ilasm used to build the coreclr Pri-1 tests, and verify they all build (and run) without error. You'll also want to test ilasm/ildasm round tripping of the new callKind.

[AaronRobinsonMSFT]

You'll also want to test ilasm/ildasm round tripping of the new callKind.

That seems to work for the few examples I have locally.

[jkotas]

Technically, I think we need a new JIT/EE interface GUID

The changes in corinfo.h can be easily excluded from this change to make new JIT/EE interface GUID unnecessary (even technically).

[jkotas]

That seems to work for the few examples I have locally.

I do not see how it can possible roundtrip with the current version of the change. I expect ildasm to error out on the unmanaged signatures.

[AaronRobinsonMSFT]

That seems to work for the few examples I have locally.

I do not see how it can possible roundtrip with the current version of the change. I expect ildasm to error out on the unmanaged signatures.

I compiled the following with ILAsm:

    IL_0020:  ldloc.3
    IL_0021:  calli      unmanaged int32(int32)
    IL_0026:  pop
    IL_0027:  ret

This decompiled without issue. However when I looked at it the actual IL the result is slightly off:

    IL_0020:  ldloc.3
    IL_0021:  calli      unmanaged cdecl int32(int32)
    IL_0026:  pop
    IL_0027:  ret

I observed the same thing in .NET Framework, but in that case it was slightly different because the signature token stated the correct thing (i.e. UNMANAGED) even though it displayed the cdecl above. I could be misunderstanding the overloaded "unmanaged" term here though.

It seems in this case, it isn't failing but instead falling back to some "default" value. I am still writing tests and haven't run through any actual runtime scenarios yet.

[AaronRobinsonMSFT]

@jkotas I now see what you are talking about (i.e. PrettyPrintSig()).

[AaronRobinsonMSFT]

Opinions on where some new tests should live?

Here are two I am considering:

1. coreclr/tests/src/baseservices/callconvs
2. coreclr/tests/src/Interop/CallConvs

I am leaning toward (1) since this really has nothing to do with interop but is support in the runtime for metadata related to calling conventions.

[AaronRobinsonMSFT]

I am going with (1) coreclr/tests/src/baseservices/callconvs above.

Well shoot. I didn't realize the IL tools weren't consumed from a local build following repo consolidation. This is unfortunate. @jkotas or @tannergooding any suggestions on how to author tests here?

1. Defer tests until we can update the package
2. Hijack IL SDK to use the locally built ILAsm

The above options are what I am thinking.

[jkotas]

Add the tests as disabled, and re-enable them once we get updated tooling

[tannergooding]

I think 2 would be good. Other tools, like the C# compiler, also support overriding/dogfooding the locally built copy or a user specified copy.

[jkotas]

Hijack IL SDK to use the locally built ILAsm

You would have an uphill battle to fight to make cross-compilation to work, and the rewire the partition dependencies. These are among the reasons why we are using LKG style model for build tools.

[AaronRobinsonMSFT]

@jkotas @tannergooding @davidwrighton Any additional feedback?

[AaronRobinsonMSFT]

@davidwrighton Any concerns here?

[davidwrighton]

No this looks good from my pov.