JIT: Add a type for call emission parameters #114542
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors call emission by replacing the old EmitCallType–based API with a new structure (EmitCallParams) to encapsulate call parameters. The change is applied across multiple architecture targets (xarch, riscv64, loongarch64, arm64, arm, etc.) and in codegen files to simplify and consolidate call emission logic.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/emit*.{h,cpp} | Removed legacy EmitCallType-based call emission functions and updated call emission signatures with EmitCallParams. |
| src/coreclr/jit/codegen*.cpp | Updated call site emission (including helper and indirect calls) to use the new EmitCallParams structure. |
| src/coreclr/jit/codegencommon.cpp | Introduced genEmitCallWithCurrentGC that extracts current GC info and forwards the EmitCallParams. |
| Other target-specific files | Similar refactoring and cleanup to ensure consistency across architectures. |
Comments suppressed due to low confidence (2)
src/coreclr/jit/emit.h:449
- The removal of the legacy EmitCallType-based API in favor of the new EmitCallParams structure is extensive; ensure that regression tests across all target architectures validate the new call emission behavior.
enum EmitCallType { ... }
src/coreclr/jit/codegencommon.cpp:1516
- Verify that all fields in the EmitCallParams structure are explicitly initialized at each call site before invoking genEmitCallWithCurrentGC to avoid unexpected defaults that might affect call emission behavior.
void CodeGen::genEmitCallWithCurrentGC(EmitCallParams& params)
|
cc @dotnet/jit-contrib PTAL @BruceForstall |
| enum EmitCallType | ||
| { | ||
| EC_FUNC_TOKEN, // Direct call to a helper/static/nonvirtual/global method (call addr with RIP-relative encoding) | ||
| EC_FUNC_TOKEN_INDIR, // Indirect call to a helper/static/nonvirtual/global method (call [addr]/call [rip+addr]) |
There was a problem hiding this comment.
Not all of these were defined for all platforms. e.g., arm64 only defined TOKEN and INDIR_R. Should we ifdef out the cases that aren't available on a platform to avoid mistakenly using them?
There was a problem hiding this comment.
Added xarch ifdefs for EC_FUNC_TOKEN_INDIR and EC_FUNC_TOKEN_ARD
In preparation of adding another parameter for async2.
This could be cleaned up even more, but that's for another day.