Add ARM64 FP register debug info support - #130980
Conversation
Extend the ARM64 debug RegNum enumeration with V0-V31 and use the unified register encoding for single- and two-register FP locations. Update debugger decoding, JIT diagnostics, NativeAOT DWARF ambient-SP handling, and the JIT-EE version identifier. Only emit two-register FP homes when each register maps to an 8-byte piece; narrower HFAs and wider HVAs require piece-size metadata that the current representation cannot encode. Bump the breaking ReadyToRun format version so runtimes do not decode older ARM64 variable locations using the new RegNum meanings. Teach cDAC to read V0-V31 and only apply its SP fallback to the ambient-SP pseudo-register. Complete the ARM64 CodeView and R2R dump register mappings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8afd92ce-0c28-4b64-b7b1-ee63f18e53ef
|
Azure Pipelines: Successfully started running 6 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR extends ARM64 debug-info register support to include SIMD/FP V0–V31 and wires that through the JIT debug-info encoding and multiple consumers (debugger DBI, cDAC, DWARF/CodeView emitters, and R2R tooling). It also bumps the ReadyToRun major/minimum version to reflect the new ARM64 register encoding.
Changes:
- Extend
ICorDebugInfo::RegNumon ARM64 to includeV0–V31, and update ARM64 register-to-debugger mappings (includingg_JITToCorDbgRegand DBI decoding paths). - Enable ARM64 single-register FP variable inspection and FP-containing two-register return-value inspection for the “8-byte piece” subset.
- Bump ReadyToRun major/minimum version from 25 → 26 and update associated tools/constants; add/adjust cDAC unit tests for ARM64 vector register behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/UnitTests/PlatformContextTests.cs | Adds ARM64 V-register round-trip coverage and a targeted test verifying the “low 64 bits only” behavior. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs | Restricts ambient-SP fallback behavior to REGNUM_AMBIENT_SP specifically; adds architecture-specific ambient-SP numbering (incl. ARM64=66). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64Context.cs | Implements numeric accessors for ARM64 V0–V31 as low-64-bit reads/writes via the V fixed buffer. |
| src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs | Bumps ReadyToRun major version constant to 26 in managed tooling. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Arm64/Registers.cs | Extends Arm64 register enum to include SP, PC, and V0–V31 for better R2R debug-info display. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/Dwarf/DwarfExpressionBuilder.cs | Updates ARM64 ambient-SP pseudo-reg number to match the expanded RegNum space (66). |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/CodeView/CodeViewSymbolsBuilder.cs | Adds ARM64 PC + Q0–Q31 mapping for CodeView register encoding of ARM64 RegNum values. |
| src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h | Bumps ReadyToRun major version constant to 26 in NativeAOT header. |
| src/coreclr/jit/scopeinfo.cpp | Updates RegNum mapping/encoding to allow ARM64 V-registers in debug-info FP and REG_REG encodings. |
| src/coreclr/jit/ee_il_dll.cpp | Updates debug dump logic to correctly display ARM64 FP regs stored as debug RegNum values. |
| src/coreclr/jit/codegencommon.cpp | Enables ARM64 FP-containing two-register return-value encoding for the 8-byte-piece subset; suppresses unsupported offsets. |
| src/coreclr/inc/readytorun.h | Bumps ReadyToRun major/minimum version to 26 and documents the format change. |
| src/coreclr/inc/jiteeversionguid.h | Updates the JIT/EE interface version GUID. |
| src/coreclr/inc/cordebuginfo.h | Extends ARM64 ICorDebugInfo::RegNum with REGNUM_FP_FIRST and REGNUM_V0–REGNUM_V31. |
| src/coreclr/debug/inc/dbgipcevents.h | Updates ARM64 DBG_TARGET_REGNUM_AMBIENT_SP to 66 to match the new RegNum layout. |
| src/coreclr/debug/inc/arm64/primitives.h | Extends g_JITToCorDbgReg to include ARM64 V0–V31 mappings. |
| src/coreclr/debug/di/rsthread.cpp | Uses the unified RegNum → CorDebugRegister path for ARM64 VLT_REG_FP and enables ARM64 mixed/FP REG_REG decoding via GetLocalTwoRegisterValue. |
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "8fa997bf0d26b10c94c1612da1f3b6541434549d",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "524517007ae75b915983fa072e80b9d63089d16d",
"last_reviewed_commit": "8fa997bf0d26b10c94c1612da1f3b6541434549d",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "524517007ae75b915983fa072e80b9d63089d16d",
"last_recorded_worker_run_id": "29687154318",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "8fa997bf0d26b10c94c1612da1f3b6541434549d",
"review_id": 4730777508
}
]
} |
![github-actions[bot]](https://cdn.github.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Holistic Review
Motivation: Justified. This follows up #129992 to enable debugger inspection of ARM64 floating-point/SIMD return values (single-register FP and two-register HFA/HVA where each piece is 8 bytes), a real gap in managed-debugging support. The scope limitation (8-byte pieces only, >2 registers deferred to #130979) is explicitly acknowledged.
Approach: Sound and consistent with the existing AMD64 design. It extends the ARM64 ICorDebugInfo::RegNum enum with REGNUM_FP_FIRST/V0..V31 and reuses the AMD64 code paths (JIT siVarLoc encoding, DBI decode, cDAC context, DWARF/CodeView/R2R dumps) by widening #ifdef TARGET_AMD64 guards to also cover TARGET_ARM64. The unrepresentable-piece cases are conservatively suppressed rather than mis-encoded.
Summary: readytorun.h, ModuleHeaders.h, and ModuleHeaders.cs, the JIT/EE GUID is rotated, and the AMBIENT_SP value (66) is updated in lockstep in dbgipcevents.h, DwarfExpressionBuilder, and ClrDataFrame. Because this is a cross-component debugger/JIT-EE contract change that alters the on-disk R2R format and cannot be validated in this build-less worker, a human should confirm the debugger read paths and R2R compatibility via CI (libraries-pgo/R2R and debugger test legs).
Detailed Findings
✅ RegNum ordering and register mapping — Verified consistent
The new ARM64 RegNum block places REGNUM_FP_FIRST (=33) immediately after REGNUM_PC (=32), and g_JITToCorDbgReg in debug/inc/arm64/primitives.h is extended in the same order with REGISTER_ARM64_V0..V31, keeping ConvertRegNumToCorDebugRegister (index into g_JITToCorDbgReg) valid for the widened range. The JIT-side mapRegNumToDebugRegNum maps V0..V31 to REGNUM_FP_FIRST + (reg - REG_FP_FIRST), and the DBI decode subtracts REGNUM_FP_FIRST back to a 0-based FP index for GetLocalFloatingPointValue/GetLocalTwoRegisterValue—symmetric and matching the AMD64 pattern.
✅ R2R breaking-version handling — Correct
Both READYTORUN_MAJOR_VERSION and MINIMUM_READYTORUN_MAJOR_VERSION are bumped to 26 (a hard break, appropriate since the ARM64 NativeVarInfo register encoding changed), the version history comment is added, and the NativeAOT/ModuleHeaders.cs CurrentMajorVersion constants are updated to match. This satisfies the git grep MINIMUM_READYTORUN_MAJOR_VERSION guidance in the header.
✅ Two-register piece-size guard — Reasonable and correctly scoped
The new ARM64 block in codegencommon.cpp suppresses MRV info when an FP-containing pair has GetReturnFieldOffset(1) != TARGET_POINTER_SIZE (i.e., the two-register encoding's implicit 8-byte second-register offset would be wrong for 4-byte or 16-byte pieces). The guard only triggers when at least one register is FP, leaving pure integer pairs encoded as before, which matches the stated scope and #130979 deferral.
✅ cDAC ReadRegister change — Behavioral improvement, not a regression
The rewritten ClrDataFrame.ReadRegister now only falls back to SP when registerNumber equals the architecture-specific REGNUM_AMBIENT_SP value, instead of the previous behavior of falling back for any unreadable register number. With V registers (33–64) now readable, the old blanket fallback would have masked genuinely out-of-range numbers by returning SP; the new form is more precise. The per-arch AMBIENT_SP values match the native enum (Arm64 => 66, X64 => 33, etc.).
✅ Test coverage — Adequate for the cDAC surface
PlatformContextTests adds round-trip cases for V0 (33) and V31 (64), updates the out-of-range cases to 65/66, and adds ARM64_VectorRegisterNumber_UsesLow64Bits asserting the low-64-bit slot (index*2) is written without disturbing neighbors. This directly exercises the new SetVectorRegister/ReadVectorRegister stride logic. Note the JIT encoding and DBI decode paths themselves are not unit-testable here and rely on CI debugger legs.
💡 CodeView Q-register mapping — Confirm the 128-bit alias is intended
In CodeViewSymbolsBuilder.GetCVRegNum, ARM64 FP regs map regNum-33 onto CV_ARM64_Q0 (the 128-bit Q registers). Since the JIT only encodes 8-byte (D-register) pieces today, emitting the 128-bit Q alias is a superset and should be fine for debuggers, but a human familiar with the CodeView consumer may want to confirm D-vs-Q register selection is acceptable for the double-precision return values this PR targets.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 293.2 AIC · ⌖ 10.4 AIC · ⊞ 10K
Address review feedback: nothing sets ARM64 vector registers through the numeric register accessor (SIMD unwind paths write the V buffer directly), so drop vector-write support from TrySetRegister and remove the now-unused SetVectorRegister helper. Vector reads remain for debugger inspection. Also simplify ReadVectorRegister to index the fixed-size V buffer directly instead of pinning it, matching the existing ARM64Unwinder access pattern. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8afd92ce-0c28-4b64-b7b1-ee63f18e53ef
|
/ba-g failures are unrelated |
Follow-up to dotnet#129992 adding debugger support for ARM64 FP and two-register HFA/HVA return values: - Extends ARM64 `RegNum` with `V0–V31`. - Enables single-register FP inspection and two-register HFA/HVA return-value inspection when each register represents an 8-byte piece. - Updates DBI, cDAC, DWARF, CodeView, and R2R dump register mappings. - Bumps the breaking ReadyToRun major/minimum version to 26. - Support for 4-byte and 16-byte register pieces and values requiring more than two registers is tracked as part of dotnet#130979. --------- Copilot-Session: 8afd92ce-0c28-4b64-b7b1-ee63f18e53ef
Follow-up to #129992 adding debugger support for ARM64 FP and two-register HFA/HVA return values:
RegNumwithV0–V31.