Add MRV debug info for multi-register FP/mixed struct returns on Unix x64#129992
Open
tommcdon wants to merge 21 commits into
Open
Add MRV debug info for multi-register FP/mixed struct returns on Unix x64#129992tommcdon wants to merge 21 commits into
tommcdon wants to merge 21 commits into
Conversation
… x64 Adds managed-return-value (MRV) debug-info encoding for value-class returns that live in two registers where at least one is a floating-point register (e.g. a 16-byte struct returned in XMM0+XMM1, or a mixed ValueTuple<double,int> returned in XMM0+RAX on SysV x64). Previously these were silently skipped. - New VarLocType values VLT_REG_FP_REG_FP, VLT_REG_FP_REG, VLT_REG_REG_FP. - JIT producer (scopeinfo/codegencommon) emits the appropriate encoding via storeVariableInTwoRegisters instead of skipping non-integer register pairs. - Serializer (debuginfostore) encodes the two register indices. - DBI consumer reconstructs the value via a snapshot-based TwoRegisterValueHome (64-bit only). - CordbType::ReturnedByValue permits FP/generic fields only for two-register (>8 byte) returns, leaving single-register value classes unsupported. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CordbJITILFrame::GetNativeVariable returned CORDBG_E_IL_VAR_NOT_AVAILABLE for the VLT_FPSTK location kind, leaving the implementation commented out since the initial CoreCLR commit. On x86, floating-point values (including return values, surfaced via GetReturnValueForILOffset) live on the x87 FP stack and are reported as VLT_FPSTK, so inspecting a float/double return or local failed with an unavailable-variable error. Enable the existing GetLocalFloatingPointValue path for TARGET_X86, which already handles loading the x87 stack state and R4/R8 conversion. Behavior on non-x86 targets is unchanged (ARM remains E_NOTIMPL; others retain the CORDBG_E_IL_VAR_NOT_AVAILABLE fallback, where VLT_FPSTK is never produced). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends CoreCLR debug-info (ICorDebugInfo VarLocType / MRV encoding) to represent value-class locations spanning two registers where at least one register is a floating-point register, enabling managed-return-value inspection for Unix x64 multi-reg FP/mixed struct returns and adding x86 x87 FP-stack (VLT_FPSTK) support in CordbJITILFrame::GetNativeVariable.
Changes:
- Add new
VarLocTypeencodings (VLT_REG_FP_REG_FP,VLT_REG_FP_REG,VLT_REG_REG_FP) and thread them through varloc equality, debug-info encoding/decoding, and tooling (R2R/AOT readers/writers). - Update the JIT’s scope/return-value debug-info emission to use a new
storeVariableInTwoRegistershelper that can encode int+fp / fp+fp register pairs (not just int+int). - Add debugger-side decoding/building of values for the new encodings via a two-register snapshot home, plus implement the missing x86
VLT_FPSTKread path.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/util.cpp | Extend VarLoc equality to treat new two-register FP/mixed loc types like VLT_REG_REG. |
| src/coreclr/vm/debuginfostore.cpp | Encode the two register indices for new loc types using the existing vlRegReg layout. |
| src/coreclr/tools/r2rdump/Extensions.cs | Add display handling for new loc types (currently prints raw indices). |
| src/coreclr/tools/Common/JitInterface/CorInfoTypes.VarInfo.cs | Add new VarLocType enum values for tooling/shared interface parity. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/DebugInfoTypes.cs | Mirror new VarLocType values for R2R reflection reader. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/DebugInfo.cs | Parse new loc types as two uint payload fields. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DebugInfoTableNode.cs | Emit new loc types with two register fields in var blobs. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/CodeView/CodeViewSymbolsBuilder.cs | Recognize new loc types in CodeView symbol emission logic. |
| src/coreclr/jit/scopeinfo.cpp | Add storeVariableInTwoRegisters; emit mixed/fp two-reg locations for SysV AMD64 params; update equality/dumps/assert-sync. |
| src/coreclr/jit/ee_il_dll.cpp | Print new loc types correctly in JIT debug dumps by mapping fp indices back to fp regs. |
| src/coreclr/jit/codegeninterface.h | Add new loc type enum members and the new storeVariableInTwoRegisters API. |
| src/coreclr/jit/codegencommon.cpp | Emit MRV return location using storeVariableInTwoRegisters; keep 32-bit guardrails. |
| src/coreclr/inc/cordebuginfo.h | Add new VarLocType values and document the vlRegReg reuse contract for fp/mixed pairs. |
| src/coreclr/debug/ee/functioninfo.cpp | Add logging for the new loc types in native-var dumps. |
| src/coreclr/debug/ee/debugger.cpp | Treat new loc types as “enregistered” for valuetype size/classification decisions. |
| src/coreclr/debug/di/valuehome.cpp | Make reg-reg reads tolerant of struct sizes < 2 registers; add TwoRegisterValueHome::GetEnregisteredValue; relax offset assumption for multi-reg returns. |
| src/coreclr/debug/di/rstype.cpp | Expand CordbType::ReturnedByValue gating for 64-bit multi-reg return scenarios, including mixed and generic-parameter fields (two-reg only). |
| src/coreclr/debug/di/rsthread.cpp | Add GetLocalTwoRegisterValue; handle new loc types in GetNativeVariable; implement x86 VLT_FPSTK read path. |
| src/coreclr/debug/di/rspriv.h | Declare GetLocalTwoRegisterValue and introduce TwoRegisterValueHome. |
jakobbotsch
reviewed
Jun 29, 2026
Replace the three new VarLocType values (VLT_REG_FP_REG_FP, VLT_REG_FP_REG, VLT_REG_REG_FP) with a unified approach that extends the RegNum enum to include FP registers on AMD64 (XMM0-XMM15). This allows VLT_REG_REG to encode any combination of int and FP registers without new VarLocType variants, addressing scalability concerns for future multi-register scenarios (HFAs, multi-field enregistration). Changes: - Add REGNUM_XMM0-REGNUM_XMM15 to RegNum enum (AMD64 only) - Extend g_JITToCorDbgReg to map FP RegNum to CorDebugRegister - Add mapRegNumToDebugRegNum helper in JIT scopeinfo - Simplify storeVariableInRegisters to handle int+FP uniformly - Update DBI VLT_REG_REG path to detect FP registers and use GetLocalTwoRegisterValue for mixed/FP multi-reg values - Remove VLT_REG_FP_REG_FP/VLT_REG_FP_REG/VLT_REG_REG_FP from all consumers (DBI, EE, R2R tools, debuginfostore) x86 is unchanged (no FP registers in RegNum; uses VLT_FPSTK). Tested: x86 Windows, x64 Windows, x64 Linux, ARM64 Windows — all MRV diagnostic tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ireg-fp-debuginfo
Use ICorDebugInfo::RegNum parameter type and static_cast<unsigned> for the cross-enum comparison in the VLT_REG_REG debug display lambda. Fixes -Werror,-Wenum-compare on clang (Linux builds). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a3d69e0 to
1f4a621
Compare
1f4a621 to
d206fe1
Compare
d206fe1 to
cab5285
Compare
AmbientSpRegNum hardcoded ARM=25/ARM64=66, which assume the FP-register-extended RegNum enum (D0-D7/V0-V31). On this branch ARM/ARM64 RegNum contains no FP registers, so REGNUM_AMBIENT_SP is REGNUM_COUNT+1 = 17 (ARM) / 34 (ARM64), as asserted in debug/inc/DbgIPCEvents.h. With the wrong values OpStackLocation would not recognize AMBIENT_SP and would route it through DwarfRegNum, emitting an incorrect DWARF register for stack slots based on the ambient SP. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23dd0633-d7b5-4721-a2cc-87dc172f0915
…n-FP fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23dd0633-d7b5-4721-a2cc-87dc172f0915
…G var-info array - AmbientSpRegNum: add LoongArch64 and RISCV64 (REGNUM_AMBIENT_SP = 34 on both, per DbgIPCEvents.h). Without these, OpStackLocation would route an ambient-SP stack base through OpBReg and emit a bogus DWARF register instead of DW_OP_call_frame_cfa on those targets. - genSetScopeInfo: value-initialize genTrnslLocalVarInfo so slots not populated by genSetScopeInfoUsingVariableRanges (call-return entries, and entries skipped for VLT_INVALID) are predictably tlviAvailable=false rather than uninitialized (DEBUG-only JIT-dump correctness). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23dd0633-d7b5-4721-a2cc-87dc172f0915
DWARF composite location descriptions list pieces least-significant first (DWARF5 2.6.1.2). For VLT_REG_REG, loc.B is vlrrReg1 (low half) and loc.C is vlrrReg2 (high half), so emit loc.B before loc.C instead of the reverse. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23dd0633-d7b5-4721-a2cc-87dc172f0915
noahfalk
approved these changes
Jul 14, 2026
Comment on lines
1824
to
1833
| { | ||
| case ELEMENT_TYPE_R4: | ||
| case ELEMENT_TYPE_R8: | ||
| unsupported = true; | ||
| // Floating-point fields are returned in FP registers. | ||
| // A single FP register holding a value class is not | ||
| // encodable here (only primitive VLT_REG_FP is), so | ||
| // restrict to the two-register multi-reg forms. | ||
| if (!twoRegister) | ||
| unsupported = true; | ||
| break; |
Comment on lines
+1841
to
+1855
| case ELEMENT_TYPE_VAR: | ||
| case ELEMENT_TYPE_MVAR: | ||
| // The field's type is a generic type parameter (e.g. the | ||
| // Item1/Item2 fields of ValueTuple<T1, T2>); the unbound field | ||
| // signature does not carry the instantiated type, so we cannot | ||
| // tell whether it resolves to an FP type. Only permit it for the | ||
| // two-register multi-reg forms, where both the all-FP and mixed | ||
| // int/FP paths are implemented (and the read path fails gracefully | ||
| // when the value is not actually register-returned). This is | ||
| // required to support mixed int/fp returns such as | ||
| // ValueTuple<double, int>, while avoiding the | ||
| // unimplemented single-FP-register value-class path. | ||
| if (!twoRegister) | ||
| unsupported = true; | ||
| break; |
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.
Adds managed-return-value (MRV) debug-info encoding for value-class returns that live in two registers. Additionally, this change implements CordbJITILFrame::GetNativeVariable for x86 x87 FP-stack return/local variables (missed scenario from #129321)
Fixes #129344