[APX] Add Jmpabs support. - #131826
[APX] Add Jmpabs support.#131826DeepakRajendrakumaran wants to merge 18 commits into
Conversation
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds initial AMD64 APX JMPABS support in CoreCLR, wiring it into VM virtual-call stub emission and adding emitter support/tests in RyuJIT.
Changes:
- Extend AMD64 virtual call short dispatch stubs to support an APX encoding path (
jne rel32; nop; jmpabs imm64) and use it when APX is available. - Add VM-side capability detection (
IsJmpAbsAvailable) plus a rawemitJmpAbsJumphelper, and routeemitBackToBackJumpthroughJMPABSwhen available. - Introduce
INS_jmpabsin the JIT instruction set and add emitter unit-test coverage for encoding.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/amd64/virtualcallstubcpu.hpp | Adds APX/legacy union encoding for DispatchStubShort, updates fail/impl target accessors, and adjusts short-stub reachability logic. |
| src/coreclr/vm/amd64/cgencpu.h | Replaces removed thunk helper with IsJmpAbsAvailable declaration and adds emitJmpAbsJump API. |
| src/coreclr/vm/amd64/cgenamd64.cpp | Implements APX availability detection and emits JMPABS for back-to-back jump stubs (with legacy fallback). |
| src/coreclr/jit/instrsxarch.h | Adds INS_jmpabs as an APX instruction. |
| src/coreclr/jit/emitxarch.cpp | Teaches the emitter to size/output INS_jmpabs (REX2 + opcode + imm64). |
| src/coreclr/jit/codegenxarch.cpp | Adds an emitter unit-test emission site for INS_jmpabs. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/coreclr/jit/emitxarch.cpp:17260
- The new
INS_jmpabssupport in this function conflicts with the existing immed/reloc assumptions immediately below:noway_assert(size < EA_8BYTE || ((int)val == val && !id->idIsCnsReloc()))and theidIsCnsRelocblock currently only allow 32-bit immediates and relocations forpush. As a result, validjmpabsuses (64-bit targets and/or relocations) will hit debug asserts. Please special-caseINS_jmpabsin those checks.
emitAttr size = id->idOpSize();
ssize_t val = emitGetInsSC(id);
bool valInByte = ((signed char)val == (target_ssize_t)val);
// We would to update GC info correctly
assert(!IsSSEInstruction(ins));
src/coreclr/vm/amd64/virtualcallstubcpu.hpp:129
DispatchHolder/LookupHoldernow callIsJmpAbsAvailable()andemitJmpAbsJump(), but this header doesn't includecgencpu.hor otherwise declare these functions. That makes the header fragile and can break compilation depending on include order (andvirtualcallstub.hincludes this directly). Add forward declarations here (or includecgencpu.h) so the header is self-contained.
/*DispatchStubShort*********************************************************************************
This is the logical continuation of DispatchStub for the case when the failure target is within
a rel32 jump (DISPL). Uses a union to handle both legacy and APX encodings. */
struct DispatchStubShort
{
cfece2d to
55729b7
Compare
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
55729b7 to
a06a07a
Compare
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
a06a07a to
b96f625
Compare
| LIMITED_METHOD_CONTRACT; | ||
| // JMPABS encoding starts with 0x0F 0x85 (jne near) | ||
| // Legacy encoding starts with 0x48 0xB8 (mov rax, imm64) | ||
| return _bytes[0] == 0x0F && _bytes[1] == 0x85; | ||
| } |
| 0xA1 == pbCode[2] && | ||
| 0x90 == pbCode[11]) |
There was a problem hiding this comment.
| 0xA1 == pbCode[2] && | |
| 0x90 == pbCode[11]) | |
| 0xA1 == pbCode[2]) |
No need to check for the padding nop
| LIMITED_METHOD_CONTRACT; | ||
| PTR_BYTE pbCode = PTR_BYTE(pCode); | ||
|
|
||
| // Check for JMPABS encoding (APX): D5 00 A1 [8 bytes] 90 |
There was a problem hiding this comment.
| // Check for JMPABS encoding (APX): D5 00 A1 [8 bytes] 90 | |
| // Check for JMPABS encoding (APX) |
No need to duplicate the bytes in the comment
| return true; | ||
| } | ||
|
|
||
| // Check for legacy encoding: 48 B8 [8 bytes] FF E0 |
There was a problem hiding this comment.
| // Check for legacy encoding: 48 B8 [8 bytes] FF E0 | |
| // Check for legacy encoding: mov rax, imm64; jmp rax |
| DEFINE_DACVAR(BOOL, CodeVersionManager__s_HasNonDefaultILVersions, CodeVersionManager::s_HasNonDefaultILVersions) | ||
| #endif // FEATURE_CODE_VERSIONING | ||
| #ifdef TARGET_AMD64 | ||
| // Selects the encoding used by the AMD64 runtime-generated stubs (APX JMPABS vs. mov rax/jmp rax). |
There was a problem hiding this comment.
| // Selects the encoding used by the AMD64 runtime-generated stubs (APX JMPABS vs. mov rax/jmp rax). | |
| // Selects the encoding used by the AMD64 runtime-generated stubs (APX jmpabs vs. mov rax/jmp rax). |
Use consistent casing for asm instructions in comments (either all upper case or all lower case - I do not have a preference). Comments that combine different casing on a single line or within single method do not look good.
(fix all places in the PR)
2f4e34d to
1282d5f
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (4)
This emits a newD5 00 A1absolute branch, but the AMD64 debugger patch-skip decoder still… Add performance measurements for APX and legacy stub layouts · New The APX-specific short/long layouts introduced here are only selected whenIsJmpAbsAvailable()is…isJmpAbsEncoding()reads_bytes(a different union member) to detect the encoding. Reading from…
| // Padding aligns _implTarget as in DispatchStubShort. The APX form needs no mov rax and uses a | ||
| // second jmpabs for the fail path, so both arms are padded to 35 bytes: the stub grows 40 -> 48, | ||
| // which is free because these are allocated with CODE_SIZE_ALIGN (16) and 40 already took 48. |
| shortStubRW->_failDispl = (DISPL) displ; | ||
| shortStubRW->_implTarget = (size_t) implTarget; | ||
| CONSISTENCY_CHECK((PCODE)&shortStubRX->_failDispl + sizeof(DISPL) + shortStubRX->_failDispl == failTarget); | ||
| #if defined(TARGET_AMD64) |
There was a problem hiding this comment.
This is Amd64 specific file. It does not need TARGET_AMD64 ifdef
| longStub->_failTarget = failTarget; | ||
| *longStubRW = dispatchLongInit; | ||
|
|
||
| #if defined(TARGET_AMD64) |
1282d5f to
fc06e11
Compare


