Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare JIT backend for structs in registers. #52039

Merged
merged 5 commits into from
May 5, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4381,7 +4381,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere

/* Register argument - hopefully it stays in the same register */
regNumber destRegNum = REG_NA;
var_types destMemType = varDsc->TypeGet();
var_types destMemType = varDsc->GetRegisterType();

if (regArgTab[argNum].slot == 1)
{
Expand Down Expand Up @@ -4657,8 +4657,6 @@ void CodeGen::genEnregisterIncomingStackArgs()
continue;
}

var_types type = genActualType(varDsc->TypeGet());

/* Is the variable dead on entry */

if (!VarSetOps::IsMember(compiler, compiler->fgFirstBB->bbLiveIn, varDsc->lvVarIndex))
Expand All @@ -4673,7 +4671,9 @@ void CodeGen::genEnregisterIncomingStackArgs()
regNumber regNum = varDsc->GetArgInitReg();
assert(regNum != REG_STK);

GetEmitter()->emitIns_R_S(ins_Load(type), emitTypeSize(type), regNum, varNum, 0);
var_types regType = varDsc->GetActualRegisterType();

GetEmitter()->emitIns_R_S(ins_Load(regType), emitTypeSize(regType), regNum, varNum, 0);
regSet.verifyRegUsed(regNum);
#ifdef USING_SCOPE_INFO
psiMoveToReg(varNum);
Expand Down
41 changes: 24 additions & 17 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -864,14 +864,14 @@ void CodeGen::genSpillVar(GenTree* tree)
// therefore be store-normalized (rather than load-normalized). In fact, not performing store normalization
// can lead to problems on architectures where a lclVar may be allocated to a register that is not
// addressable at the granularity of the lclVar's defined type (e.g. x86).
var_types lclTyp = genActualType(varDsc->TypeGet());
emitAttr size = emitTypeSize(lclTyp);
var_types lclType = varDsc->GetActualRegisterType();
emitAttr size = emitTypeSize(lclType);

// If this is a write-thru variable, we don't actually spill at a use, but we will kill the var in the reg
// (below).
if (!varDsc->lvLiveInOutOfHndlr)
{
instruction storeIns = ins_Store(lclTyp, compiler->isSIMDTypeLocalAligned(varNum));
instruction storeIns = ins_Store(lclType, compiler->isSIMDTypeLocalAligned(varNum));
assert(varDsc->GetRegNum() == tree->GetRegNum());
inst_TT_RV(storeIns, size, tree, tree->GetRegNum());
}
Expand Down Expand Up @@ -1181,7 +1181,8 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree)

GenTreeLclVar* lcl = unspillTree->AsLclVar();
LclVarDsc* varDsc = compiler->lvaGetDesc(lcl->GetLclNum());
var_types spillType = unspillTree->TypeGet();
var_types spillType = varDsc->GetRegisterType(lcl);
assert(spillType != TYP_UNDEF);

// TODO-Cleanup: The following code could probably be further merged and cleaned up.
#ifdef TARGET_XARCH
Expand All @@ -1196,11 +1197,12 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree)
// later used as a long, we will have incorrectly truncated the long.
// In the normalizeOnLoad case ins_Load will return an appropriate sign- or zero-
// extending load.

if (spillType != genActualType(varDsc->lvType) && !varTypeIsGC(spillType) && !varDsc->lvNormalizeOnLoad())
var_types lclActualType = varDsc->GetActualRegisterType();
assert(lclActualType != TYP_UNDEF);
if (spillType != lclActualType && !varTypeIsGC(spillType) && !varDsc->lvNormalizeOnLoad())
{
assert(!varTypeIsGC(varDsc));
spillType = genActualType(varDsc->lvType);
spillType = lclActualType;
}
#elif defined(TARGET_ARM64)
var_types targetType = unspillTree->gtType;
Expand Down Expand Up @@ -2046,8 +2048,8 @@ void CodeGen::genConsumeBlockOp(GenTreeBlk* blkNode, regNumber dstReg, regNumber
//
void CodeGen::genSpillLocal(unsigned varNum, var_types type, GenTreeLclVar* lclNode, regNumber regNum)
{
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);
assert(!varDsc->lvNormalizeOnStore() || (type == genActualType(varDsc->TypeGet())));
const LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);
assert(!varDsc->lvNormalizeOnStore() || (type == varDsc->GetActualRegisterType()));

