Skip to content

Commit

Permalink
Addressing review comments from Kunal.
Browse files Browse the repository at this point in the history
  • Loading branch information
DeepakRajendrakumaran committed Jan 30, 2025
1 parent 58e9112 commit 99438b6
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 51 deletions.
18 changes: 3 additions & 15 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5778,11 +5778,7 @@ void CodeGen::genFnProlog()

if (initRegs)
{
#ifdef TARGET_AMD64
for (regNumber reg = REG_INT_FIRST; reg <= REG_INT_LAST_APX_AWARE; reg = REG_NEXT(reg))
#else
for (regNumber reg = REG_INT_FIRST; reg <= REG_INT_LAST; reg = REG_NEXT(reg))
#endif
for (regNumber reg = REG_INT_FIRST; reg <= get_REG_INT_LAST(); reg = REG_NEXT(reg))
{
regMaskTP regMask = genRegMask(reg);
if (regMask & initRegs)
Expand Down Expand Up @@ -6324,11 +6320,7 @@ regMaskTP CodeGen::genPushRegs(regMaskTP regs, regMaskTP* byrefRegs, regMaskTP*
noway_assert(genTypeStSz(TYP_BYREF) == genTypeStSz(TYP_I_IMPL));

regMaskTP pushedRegs = regs;
#ifdef TARGET_AMD64
for (regNumber reg = REG_INT_FIRST; reg <= REG_INT_LAST_APX_AWARE; reg = REG_NEXT(reg))
#else
for (regNumber reg = REG_INT_FIRST; reg <= REG_INT_LAST; reg = REG_NEXT(reg))
#endif
for (regNumber reg = REG_INT_FIRST; reg <= get_REG_INT_LAST(); reg = REG_NEXT(reg))
{
regMaskTP regMask = genRegMask(reg);

Expand Down Expand Up @@ -6400,11 +6392,7 @@ void CodeGen::genPopRegs(regMaskTP regs, regMaskTP byrefRegs, regMaskTP noRefReg
regMaskTP popedRegs = regs;

// Walk the registers in the reverse order as genPushRegs()
#ifdef TARGET_AMD64
for (regNumber reg = REG_INT_LAST_APX_AWARE; reg >= REG_INT_LAST; reg = REG_PREV(reg))
#else
for (regNumber reg = REG_INT_LAST; reg >= REG_INT_LAST; reg = REG_PREV(reg))
#endif
for (regNumber reg = get_REG_INT_LAST(); reg >= REG_INT_FIRST; reg = REG_PREV(reg))
{
regMaskTP regMask = genRegMask(reg);

Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/codegeninterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ class CodeGenInterface
{
return this->regIntLast;
}
#else
FORCEINLINE regNumber get_REG_INT_LAST() const
{
return REG_INT_LAST;
}
#endif // TARGET_AMD64

#if defined(TARGET_XARCH)
Expand Down
18 changes: 3 additions & 15 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9908,11 +9908,7 @@ void CodeGen::genOSRRecordTier0CalleeSavedRegistersAndFrame()

// Now the rest of the Tier0 callee saves.
//
#ifdef TARGET_AMD64
for (regNumber reg = REG_INT_LAST_APX_AWARE; tier0IntCalleeSaves != RBM_NONE; reg = REG_PREV(reg))
#else
for (regNumber reg = REG_INT_LAST; tier0IntCalleeSaves != RBM_NONE; reg = REG_PREV(reg))
#endif
for (regNumber reg = get_REG_INT_LAST(); tier0IntCalleeSaves != RBM_NONE; reg = REG_PREV(reg))
{
regMaskTP regBit = genRegMask(reg);

Expand Down Expand Up @@ -10003,11 +9999,7 @@ void CodeGen::genOSRSaveRemainingCalleeSavedRegisters()

// The OSR method must use MOVs to save additional callee saves.
//
#ifdef TARGET_AMD64
for (regNumber reg = REG_INT_LAST_APX_AWARE; osrAdditionalIntCalleeSaves != RBM_NONE; reg = REG_PREV(reg))
#else
for (regNumber reg = REG_INT_LAST; osrAdditionalIntCalleeSaves != RBM_NONE; reg = REG_PREV(reg))
#endif
for (regNumber reg = get_REG_INT_LAST(); osrAdditionalIntCalleeSaves != RBM_NONE; reg = REG_PREV(reg))
{
regMaskTP regBit = genRegMask(reg);

Expand Down Expand Up @@ -10072,11 +10064,7 @@ void CodeGen::genPushCalleeSavedRegisters()

// Push backwards so we match the order we will pop them in the epilog
// and all the other code that expects it to be in this order.
#ifdef TARGET_AMD64
for (regNumber reg = REG_INT_LAST_APX_AWARE; rsPushRegs != RBM_NONE; reg = REG_PREV(reg))
#else
for (regNumber reg = REG_INT_LAST; rsPushRegs != RBM_NONE; reg = REG_PREV(reg))
#endif
for (regNumber reg = get_REG_INT_LAST(); rsPushRegs != RBM_NONE; reg = REG_PREV(reg))
{
regMaskTP regBit = genRegMask(reg);

Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -11789,7 +11789,11 @@ class Compiler
{
return this->regIntLast;
}

#else
FORCEINLINE regNumber get_REG_INT_LAST() const
{
return REG_INT_LAST;
}
#endif // TARGET_AMD64

#if defined(TARGET_XARCH)
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/jit/lsra.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,21 +275,23 @@ SingleTypeRegSet LinearScan::lowSIMDRegs()
#endif
}

#if defined(TARGET_XARCH)
//------------------------------------------------------------------------
// lowGPRRegs(): Return the set of GPR registers associated with non APX
// getLowGprRegs(): Return the set of GPR registers associated with non APX
// encoding only, i.e., remove the eGPR registers from the available
// set.
//
// Return Value:
// Register mask of non APX GPR registers.
SingleTypeRegSet LinearScan::lowGPRRegs()
SingleTypeRegSet LinearScan::getLowGprRegs()
{
#if defined(TARGET_AMD64)
return (availableIntRegs & RBM_LOWINT.GetIntRegSet());
#else
return availableIntRegs;
#endif
#endif // TARGET_AMD64
}
#endif // TARGET_XARCH

void LinearScan::updateNextFixedRef(RegRecord* regRecord, RefPosition* nextRefPosition, RefPosition* nextKill)
{
Expand Down Expand Up @@ -815,6 +817,7 @@ LinearScan::LinearScan(Compiler* theCompiler)
rbmAllInt = compiler->rbmAllInt;
rbmIntCalleeTrash = compiler->rbmIntCalleeTrash;
regIntLast = compiler->regIntLast;
isApxSupported = compiler->canUseApxEncoding();
#endif // TARGET_AMD64

#if defined(TARGET_XARCH)
Expand Down
14 changes: 13 additions & 1 deletion src/coreclr/jit/lsra.h
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,9 @@ class LinearScan : public LinearScanInterface
SingleTypeRegSet allByteRegs();
SingleTypeRegSet allSIMDRegs();
SingleTypeRegSet lowSIMDRegs();
SingleTypeRegSet lowGPRRegs();
#if defined(TARGET_XARCH)
SingleTypeRegSet getLowGprRegs();
#endif
SingleTypeRegSet internalFloatRegCandidates();

void makeRegisterInactive(RegRecord* physRegRecord);
Expand Down Expand Up @@ -2055,6 +2057,7 @@ class LinearScan : public LinearScanInterface
regMaskTP rbmAllInt;
regMaskTP rbmIntCalleeTrash;
regNumber regIntLast;
bool isApxSupported;

FORCEINLINE regMaskTP get_RBM_ALLFLOAT() const
{
Expand All @@ -2076,6 +2079,15 @@ class LinearScan : public LinearScanInterface
{
return this->regIntLast;
}
FORCEINLINE bool getIsApxSupported() const
{
return this->isApxSupported;
}
#else
FORCEINLINE regNumber get_REG_INT_LAST() const
{
return REG_INT_LAST;
}
#endif // TARGET_AMD64

#if defined(TARGET_XARCH)
Expand Down
36 changes: 26 additions & 10 deletions src/coreclr/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ regMaskTP LinearScan::getKillSetForCall(GenTreeCall* call)
#ifdef TARGET_ARM
if (call->IsVirtualStub())
{
killMask.AddGprRegs(compiler->virtualStubParamInfo->GetRegMask().GetIntRegSet(), RBM_ALLINT);
killMask.AddGprRegs(compiler->virtualStubParamInfo->GetRegMask().GetIntRegSet());
}
#else // !TARGET_ARM
// Verify that the special virtual stub call registers are in the kill mask.
Expand All @@ -879,7 +879,11 @@ regMaskTP LinearScan::getKillSetForCall(GenTreeCall* call)
// so don't use the register post-call until it is consumed by SwiftError.
if (call->HasSwiftErrorHandling())
{
#ifdef TARGET_AMD64
killMask.AddGprRegs(RBM_SWIFT_ERROR.GetIntRegSet(), RBM_ALLINT);
#else
killMask.AddGprRegs(RBM_SWIFT_ERROR.GetIntRegSet());
#endif
}
#endif // SWIFT_SUPPORT

Expand Down Expand Up @@ -915,15 +919,23 @@ regMaskTP LinearScan::getKillSetForBlockStore(GenTreeBlk* blkNode)
if (isCopyBlk)
{
// rep movs kills RCX, RDI and RSI
#ifdef TARGET_AMD64
killMask.AddGprRegs(SRBM_RCX | SRBM_RDI | SRBM_RSI, RBM_ALLINT);
#else
killMask.AddGprRegs(SRBM_RCX | SRBM_RDI | SRBM_RSI);
#endif
}
else
{
// rep stos kills RCX and RDI.
// (Note that the Data() node, if not constant, will be assigned to
// RCX, but it's find that this kills it, as the value is not available
// after this node in any case.)
#ifdef TARGET_AMD64
killMask.AddGprRegs(SRBM_RDI | SRBM_RCX, RBM_ALLINT);
#else
killMask.AddGprRegs(SRBM_RDI | SRBM_RCX);
#endif
}
break;
#endif
Expand Down Expand Up @@ -2275,7 +2287,11 @@ void LinearScan::buildIntervals()
// If there is a secret stub param, it is also live in
if (compiler->info.compPublishStubParam)
{
#ifdef TARGET_AMD64
intRegState->rsCalleeRegArgMaskLiveIn.AddGprRegs(RBM_SECRET_STUB_PARAM.GetIntRegSet(), RBM_ALLINT);
#else
intRegState->rsCalleeRegArgMaskLiveIn.AddGprRegs(RBM_SECRET_STUB_PARAM.GetIntRegSet());
#endif

LclVarDsc* stubParamDsc = compiler->lvaGetDesc(compiler->lvaStubArgumentVar);
if (isCandidateVar(stubParamDsc))
Expand Down Expand Up @@ -3802,11 +3818,11 @@ int LinearScan::BuildBinaryUses(GenTreeOp* node, SingleTypeRegSet candidates)
{
if (op1->isContainedIndir())
{
return BuildRMWUses(node, op1, op2, lowGPRRegs(), candidates);
return BuildRMWUses(node, op1, op2, getLowGprRegs(), candidates);
}
else
{
return BuildRMWUses(node, op1, op2, candidates, lowGPRRegs());
return BuildRMWUses(node, op1, op2, candidates, getLowGprRegs());
}
}
return BuildRMWUses(node, op1, op2, candidates, candidates);
Expand All @@ -3821,7 +3837,7 @@ int LinearScan::BuildBinaryUses(GenTreeOp* node, SingleTypeRegSet candidates)
((varTypeUsesFloatReg(node) || node->OperGet() == GT_BSWAP || node->OperGet() == GT_BSWAP16)) &&
candidates == RBM_NONE)
{
srcCount += BuildOperandUses(op1, lowGPRRegs());
srcCount += BuildOperandUses(op1, getLowGprRegs());
}
else
#endif
Expand All @@ -3835,7 +3851,7 @@ int LinearScan::BuildBinaryUses(GenTreeOp* node, SingleTypeRegSet candidates)
#ifdef TARGET_XARCH
if (op2->isContainedIndir() && varTypeUsesFloatReg(op1) && candidates == RBM_NONE)
{
candidates = lowGPRRegs();
candidates = getLowGprRegs();
}
#endif
srcCount += BuildOperandUses(op2, candidates);
Expand Down Expand Up @@ -3939,7 +3955,7 @@ void LinearScan::BuildStoreLocDef(GenTreeLclVarCommon* storeLoc,
#ifdef TARGET_AMD64
if (op1->isContained() && op1->OperIs(GT_BITCAST) && varTypeUsesIntReg(varDsc->GetRegisterType(storeLoc)))
{
defCandidates = lowGPRRegs();
defCandidates = getLowGprRegs();
}

#endif // TARGET_AMD64
Expand Down Expand Up @@ -4113,7 +4129,7 @@ int LinearScan::BuildStoreLoc(GenTreeLclVarCommon* storeLoc)
#ifdef TARGET_AMD64
if (registerType == IntRegisterType)
{
candidates = lowGPRRegs();
candidates = getLowGprRegs();
}
else
#endif // TARGET_AMD64
Expand Down Expand Up @@ -4205,7 +4221,7 @@ int LinearScan::BuildSimple(GenTree* tree)
#ifdef TARGET_AMD64
if ((tree->OperGet() == GT_BSWAP || tree->OperGet() == GT_BSWAP16) && varTypeUsesIntReg(tree))
{
BuildDef(tree, lowGPRRegs());
BuildDef(tree, getLowGprRegs());
}
else
#endif // TARGET_AMD64
Expand Down Expand Up @@ -4633,12 +4649,12 @@ int LinearScan::BuildCmpOperands(GenTree* tree)
if (op2->isContainedIndir() && varTypeUsesFloatReg(op1) && op2Candidates == RBM_NONE)
{
// We only use RSI and RDI for EnC code, so we don't want to favor callee-save regs.
op2Candidates = lowGPRRegs();
op2Candidates = getLowGprRegs();
}
if (op1->isContainedIndir() && varTypeUsesFloatReg(op2) && op1Candidates == RBM_NONE)
{
// We only use RSI and RDI for EnC code, so we don't want to favor callee-save regs.
op1Candidates = lowGPRRegs();
op1Candidates = getLowGprRegs();
}
#endif // TARGET_AMD64

Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/jit/lsraxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ int LinearScan::BuildNode(GenTree* tree)
{
if (varTypeUsesFloatReg(tree) && (varTypeUsesIntReg(tree->gtGetOp1())))
{
BuildUse(tree->gtGetOp1(), lowGPRRegs());
BuildUse(tree->gtGetOp1(), getLowGprRegs());
}
else
{
Expand All @@ -371,7 +371,7 @@ int LinearScan::BuildNode(GenTree* tree)
// TODO-Xarch-apx: Revisit once extended EVEX is available. Currently limiting high GPR for int <-> float
if (varTypeUsesFloatReg(tree->gtGetOp1()) && (varTypeUsesIntReg(tree)))
{
BuildDef(tree, lowGPRRegs());
BuildDef(tree, getLowGprRegs());
}
else
{
Expand Down Expand Up @@ -3356,7 +3356,7 @@ inline SingleTypeRegSet LinearScan::BuildApxIncompatibleGPRMask(GenTree*
{
#if defined(TARGET_AMD64)

if (!(compiler->canUseApxEncoding()))
if (!getIsApxSupported())
{
return candidates;
}
Expand All @@ -3365,11 +3365,11 @@ inline SingleTypeRegSet LinearScan::BuildApxIncompatibleGPRMask(GenTree*
{
if (candidates == RBM_NONE)
{
return lowGPRRegs();
return getLowGprRegs();
}
else
{
return (candidates & lowGPRRegs());
return (candidates & getLowGprRegs());
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/jit/regMaskTPOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,19 @@ bool regMaskTP::IsRegNumInMask(regNumber reg, var_types type) const
// gprRegs - Register to check
// availableIntRegs - RBM_ALLINT passed in at runtime
//
#ifdef TARGET_AMD64
void regMaskTP::AddGprRegs(SingleTypeRegSet gprRegs, regMaskTP availableIntRegs)
{
assert((gprRegs == RBM_NONE) || ((gprRegs & availableIntRegs) != RBM_NONE));
low |= gprRegs;
}
#else
void regMaskTP::AddGprRegs(SingleTypeRegSet gprRegs)
{
assert((gprRegs == RBM_NONE) || ((gprRegs & RBM_ALLINT) != RBM_NONE));
low |= gprRegs;
}
#endif

//------------------------------------------------------------------------
// AddRegNum: This is similar to AddRegNumInMask(reg, regType) for all platforms
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/target.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,11 @@ struct regMaskTP
void RemoveRegNumFromMask(regNumber reg, var_types type);
bool IsRegNumInMask(regNumber reg, var_types type) const;
#endif
void AddGprRegs(SingleTypeRegSet gprRegs, regMaskTP availableIntRegs);
#ifdef TARGET_AMD64
void AddGprRegs(SingleTypeRegSet gprRegs, regMaskTP availableIntRegs);
#else
void AddGprRegs(SingleTypeRegSet gprRegs);
#endif
void AddRegNum(regNumber reg, var_types type);
void AddRegNumInMask(regNumber reg);
void AddRegsetForType(SingleTypeRegSet regsToAdd, var_types type);
Expand Down

0 comments on commit 99438b6

Please sign in to comment.