Skip to content

Commit

Permalink
LclVar which addresses are taken should be marked as doNotEnreg.
Browse files Browse the repository at this point in the history
Check that we don't have independently promoted LCL_VAR that are references after lowering.
Check that all LclVars that have ADDR() on top of them are marked as doNotEnreg.

In the past when we did not enregister structs we were allocating them on the stack even without doNotEnreg set.
  • Loading branch information
Sergey committed May 27, 2021
1 parent 4125826 commit ca4fb4d
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -3433,6 +3433,7 @@ class Compiler
DNER_NoRegVars, // opts.compFlags & CLFLG_REGVAR is not set
DNER_MinOptsGC, // It is a GC Ref and we are compiling MinOpts
#if !defined(TARGET_64BIT)
DNER_LongParamVar, // It is a long parameter.
DNER_LongParamField, // It is a decomposed field of a long parameter.
#endif
#ifdef JIT32_GCENCODER
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/decomposelongs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ GenTree* DecomposeLongs::DecomposeLclVar(LIR::Use& use)
}
else
{
m_compiler->lvaSetVarDoNotEnregister(varNum DEBUGARG(Compiler::DNER_LocalField));
loResult->SetOper(GT_LCL_FLD);
loResult->AsLclFld()->SetLclOffs(0);
loResult->AsLclFld()->SetFieldSeq(FieldSeqStore::NotAField());
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2635,6 +2635,9 @@ void Compiler::lvaSetVarDoNotEnregister(unsigned varNum DEBUGARG(DoNotEnregister
case DNER_LongParamField:
JITDUMP("it is a decomposed field of a long parameter\n");
break;
case DNER_LongParamVar:
JITDUMP("it is a long parameter\n");
break;
#endif
default:
unreached();
Expand Down
47 changes: 39 additions & 8 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,16 @@ GenTree* Lowering::LowerNode(GenTree* node)
node->gtGetOp1()->SetRegOptional();
break;

case GT_LCL_FLD_ADDR:
case GT_LCL_VAR_ADDR:
{
// TODO-Cleanup: this is definitely not the best place for this detection,
// but for now it is the easiest. Move it to morph.
const GenTreeLclVarCommon* lclAddr = node->AsLclVarCommon();
comp->lvaSetVarDoNotEnregister(lclAddr->GetLclNum() DEBUGARG(Compiler::DNER_BlockOp));
}
break;

default:
break;
}
Expand Down Expand Up @@ -3208,7 +3218,9 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)

if (convertToStoreObj)
{
GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(lclStore->GetLclNum(), TYP_BYREF);
const unsigned lclNum = lclStore->GetLclNum();
GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(lclNum, TYP_BYREF);
comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_BlockOp));

addr->gtFlags |= GTF_VAR_DEF;
assert(!addr->IsPartialLclFld(comp));
Expand Down Expand Up @@ -3584,7 +3596,8 @@ void Lowering::LowerStoreSingleRegCallStruct(GenTreeBlk* store)
GenTreeLclVar* Lowering::SpillStructCallResult(GenTreeCall* call) const
{
// TODO-1stClassStructs: we can support this in codegen for `GT_STORE_BLK` without new temps.
const unsigned spillNum = comp->lvaGrabTemp(true DEBUGARG("Return value temp for an odd struct return size"));
const unsigned spillNum = comp->lvaGrabTemp(true DEBUGARG("Return value temp for an odd struct return size"));
comp->lvaSetVarDoNotEnregister(spillNum DEBUGARG(Compiler::DNER_LocalField));
CORINFO_CLASS_HANDLE retClsHnd = call->gtRetClsHnd;
comp->lvaSetStruct(spillNum, retClsHnd, false);
GenTreeLclFld* spill = new (comp, GT_STORE_LCL_FLD) GenTreeLclFld(GT_STORE_LCL_FLD, call->gtType, spillNum, 0);
Expand Down Expand Up @@ -6011,20 +6024,26 @@ void Lowering::CheckNode(Compiler* compiler, GenTree* node)
case GT_HWINTRINSIC:
assert(node->TypeGet() != TYP_SIMD12);
break;
#ifdef TARGET_64BIT
#endif // FEATURE_SIMD

case GT_LCL_VAR:
case GT_STORE_LCL_VAR:
{
unsigned lclNum = node->AsLclVarCommon()->GetLclNum();
LclVarDsc* lclVar = &compiler->lvaTable[lclNum];
GenTreeLclVar* lclVar = node->AsLclVar();
const unsigned lclNum = lclVar->GetLclNum();
const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum);
#if defined(FEATURE_SIMD) && defined(TARGET_64BIT)
if (node->TypeIs(TYP_SIMD12))
{
assert(compiler->lvaIsFieldOfDependentlyPromotedStruct(lclVar) || (lclVar->lvSize() == 12));
assert(compiler->lvaIsFieldOfDependentlyPromotedStruct(varDsc) || (varDsc->lvSize() == 12));
}
#endif // FEATURE_SIMD && TARGET_64BIT
if (varDsc->lvPromoted)
{
assert(varDsc->lvDoNotEnregister || varDsc->lvIsMultiRegRet);
}
}
break;
#endif // TARGET_64BIT
#endif // SIMD