// We have a register candidate local that is marked with GTF_SPILL.
// This flag generally means that we need to spill this local.
Expand Down Expand Up @@ -2093,24 +2095,29 @@ void CodeGen::genProduceReg(GenTree* tree)

if (genIsRegCandidateLocal(tree))
{
unsigned varNum = tree->AsLclVarCommon()->GetLclNum();
genSpillLocal(varNum, tree->TypeGet(), tree->AsLclVar(), tree->GetRegNum());
GenTreeLclVar* lclNode = tree->AsLclVar();
const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNode);
const unsigned varNum = lclNode->GetLclNum();
const var_types spillType = varDsc->GetRegisterType(lclNode);
genSpillLocal(varNum, spillType, lclNode, tree->GetRegNum());
}
else if (tree->IsMultiRegLclVar())
{
assert(compiler->lvaEnregMultiRegVars);
GenTreeLclVar* lclNode = tree->AsLclVar();
LclVarDsc* varDsc = compiler->lvaGetDesc(lclNode->GetLclNum());
unsigned regCount = lclNode->GetFieldCount(compiler);

GenTreeLclVar* lclNode = tree->AsLclVar();
const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNode);
const unsigned regCount = lclNode->GetFieldCount(compiler);

for (unsigned i = 0; i < regCount; ++i)
{
unsigned flags = lclNode->GetRegSpillFlagByIdx(i);
if ((flags & GTF_SPILL) != 0)
{
regNumber reg = lclNode->GetRegNumByIdx(i);
unsigned fieldVarNum = varDsc->lvFieldLclStart + i;
genSpillLocal(fieldVarNum, compiler->lvaGetDesc(fieldVarNum)->TypeGet(), lclNode, reg);
const regNumber reg = lclNode->GetRegNumByIdx(i);
const unsigned fieldVarNum = varDsc->lvFieldLclStart + i;
const var_types spillType = compiler->lvaGetDesc(fieldVarNum)->GetRegisterType();
genSpillLocal(fieldVarNum, spillType, lclNode, reg);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4512,7 +4512,7 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* lclNode)
else if (op1->GetRegNum() != targetReg)
{
assert(op1->GetRegNum() != REG_NA);
emit->emitInsBinary(ins_Move_Extend(targetType, true), emitTypeSize(lclNode), lclNode, op1);
emit->emitInsBinary(ins_Move_Extend(targetType, true), emitTypeSize(targetType), lclNode, op1);
}
}
if (targetReg != REG_NA)
Expand Down
37 changes: 12 additions & 25 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1014,33 +1014,15 @@ class LclVarDsc
return lvPerSsaData.GetSsaDef(ssaNum);
}

//------------------------------------------------------------------------
// GetRegisterType: Determine register type for that local var.
//
// Arguments:
// tree - node that uses the local, its type is checked first.
//
// Return Value:
// TYP_UNDEF if the layout is enregistrable, register type otherwise.
//
var_types GetRegisterType(const GenTreeLclVarCommon* tree) const
{
var_types targetType = tree->gtType;
var_types GetRegisterType(const GenTreeLclVarCommon* tree) const;

#ifdef DEBUG
// Ensure that lclVar nodes are typed correctly.
if (tree->OperIs(GT_STORE_LCL_VAR) && lvNormalizeOnStore())
{
// TODO: update that assert to work with TypeGet() == TYP_STRUCT case.
// assert(targetType == genActualType(TypeGet()));
}
#endif
var_types GetRegisterType() const;

if (targetType != TYP_STRUCT)
{
return targetType;
}
return GetLayout()->GetRegisterType();
var_types GetActualRegisterType() const;

bool IsEnregisterable() const
{
return GetRegisterType() != TYP_UNDEF;
}

bool CanBeReplacedWithItsField(Compiler* comp) const;
Expand Down Expand Up @@ -9684,6 +9666,11 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
#endif // FEATURE_MULTIREG_RET
}

bool compEnregStructLocals()
{
return JitConfig.JitEnregStructLocals();
}

