Skip to content

Commit 4537ce8

Browse files
authored
JIT: fix double reporting of some GC frame slots (#71245)
If there is a gc struct local that is dependently promoted, the struct local may be untracked while the promoted gc fields of the struct are tracked. If so, the jit will double report the stack offset for the gc field, first as an untracked slot, and then as a tracked slot. Detect this case and report the slot as tracked only. Closes #71005.
1 parent 25e369f commit 4537ce8

File tree

3 files changed

+33
-11
lines changed

3 files changed

+33
-11
lines changed

src/coreclr/jit/compiler.hpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4005,24 +4005,17 @@ inline bool Compiler::lvaIsFieldOfDependentlyPromotedStruct(const LclVarDsc* var
40054005
// This is because struct variables are never tracked as a whole for GC purposes.
40064006
// It is up to the caller to ensure that the fields of struct variables are
40074007
// correctly tracked.
4008-
// On Amd64, we never GC-track fields of dependently promoted structs, even
4008+
//
4009+
// We never GC-track fields of dependently promoted structs, even
40094010
// though they may be tracked for optimization purposes.
4010-
// It seems that on x86 and arm, we simply don't track these
4011-
// fields, though I have not verified that. I attempted to make these GC-tracked,
4012-
// but there was too much logic that depends on these being untracked, so changing
4013-
// this would require non-trivial effort.
4014-
4011+
//
40154012
inline bool Compiler::lvaIsGCTracked(const LclVarDsc* varDsc)
40164013
{
40174014
if (varDsc->lvTracked && (varDsc->lvType == TYP_REF || varDsc->lvType == TYP_BYREF))
40184015
{
40194016
// Stack parameters are always untracked w.r.t. GC reportings
40204017
const bool isStackParam = varDsc->lvIsParam && !varDsc->lvIsRegArg;
4021-
#ifdef TARGET_AMD64
40224018
return !isStackParam && !lvaIsFieldOfDependentlyPromotedStruct(varDsc);
4023-
#else // !TARGET_AMD64
4024-
return !isStackParam;
4025-
#endif // !TARGET_AMD64
40264019
}
40274020
else
40284021
{

src/coreclr/jit/emit.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,11 @@ class emitter
23682368
void emitSetFrameRangeLcls(int offsLo, int offsHi);
23692369
void emitSetFrameRangeArgs(int offsLo, int offsHi);
23702370

2371+
bool emitIsWithinFrameRangeGCRs(int offs)
2372+
{
2373+
return (offs >= emitGCrFrameOffsMin) && (offs < emitGCrFrameOffsMax);
2374+
}
2375+
23712376
static instruction emitJumpKindToIns(emitJumpKind jumpKind);
23722377
static emitJumpKind emitInsToJumpKind(instruction ins);
23732378
static emitJumpKind emitReverseJumpKind(emitJumpKind jumpKind);

src/coreclr/jit/gcencode.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4226,7 +4226,31 @@ void GCInfo::gcMakeRegPtrTable(
42264226
continue;
42274227
}
42284228

4229-
int offset = varDsc->GetStackOffset() + i * TARGET_POINTER_SIZE;
4229+
unsigned const fieldOffset = i * TARGET_POINTER_SIZE;
4230+
int const offset = varDsc->GetStackOffset() + fieldOffset;
4231+
4232+
#ifdef DEBUG
4233+
if (varDsc->lvPromoted)
4234+
{
4235+
assert(compiler->lvaGetPromotionType(varDsc) == Compiler::PROMOTION_TYPE_DEPENDENT);
4236+
4237+
// A dependently promoted tracked gc local can end up in the gc tracked
4238+
// frame range. If so it should be excluded from tracking via lvaIsGCTracked.
4239+
//
4240+
unsigned const fieldLclNum = compiler->lvaGetFieldLocal(varDsc, fieldOffset);
4241+
assert(fieldLclNum != BAD_VAR_NUM);
4242+
LclVarDsc* const fieldVarDsc = compiler->lvaGetDesc(fieldLclNum);
4243+
4244+
if (compiler->GetEmitter()->emitIsWithinFrameRangeGCRs(offset))
4245+
{
4246+
assert(!compiler->lvaIsGCTracked(fieldVarDsc));
4247+
JITDUMP("Untracked GC struct slot V%02u+%u (P-DEP promoted V%02u) is at frame offset %d within "
4248+
"tracked ref range; will report slot as untracked\n",
4249+
varNum, fieldOffset, fieldLclNum, offset);
4250+
}
4251+
}
4252+
#endif
4253+
42304254
#if DOUBLE_ALIGN
42314255
// For genDoubleAlign(), locals are addressed relative to ESP and
42324256
// arguments are addressed relative to EBP.

0 commit comments

Comments
 (0)