case GT_LCL_VAR_ADDR:
case GT_LCL_FLD_ADDR:
Expand All @@ -6043,6 +6062,8 @@ void Lowering::CheckNode(Compiler* compiler, GenTree* node)
assert(lclVarAddr->isContained() || !varDsc->lvTracked || varTypeIsStruct(varDsc));
// TODO: support this assert for uses, see https://github.com/dotnet/runtime/issues/51900.
}

assert(varDsc->lvDoNotEnregister);
break;
}

Expand All @@ -6051,6 +6072,16 @@ void Lowering::CheckNode(Compiler* compiler, GenTree* node)
assert(!"Should not see phi nodes after rationalize");
break;

case GT_LCL_FLD:
case GT_STORE_LCL_FLD:
{
GenTreeLclFld* lclFld = node->AsLclFld();
const unsigned lclNum = lclFld->GetLclNum();
const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum);
assert(varDsc->lvDoNotEnregister);
}
break;

default:
break;
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3959,6 +3959,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
{
// Can't use the existing field's type, so use GT_LCL_FLD to swizzle
// to a new type
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_LocalField));
argObj->ChangeOper(GT_LCL_FLD);
argObj->gtType = structBaseType;
}
Expand All @@ -3983,6 +3984,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
else if (genTypeSize(varDsc->TypeGet()) != genTypeSize(structBaseType))
{
// Not a promoted struct, so just swizzle the type by using GT_LCL_FLD
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_LocalField));
argObj->ChangeOper(GT_LCL_FLD);
argObj->gtType = structBaseType;
}
Expand Down
12 changes: 10 additions & 2 deletions src/coreclr/jit/rationalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ void Rationalizer::RewriteSIMDIndir(LIR::Use& use)
// replace the expression by GT_LCL_VAR or GT_LCL_FLD.
BlockRange().Remove(indir);

var_types lclType = comp->lvaGetDesc(addr->AsLclVar())->TypeGet();
const GenTreeLclVar* lclAddr = addr->AsLclVar();
const unsigned lclNum = lclAddr->GetLclNum();
LclVarDsc* varDsc = comp->lvaGetDesc(lclNum);

var_types lclType = varDsc->TypeGet();

if (lclType == simdType)
{
Expand All @@ -156,7 +160,11 @@ void Rationalizer::RewriteSIMDIndir(LIR::Use& use)
addr->gtFlags |= GTF_VAR_USEASG;
}

comp->lvaSetVarDoNotEnregister(addr->AsLclFld()->GetLclNum() DEBUGARG(Compiler::DNER_LocalField));
comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_LocalField));
}
if (varDsc->lvPromotedStruct())
{
comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_IsStructArg));
}

addr->gtType = simdType;
Expand Down

0 comments on commit ca4fb4d

Please sign in to comment.