// Returns true if the method returns a value in more than one return register,
// it should replace/be merged with compMethodReturnsMultiRegRetType when #36868 is fixed.
// The difference from original `compMethodReturnsMultiRegRetType` is in ARM64 SIMD* handling,
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5845,6 +5845,7 @@ var_types GenTreeLclVar::GetFieldTypeByIndex(Compiler* compiler, unsigned idx)
assert(IsMultiReg());
LclVarDsc* varDsc = compiler->lvaGetDesc(GetLclNum());
LclVarDsc* fieldVarDsc = compiler->lvaGetDesc(varDsc->lvFieldLclStart + idx);
assert(fieldVarDsc->TypeGet() != TYP_STRUCT); // Don't expect struct fields.
return fieldVarDsc->TypeGet();
}

Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,9 @@ CONFIG_INTEGER(JitSaveFpLrWithCalleeSavedRegisters, W("JitSaveFpLrWithCalleeSave
#endif // defined(TARGET_ARM64)
#endif // DEBUG

// Allow to enregister locals with struct type.
CONFIG_INTEGER(JitEnregStructLocals, W("JitEnregStructLocals"), 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want this to be available in Release?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes


#undef CONFIG_INTEGER
#undef CONFIG_STRING
#undef CONFIG_METHODSET
68 changes: 68 additions & 0 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3845,6 +3845,74 @@ var_types LclVarDsc::lvaArgType()
return type;
}

//------------------------------------------------------------------------
// GetRegisterType: Determine register type for this local var.
//
// Arguments:
// tree - node that uses the local, its type is checked first.
//
// Return Value:
// TYP_UNDEF if the layout is not enregistrable, the register type otherwise.
//
var_types LclVarDsc::GetRegisterType(const GenTreeLclVarCommon* tree) const
{
var_types targetType = tree->gtType;
var_types lclVarType = TypeGet();

if (targetType == TYP_STRUCT)
{
if (lclVarType == TYP_STRUCT)
{
lclVarType = GetLayout()->GetRegisterType();
}
targetType = lclVarType;
}

#ifdef DEBUG
if ((targetType != TYP_UNDEF) && tree->OperIs(GT_STORE_LCL_VAR) && lvNormalizeOnStore())
{
const bool phiStore = (tree->gtGetOp1()->OperIsNonPhiLocal() == false);
// Ensure that the lclVar node is typed correctly,
// does not apply to phi-stores because they do not produce code in the merge block.
assert(phiStore || targetType == genActualType(lclVarType));
}
#endif
return targetType;
}

//------------------------------------------------------------------------
// GetRegisterType: Determine register type for this local var.
//
// Return Value:
// TYP_UNDEF if the layout is not enregistrable, the register type otherwise.
//
var_types LclVarDsc::GetRegisterType() const
{
if (TypeGet() != TYP_STRUCT)
{
#if !defined(TARGET_64BIT)
if (TypeGet() == TYP_LONG)
{
return TYP_UNDEF;
}
#endif
return TypeGet();
}
assert(m_layout != nullptr);
return m_layout->GetRegisterType();
}

//------------------------------------------------------------------------
// GetActualRegisterType: Determine an actual register type for this local var.
//
// Return Value:
// TYP_UNDEF if the layout is not enregistrable, the register type otherwise.
//
var_types LclVarDsc::GetActualRegisterType() const
{
return genActualType(GetRegisterType());
}

//----------------------------------------------------------------------------------------------
// CanBeReplacedWithItsField: check if a whole struct reference could be replaced by a field.
//
Expand Down
26 changes: 20 additions & 6 deletions src/coreclr/jit/lsra.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ BasicBlock::weight_t LinearScan::getWeight(RefPosition* refPos)
// in time (more of a 'bank' of registers).
regMaskTP LinearScan::allRegs(RegisterType rt)
{
assert((rt != TYP_UNDEF) && (rt != TYP_STRUCT));
if (rt == TYP_FLOAT)
{
return availableFloatRegs;
Expand Down Expand Up @@ -1495,11 +1496,16 @@ bool LinearScan::isRegCandidate(LclVarDsc* varDsc)
// or enregistered, on x86 -- it is believed that we can enregister pinned (more properly, "pinning")
// references when using the general GC encoding.
unsigned lclNum = (unsigned)(varDsc - compiler->lvaTable);
if (varDsc->lvAddrExposed || !varTypeIsEnregisterable(varDsc))
if (varDsc->lvAddrExposed || !varDsc->IsEnregisterable() ||
(!compiler->compEnregStructLocals() && (varDsc->lvType == TYP_STRUCT)))
{
#ifdef DEBUG
Compiler::DoNotEnregisterReason dner = Compiler::DNER_AddrExposed;
if (!varDsc->lvAddrExposed)
Compiler::DoNotEnregisterReason dner;
if (varDsc->lvAddrExposed)
{
dner = Compiler::DNER_AddrExposed;
}
else
{
dner = Compiler::DNER_IsStruct;
}
Expand Down Expand Up @@ -1551,7 +1557,11 @@ bool LinearScan::isRegCandidate(LclVarDsc* varDsc)
#endif // FEATURE_SIMD

case TYP_STRUCT:
return false;
// TODO-1stClassStructs: support vars with GC pointers. The issue is that such
// vars will have `lvMustInit` set, because emitter has poor support for struct liveness,
// but if the variable is tracked the prolog generator would expect it to be in liveIn set,
// so an assert in `genFnProlog` will fire.
return compiler->compEnregStructLocals() && !varDsc->HasGCPtr();

case TYP_UNDEF:
case TYP_UNKNOWN:
Expand Down Expand Up @@ -1771,7 +1781,11 @@ void LinearScan::identifyCandidates()

if (varDsc->lvLRACandidate)
{
var_types type = genActualType(varDsc->TypeGet());
var_types type = varDsc->GetActualRegisterType();
if (varTypeUsesFloatReg(type))
{
compiler->compFloatingPointUsed = true;
}
Interval* newInt = newInterval(type);
newInt->setLocalNumber(compiler, lclNum, this);
VarSetOps::AddElemD(compiler, registerCandidateVars, varDsc->lvVarIndex);
Expand Down Expand Up @@ -7935,7 +7949,7 @@ void LinearScan::insertMove(
}
else
{
var_types movType = genActualType(varDsc->TypeGet());
var_types movType = varDsc->GetRegisterType();
src->gtType = movType;

dst = new (compiler, GT_COPY) GenTreeCopyOrReload(GT_COPY, movType, src);
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2145,8 +2145,9 @@ void LinearScan::buildIntervals()

if (isCandidateVar(argDsc))
{
Interval* interval = getIntervalForLocalVar(varIndex);
regMaskTP mask = allRegs(TypeGet(argDsc));
Interval* interval = getIntervalForLocalVar(varIndex);
const var_types regType = argDsc->GetRegisterType();
regMaskTP mask = allRegs(regType);
if (argDsc->lvIsRegArg)
{
// Set this interval as currently assigned to that register
Expand Down Expand Up @@ -3207,7 +3208,7 @@ void LinearScan::BuildStoreLocDef(GenTreeLclVarCommon* storeLoc,
}

regMaskTP defCandidates = RBM_NONE;
var_types type = varDsc->TypeGet();
var_types type = varDsc->GetRegisterType();

#ifdef TARGET_X86
if (varTypeIsByte(type))
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/scopeinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,15 +452,15 @@ void CodeGenInterface::siVarLoc::siFillRegisterVarLoc(
// Called for every psiScope in "psiScopeList" codegen.h
CodeGenInterface::siVarLoc::siVarLoc(const LclVarDsc* varDsc, regNumber baseReg, int offset, bool isFramePointerUsed)
{
var_types type = genActualType(varDsc->TypeGet());

if (varDsc->lvIsInReg())
{
siFillRegisterVarLoc(varDsc, type, baseReg, offset, isFramePointerUsed);
var_types regType = varDsc->GetActualRegisterType();
siFillRegisterVarLoc(varDsc, regType, baseReg, offset, isFramePointerUsed);
}
else
{
siFillStackVarLoc(varDsc, type, baseReg, offset, isFramePointerUsed);
var_types stackType = genActualType(varDsc->TypeGet());
siFillStackVarLoc(varDsc, stackType, baseReg, offset, isFramePointerUsed);
}
}

Expand Down