Skip to content

Report CORINFO_CALLCONV_GENERIC to JIT#121561

Merged
jkotas merged 4 commits intodotnet:mainfrom
MichalStrehovsky:genericcallconv
Nov 13, 2025
Merged

Report CORINFO_CALLCONV_GENERIC to JIT#121561
jkotas merged 4 commits intodotnet:mainfrom
MichalStrehovsky:genericcallconv

Conversation

@MichalStrehovsky
Copy link
Member

Outerloops are broken.

Apparently we got away without reporting this until the test in #121130 was added. Not reporting the calling convention results in this not kicking in:

eeGetMethodSig(resolvedToken.hMethod, &sig);
if (sig.numArgs != info.compMethodInfo->args.numArgs ||
sig.retType != info.compMethodInfo->args.retType ||
sig.callConv != info.compMethodInfo->args.callConv)
{
BADCODE("Incompatible target for CEE_JMPs");
}

and failing the test.

Cc @dotnet/ilc-contrib

Apparently we got away without reporting this until the test in dotnet#121130 was added. Not reporting the calling convention results in this not kicking in:

https://github.com/dotnet/runtime/blob/5e97723bdd8eb9eff95c52b7120cec6bfb5a0a19/src/coreclr/jit/importer.cpp#L7291-L7297

And failing the test.
Copilot AI review requested due to automatic review settings November 12, 2025 21:51
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 12, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes broken outerloop tests by ensuring that the CORINFO_CALLCONV_GENERIC calling convention flag is properly reported to the JIT compiler. Previously, the AOT compiler was not setting the generic flag in method signatures, which caused the JIT to miss important optimizations/checks when compiling generic method calls.

  • Added Generic flag to MethodSignatureFlags enum
  • Implemented IsGeneric property on MethodSignature to check the flag
  • Set the flag during ECMA signature parsing when the metadata indicates a generic signature
  • Report the flag to JIT through CORINFO_CALLCONV_GENERIC in the calling convention

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/coreclr/tools/Common/TypeSystem/Ecma/EcmaSignatureParser.cs Set the Generic flag when parsing method signatures with generic parameters from ECMA metadata
src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs Add Generic flag enum value and IsGeneric property to MethodSignature class
src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs Report CORINFO_CALLCONV_GENERIC to JIT when signature is generic

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@MichalStrehovsky
Copy link
Member Author

There was an error trying to commit changes.

Thanks for nothing, Github.

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jkotas jkotas added area-NativeAOT-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 12, 2025
@dotnet-policy-service
Copy link
Contributor

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

@jkotas
Copy link
Member

jkotas commented Nov 13, 2025

/ba-g timeouts

@jkotas jkotas merged commit 228a12c into dotnet:main Nov 13, 2025
109 of 121 checks passed
@MichalStrehovsky MichalStrehovsky deleted the genericcallconv branch November 13, 2025 11:03
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants