Skip to content

Commit

Permalink
Make more extensive use of lvaGetDesc() (#61494)
Browse files Browse the repository at this point in the history 
Including the version with a `GenTreeLclVarCommon*` overload.

I mostly replaced `&lvaTable[varNum]` and `lvaTable + varNum`
expressions, leaving `lvaTable[varNum].xxx`.

Made many resulting `varDsc*` const.

Removed unused `lvaRefCount`.

Simplifies code, and centralizes assert checking.

Added new `lvaGetLclNum(LclVarDsc*)` function to map back to a varNum.

This deletes many `noway_assert` in favor of the lvaGetDesc `assert`;
I'm not worried about removing asserts from the Release build.
  • Loading branch information
@BruceForstall
BruceForstall committed Nov 24, 2021
1 parent ef5ddf5 commit 2c55431
Show file tree
Hide file tree
Showing 45 changed files with 364 additions and 491 deletions.
34 changes: 13 additions & 21 deletions src/coreclr/jit/assertionprop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,12 +660,11 @@ void Compiler::optAddCopies()
Statement* stmt;
unsigned copyLclNum = lvaGrabTemp(false DEBUGARG("optAddCopies"));

// Because lvaGrabTemp may have reallocated the lvaTable, ensure varDsc
// is still in sync with lvaTable[lclNum];
varDsc = &lvaTable[lclNum];
// Because lvaGrabTemp may have reallocated the lvaTable, ensure varDsc is still in sync.
varDsc = lvaGetDesc(lclNum);

// Set lvType on the new Temp Lcl Var
lvaTable[copyLclNum].lvType = typ;
lvaGetDesc(copyLclNum)->lvType = typ;

#ifdef DEBUG
if (verbose)
Expand Down Expand Up @@ -1103,9 +1102,7 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
else
{
unsigned lclNum = curAssertion->op1.lcl.lclNum;
assert(lclNum < lvaCount);
LclVarDsc* varDsc = lvaTable + lclNum;
op1Type = varDsc->lvType;
op1Type = lvaGetDesc(lclNum)->lvType;
}

if (op1Type == TYP_REF)
Expand Down Expand Up @@ -1315,9 +1312,8 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
goto DONE_ASSERTION; // Don't make an assertion
}

unsigned lclNum = op1->AsLclVarCommon()->GetLclNum();
noway_assert(lclNum < lvaCount);
LclVarDsc* lclVar = &lvaTable[lclNum];
unsigned lclNum = op1->AsLclVarCommon()->GetLclNum();
LclVarDsc* lclVar = lvaGetDesc(lclNum);

ValueNum vn;

Expand Down Expand Up @@ -1394,9 +1390,8 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
//
else if (op1->gtOper == GT_LCL_VAR)
{
unsigned lclNum = op1->AsLclVarCommon()->GetLclNum();
noway_assert(lclNum < lvaCount);
LclVarDsc* lclVar = &lvaTable[lclNum];
unsigned lclNum = op1->AsLclVarCommon()->GetLclNum();
LclVarDsc* lclVar = lvaGetDesc(lclNum);

// If the local variable has its address exposed then bail
if (lclVar->IsAddressExposed())
Expand Down Expand Up @@ -1559,9 +1554,8 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
goto DONE_ASSERTION; // Don't make an assertion
}

unsigned lclNum2 = op2->AsLclVarCommon()->GetLclNum();
noway_assert(lclNum2 < lvaCount);
LclVarDsc* lclVar2 = &lvaTable[lclNum2];
unsigned lclNum2 = op2->AsLclVarCommon()->GetLclNum();
LclVarDsc* lclVar2 = lvaGetDesc(lclNum2);

// If the two locals are the same then bail
if (lclNum == lclNum2)
Expand Down Expand Up @@ -1632,7 +1626,6 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
if (op1->gtOper == GT_LCL_VAR)
{
unsigned lclNum = op1->AsLclVarCommon()->GetLclNum();
noway_assert(lclNum < lvaCount);

// If the local variable is not in SSA then bail
if (!lvaInSsa(lclNum))
Expand All @@ -1656,7 +1649,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
assert((assertion.op1.lcl.ssaNum == SsaConfig::RESERVED_SSA_NUM) ||
(assertion.op1.vn ==
vnStore->VNConservativeNormalValue(
lvaTable[lclNum].GetPerSsaData(assertion.op1.lcl.ssaNum)->m_vnPair)));
lvaGetDesc(lclNum)->GetPerSsaData(assertion.op1.lcl.ssaNum)->m_vnPair)));

