JIT: Centralize WellKnownArg definitions#130252
Draft
jakobbotsch wants to merge 4 commits into
Draft
Conversation
Use a single wellknownargs.h that is included multiple times to build various pieces of information we query about arguments. Rewrite users in terms of columns defined in that file.
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
jakobbotsch
commented
Jul 6, 2026
jakobbotsch
commented
Jul 6, 2026
Comment on lines
7929
to
7935
| for (CallArg& arg : call->gtArgs.Args()) | ||
| { | ||
| switch (arg.GetWellKnownArg()) | ||
| if (!arg.IsUserArg() || (arg.GetWellKnownArg() == WellKnownArg::ThisPointer)) | ||
| { | ||
| case WellKnownArg::RetBuffer: | ||
| case WellKnownArg::ThisPointer: | ||
| case WellKnownArg::AsyncContinuation: | ||
| // Not part of signature but we still expect to see it here | ||
| continue; | ||
| case WellKnownArg::None: | ||
| break; | ||
| default: | ||
| assert(!"Unexpected well known arg to method GDV candidate"); | ||
| continue; | ||
| // Not part of the signature | ||
| continue; | ||
| } |
Member
Author
There was a problem hiding this comment.
Actual bug fix is here.
jakobbotsch
commented
Jul 6, 2026
Comment on lines
+34
to
+35
| WELL_KNOWN_ARG(SwiftError, "swift error", true, false) | ||
| WELL_KNOWN_ARG(SwiftSelf, "swift self", true, false) |
Member
Author
There was a problem hiding this comment.
These are defined in IL and also marked well-known, so I made them true for isILArg.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes WellKnownArg metadata into a single wellknownargs.h “X-macro” list that is included multiple times to generate the enum and related query helpers, and it adjusts GDV candidate argument handling to avoid asserting on unexpected well-known args.
Changes:
- Introduces
src/coreclr/jit/wellknownargs.has the single source of truth forWellKnownArgentries and their metadata. - Rewrites
WellKnownArg-related helpers (getWellKnownArgName,CallArg::IsUserArg,CallArg::IsArgAddedLate, arg dump short names) to be table-driven fromwellknownargs.h. - Updates GDV compatibility checking to skip non-signature arguments via
CallArg::IsUserArg()instead of asserting on “unexpected” well-known args.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/wellknownargs.h | New centralized table defining well-known args and their metadata. |
| src/coreclr/jit/morph.cpp | Converts getWellKnownArgName to use the centralized table. |
| src/coreclr/jit/importercalls.cpp | Updates GDV arg filtering logic; also includes a small indentation-only change. |
| src/coreclr/jit/importer.cpp | Refactors inline arg init to use IsUserArg/InstParam instead of a well-known switch. |
| src/coreclr/jit/gentree.h | Generates WellKnownArg enum members from the centralized table. |
| src/coreclr/jit/gentree.cpp | Table-driven IsArgAddedLate, IsUserArg, and short-name lookup for arg messages. |
| src/coreclr/jit/fginline.cpp | Refactors inline argument setup logic to use IsUserArg/InstParam. |
| src/coreclr/jit/compiler.hpp | Stops tagging the helper “method handle” arg as a well-known arg. |
| src/coreclr/jit/CMakeLists.txt | Adds wellknownargs.h to the JIT header list. |
Comments suppressed due to low confidence (1)
src/coreclr/jit/importercalls.cpp:7648
- Extra leading spaces were introduced on this loop and its body, making indentation inconsistent with the surrounding block and adding whitespace-only noise.
for (UINT32 i = 0; i < numberOfMethods; i++)
{
CORINFO_CONST_LOOKUP lookup = {};
info.compCompHnd->getFunctionFixedEntryPoint((CORINFO_METHOD_HANDLE)likelyMethods[i].handle, false,
&lookup);
This was referenced Jul 6, 2026
jakobbotsch
commented
Jul 6, 2026
| pParam->inlineInfo->InlineRoot->m_inlineStrategy | ||
| ->NewContext(pParam->inlineInfo->inlineCandidateInfo->inlinersContext, pParam->inlineInfo->iciStmt, | ||
| pParam->inlineInfo->iciCall); | ||
| pParam->inlineInfo->argCnt = pParam->inlineCandidateInfo->methInfo.args.totalILArgs(); |
Member
Author
There was a problem hiding this comment.
Moved this into impInlineInitVars since I added some asserts that needed it, and this would otherwise be initialized after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use a single wellknownargs.h that is included multiple times to build various pieces of information we query about arguments. Rewrite users in terms of columns defined in that file.
Fix #130227