Add JMPABS (APX) Support for Virtual Call Stubs on AMD64
Summary
This PR adds support for the JMPABS instruction (part of Intel's APX - Advanced Performance Extensions) to virtual call stubs on AMD64, reducing stub size and improving instruction fetch efficiency.
Background
APX introduces the JMPABS instruction (
D5 00 A1+ 8-byte address), which performs a 64-bit absolute jump. JMPABS provides performance benefits over the traditionalmov rax, imm64; jmp raxsequence:Virtual call stubs are small code sequences generated at runtime for dispatching interface and virtual method calls. Currently, these stubs use a two-instruction sequence (
mov rax, target; jmp rax) to perform 64-bit absolute jumps.Changes
Virtual Call Stub Infrastructure
Stub Generation Helpers (src/coreclr/vm/amd64/cgenamd64.cpp, cgencpu.h)
emitJmpAbsJump()helper function to emit raw JMPABS instructions (11 bytes)emitBackToBackJump()to use JMPABS encoding when available, with fallback to legacy encodingIsJmpAbsAvailable()CPU capability detection functionEncodeLoadAndJumpThunk()(obsolete thunk generation helper)DispatchStubShort_offsetof_failDisplBasemacro referencing wrong struct type (DispatchStubLong→DispatchStubShort)Testing
src/testswith and without APX to validate stub changes on non-APX hardwareIssue
Fixes #131817
JIT Emitter
src/coreclr/jit/emitxarch.cpp
Edit - the emitter changes have been reveretd based on feedback from @jkotas
Opens
1. Static Stub Size with NOP Padding
Current approach: To maintain consistent stub size and simplify stub management, this PR uses NOP padding in the APX encoding path:
emitBackToBackJump (used in jump stubs): Both encodings are 12 bytes
Legacy:
mov rax, imm64; jmp rax(12 bytes)APX:
jmpabs imm64 (11 bytes) + nop (1 byte)= 12 bytesDispatchStubShort(used in dispatch stubs): Both encodings are 18 bytesLegacy:
jne rel32 (6 bytes) + mov rax, imm64; jmp rax (12 bytes)= 18 bytesAPX:
jne rel32 (6 bytes) + nop (1 byte) + jmpabs imm64 (11 bytes)= 18 bytes2. APX Availability Detection
JMPABS availability is determined at runtime via
IsJmpAbsAvailable(), which checksHasInstructionSet(InstructionSet_APX)from the JIT's CPU capability flags. Is runtime-only detection sufficient, or do we need additional compile-time or toolchain checks for crossgen/ReadyToRun scenarios?3. DispatchStubLong JMPABS Support
DispatchStubLongcould benefit from JMPABS encoding, but the instruction's 3-byte prefix shifts the 64-bit immediate from offset +2 (legacymov rax, imm64) to offset +3 (JMPABS). This offset change may break the 8-byte alignment requirement.Should we defer this optimization?