ssize_t cnsValue = 0;
GenTreeFlags iconFlags = GTF_EMPTY;
Expand Down Expand Up @@ -1971,9 +1964,8 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion)
case O1K_LCLVAR:
case O1K_EXACT_TYPE:
case O1K_SUBTYPE:
assert(assertion->op1.lcl.lclNum < lvaCount);
assert(optLocalAssertionProp ||
lvaTable[assertion->op1.lcl.lclNum].lvPerSsaData.IsValidSsaNum(assertion->op1.lcl.ssaNum));
lvaGetDesc(assertion->op1.lcl.lclNum)->lvPerSsaData.IsValidSsaNum(assertion->op1.lcl.ssaNum));
break;
case O1K_ARR_BND:
// It would be good to check that bnd.vnIdx and bnd.vnLen are valid value numbers.
Expand Down Expand Up @@ -2006,7 +1998,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion)
assert(assertion->op2.u1.iconFlags != GTF_EMPTY);
break;
case O1K_LCLVAR:
assert((lvaTable[assertion->op1.lcl.lclNum].lvType != TYP_REF) ||
assert((lvaGetDesc(assertion->op1.lcl.lclNum)->lvType != TYP_REF) ||
(assertion->op2.u1.iconVal == 0) || doesMethodHaveFrozenString());
break;
case O1K_VALUE_NUMBER:
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1302,8 +1302,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
{
return false;
}
const LclVarDsc* varDsc = &compiler->lvaTable[tree->AsLclVarCommon()->GetLclNum()];
return (varDsc->lvIsRegCandidate());
return compiler->lvaGetDesc(tree->AsLclVarCommon())->lvIsRegCandidate();
}

#ifdef FEATURE_PUT_STRUCT_ARG_STK
Expand Down
10 changes: 4 additions & 6 deletions src/coreclr/jit/codegenarm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ void CodeGen::genCodeForLclVar(GenTreeLclVar* tree)
// lcl_vars are not defs
assert((tree->gtFlags & GTF_VAR_DEF) == 0);

bool isRegCandidate = compiler->lvaTable[tree->GetLclNum()].lvIsRegCandidate();
bool isRegCandidate = compiler->lvaGetDesc(tree)->lvIsRegCandidate();

// If this is a register candidate that has been spilled, genConsumeReg() will
// reload it at the point of use. Otherwise, if it's not in a register, we load it here.
Expand Down Expand Up @@ -1001,9 +1001,8 @@ void CodeGen::genCodeForStoreLclFld(GenTreeLclFld* tree)
// We must have a stack store with GT_STORE_LCL_FLD
noway_assert(targetReg == REG_NA);

unsigned varNum = tree->GetLclNum();
assert(varNum < compiler->lvaCount);
LclVarDsc* varDsc = &(compiler->lvaTable[varNum]);
unsigned varNum = tree->GetLclNum();
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);

// Ensure that lclVar nodes are typed correctly.
assert(!varDsc->lvNormalizeOnStore() || targetType == genActualType(varDsc->TypeGet()));
Expand Down Expand Up @@ -1083,8 +1082,7 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* tree)
}
if (regCount == 1)
{
unsigned varNum = tree->GetLclNum();
assert(varNum < compiler->lvaCount);
unsigned varNum = tree->GetLclNum();
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);
var_types targetType = varDsc->GetRegisterType(tree);

Expand Down
21 changes: 9 additions & 12 deletions src/coreclr/jit/codegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1871,8 +1871,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* treeNode)
void CodeGen::genCodeForLclVar(GenTreeLclVar* tree)
{

unsigned varNum = tree->GetLclNum();
assert(varNum < compiler->lvaCount);
unsigned varNum = tree->GetLclNum();
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);
var_types targetType = varDsc->GetRegisterType(tree);

Expand Down Expand Up @@ -1926,9 +1925,8 @@ void CodeGen::genCodeForStoreLclFld(GenTreeLclFld* tree)
// We must have a stack store with GT_STORE_LCL_FLD
noway_assert(targetReg == REG_NA);

unsigned varNum = tree->GetLclNum();
assert(varNum < compiler->lvaCount);
LclVarDsc* varDsc = &(compiler->lvaTable[varNum]);
unsigned varNum = tree->GetLclNum();
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);

// Ensure that lclVar nodes are typed correctly.
assert(!varDsc->lvNormalizeOnStore() || targetType == genActualType(varDsc->TypeGet()));
Expand Down Expand Up @@ -2120,15 +2118,14 @@ void CodeGen::genSimpleReturn(GenTree* treeNode)
if (op1->OperGet() == GT_LCL_VAR)
{
GenTreeLclVarCommon* lcl = op1->AsLclVarCommon();
bool isRegCandidate = compiler->lvaTable[lcl->GetLclNum()].lvIsRegCandidate();
const LclVarDsc* varDsc = compiler->lvaGetDesc(lcl);
bool isRegCandidate = varDsc->lvIsRegCandidate();
if (isRegCandidate && ((op1->gtFlags & GTF_SPILLED) == 0))
{
// We may need to generate a zero-extending mov instruction to load the value from this GT_LCL_VAR

unsigned lclNum = lcl->GetLclNum();
LclVarDsc* varDsc = &(compiler->lvaTable[lclNum]);
var_types op1Type = genActualType(op1->TypeGet());
var_types lclType = genActualType(varDsc->TypeGet());
var_types op1Type = genActualType(op1->TypeGet());
var_types lclType = genActualType(varDsc->TypeGet());

if (genTypeSize(op1Type) < genTypeSize(lclType))
{
Expand Down Expand Up @@ -3310,10 +3307,10 @@ void CodeGen::genCodeForSwap(GenTreeOp* tree)
assert(genIsRegCandidateLocal(tree->gtOp1) && genIsRegCandidateLocal(tree->gtOp2));

GenTreeLclVarCommon* lcl1 = tree->gtOp1->AsLclVarCommon();
LclVarDsc* varDsc1 = &(compiler->lvaTable[lcl1->GetLclNum()]);
LclVarDsc* varDsc1 = compiler->lvaGetDesc(lcl1);
var_types type1 = varDsc1->TypeGet();
GenTreeLclVarCommon* lcl2 = tree->gtOp2->AsLclVarCommon();
LclVarDsc* varDsc2 = &(compiler->lvaTable[lcl2->GetLclNum()]);
LclVarDsc* varDsc2 = compiler->lvaGetDesc(lcl2);
var_types type2 = varDsc2->TypeGet();

// We must have both int or both fp regs
Expand Down
23 changes: 11 additions & 12 deletions src/coreclr/jit/codegenarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
// Since it is a fast tail call, the existence of first incoming arg is guaranteed
// because fast tail call requires that in-coming arg area of caller is >= out-going
// arg area required for tail call.
LclVarDsc* varDsc = &(compiler->lvaTable[varNumOut]);
LclVarDsc* varDsc = compiler->lvaGetDesc(varNumOut);
assert(varDsc != nullptr);
#endif // FEATURE_FASTTAILCALL
}
Expand Down Expand Up @@ -1247,10 +1247,9 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
{
assert(varNode->isContained());
srcVarNum = varNode->GetLclNum();
assert(srcVarNum < compiler->lvaCount);

// handle promote situation
LclVarDsc* varDsc = compiler->lvaTable + srcVarNum;
LclVarDsc* varDsc = compiler->lvaGetDesc(srcVarNum);

// This struct also must live in the stack frame
// And it can't live in a register (SIMD)
Expand Down Expand Up @@ -2648,16 +2647,16 @@ void CodeGen::genJmpMethod(GenTree* jmp)
// But that would require us to deal with circularity while moving values around. Spilling
// to stack makes the implementation simple, which is not a bad trade off given Jmp calls
// are not frequent.
for (varNum = 0; (varNum < compiler->info.compArgsCount); varNum++)
for (varNum = 0; varNum < compiler->info.compArgsCount; varNum++)
{
varDsc = compiler->lvaTable + varNum;
varDsc = compiler->lvaGetDesc(varNum);

if (varDsc->lvPromoted)
{
noway_assert(varDsc->lvFieldCnt == 1); // We only handle one field here

unsigned fieldVarNum = varDsc->lvFieldLclStart;
varDsc = compiler->lvaTable + fieldVarNum;
varDsc = compiler->lvaGetDesc(fieldVarNum);
}
noway_assert(varDsc->lvIsParam);

Expand Down Expand Up @@ -2723,15 +2722,15 @@ void CodeGen::genJmpMethod(GenTree* jmp)
// Next move any un-enregistered register arguments back to their register.
regMaskTP fixedIntArgMask = RBM_NONE; // tracks the int arg regs occupying fixed args in case of a vararg method.
unsigned firstArgVarNum = BAD_VAR_NUM; // varNum of the first argument in case of a vararg method.
for (varNum = 0; (varNum < compiler->info.compArgsCount); varNum++)
for (varNum = 0; varNum < compiler->info.compArgsCount; varNum++)
{
varDsc = compiler->lvaTable + varNum;
varDsc = compiler->lvaGetDesc(varNum);
if (varDsc->lvPromoted)
{
noway_assert(varDsc->lvFieldCnt == 1); // We only handle one field here

unsigned fieldVarNum = varDsc->lvFieldLclStart;
varDsc = compiler->lvaTable + fieldVarNum;
varDsc = compiler->lvaGetDesc(fieldVarNum);
}
noway_assert(varDsc->lvIsParam);

Expand Down Expand Up @@ -3253,9 +3252,9 @@ void CodeGen::genCreateAndStoreGCInfo(unsigned codeSize,
if (compiler->opts.IsReversePInvoke())
{
unsigned reversePInvokeFrameVarNumber = compiler->lvaReversePInvokeFrameVar;
assert(reversePInvokeFrameVarNumber != BAD_VAR_NUM && reversePInvokeFrameVarNumber < compiler->lvaRefCount);
LclVarDsc& reversePInvokeFrameVar = compiler->lvaTable[reversePInvokeFrameVarNumber];
gcInfoEncoder->SetReversePInvokeFrameSlot(reversePInvokeFrameVar.GetStackOffset());
assert(reversePInvokeFrameVarNumber != BAD_VAR_NUM);
const LclVarDsc* reversePInvokeFrameVar = compiler->lvaGetDesc(reversePInvokeFrameVarNumber);
gcInfoEncoder->SetReversePInvokeFrameSlot(reversePInvokeFrameVar->GetStackOffset());
}

gcInfoEncoder->Build();
Expand Down
Loading

0 comments on commit 2c55431

Please sign in to comment.