`BuildUse` was setting the regNumber for all of the uses of a multi-reg node to the main/first regNum. This was missed because this results in a def/use conflict on that reg (the def was set correctly), which is generally resolved in favor of the def. The exception is when there is a kill of the register in between, in which case the use tries to allocate the register its been assigned, causing the `farthestRefPhysRegRecord != nullptr` assert (aka "a register can't be found to spill").
This fixes the specific issue, and adds additional asserts to identify future/additional such issues.
The new asserts depend upon all the regNums being appropriately when/if any are set, which wasn't always the case prior to register allocation.
Fix dotnet/coreclr#18153
Commit migrated from https://github.com/dotnet/coreclr/commit/
b4470d75fa6ab4461607f2a0135cd118f466c2ef
// Returns true if it is a node returning its value in more than one register
inline bool IsMultiRegNode() const;
+ // Returns the regIndex'th register defined by a possibly-multireg node.
+ regNumber GetRegByIndex(int regIndex);
+
// Returns true if it is a GT_COPY or GT_RELOAD node
inline bool IsCopyOrReload() const;
return false;
}
+//-----------------------------------------------------------------------------------
+// GetRegByIndex: Get a specific register, based on regIndex, that is produced
+// by this node.
+//
+// Arguments:
+// regIndex - which register to return (must be 0 for non-multireg nodes)
+//
+// Return Value:
+// The register, if any, assigned to this index for this node.
+//
+inline regNumber GenTree::GetRegByIndex(int regIndex)
+{
+ if (regIndex == 0)
+ {
+ return gtRegNum;
+ }
+ if (IsMultiRegCall())
+ {
+ return AsCall()->GetRegNumByIdx(regIndex);
+ }
+
+#if defined(_TARGET_ARM_)
+ if (OperIsMultiRegOp())
+ {
+ return AsMultiRegOp()->GetRegNumByIdx(regIndex);
+ }
+ else if (OperIsPutArgSplit())
+ {
+ return AsPutArgSplit()->GetRegNumByIdx(regIndex);
+ }
+ else if (OperIs(GT_COPY, GT_RELOAD))
+ {
+ return AsCopyOrReload()->GetRegNumByIdx(regIndex);
+ }
+#endif
+ assert(!"Invalid regIndex for GetRegFromMultiRegNode");
+ return REG_NA;
+}
+
//-------------------------------------------------------------------------
// IsCopyOrReload: whether this is a GT_COPY or GT_RELOAD node.
//
putArg = new (comp, GT_PUTARG_SPLIT)
GenTreePutArgSplit(arg, info->slotNum PUT_STRUCT_ARG_STK_ONLY_ARG(info->numSlots), info->numRegs,
call->IsFastTailCall(), call);
- putArg->gtRegNum = info->regNum;
// If struct argument is morphed to GT_FIELD_LIST node(s),
// we can know GC info by type of each GT_FIELD_LIST node.
// So we skip setting GC Pointer info.
//
GenTreePutArgSplit* argSplit = putArg->AsPutArgSplit();
+ for (unsigned regIndex = 0; regIndex < info->numRegs; regIndex++)
+ {
+ argSplit->SetRegNumByIdx(info->getRegNum(regIndex), regIndex);
+ }
+
if (arg->OperGet() == GT_OBJ)
{
BYTE* gcLayout = nullptr;
printf("\n Use:");
interval->microDump();
printf("(#%d)", currentRefPosition->rpNum);
- if (currentRefPosition->isFixedRegRef)
+ if (currentRefPosition->isFixedRegRef && !interval->isInternal)
{
assert(genMaxOneBit(currentRefPosition->registerAssignment));
assert(lastFixedRegRefPos != nullptr);
assert(regCount == curArgTabEntry->numRegs);
for (unsigned int i = 0; i < regCount; i++)
{
- BuildUse(argNode, genRegMask(argNode->gtRegNum), i);
+ BuildUse(argNode, genRegMask(argNode->AsPutArgSplit()->GetRegNumByIdx(i)), i);
}
srcCount += regCount;
}
// incorrectly allocate that register.
// TODO-CQ: This means that we may often require a copy at the use of this node's result.
// This case could be moved to BuildRefPositionsForNode, at the point where the def RefPosition is
-// created, causing a RefTypeFixedRef to be added at that location. This, however, results in
+// created, causing a RefTypeFixedReg to be added at that location. This, however, results in
// more PhysReg RefPositions (a throughput impact), and a large number of diffs that require
// further analysis to determine benefit.
// See Issue #11274.
{
RefPosition* newRP = newRefPositionRaw(theLocation, theTreeNode, theRefType);
- newRP->setReg(getRegisterRecord(reg));
+ RegRecord* regRecord = getRegisterRecord(reg);
+ newRP->setReg(regRecord);
newRP->registerAssignment = mask;
newRP->setMultiRegIdx(0);
newRP->setAllocateIfProfitable(false);
+ // We can't have two RefPositions on a RegRecord at the same location, unless they are different types.
+ assert((regRecord->lastRefPosition == nullptr) || (regRecord->lastRefPosition->nodeLocation < theLocation) ||
+ (regRecord->lastRefPosition->refType != theRefType));
associateRefPosWithInterval(newRP);
DBEXEC(VERBOSE, newRP->dump());
bool insertFixedRef = false;
if (isFixedRegister)
{
- // Insert a RefTypeFixedReg for any normal def or use (not ParamDef or BB)
- if (theRefType == RefTypeUse || theRefType == RefTypeDef)
+ // Insert a RefTypeFixedReg for any normal def or use (not ParamDef or BB),
+ // but not an internal use (it will already have a FixedRef for the def).
+ if ((theRefType == RefTypeDef) || ((theRefType == RefTypeUse) && !theInterval->isInternal))
{
insertFixedRef = true;
}
{
assert(!tree->isContained());
RegisterType type = getDefType(tree);
+
+ if (dstCandidates != RBM_NONE)
+ {
+ assert((tree->gtRegNum == REG_NA) || (dstCandidates == genRegMask(tree->GetRegByIndex(multiRegIdx))));
+ }
+
#ifdef FEATURE_MULTIREG_ARGS_OR_RET
if (tree->TypeGet() == TYP_STRUCT)
{
Interval* interval;
bool regOptional = operand->IsRegOptional();
- if (operand->gtRegNum != REG_NA)
- {
- candidates = genRegMask(operand->gtRegNum);
- }
if (isCandidateLocalRef(operand))
{
interval = getIntervalForLocalVarNode(operand->AsLclVarCommon());
projects / platform / upstream / dotnet / runtime.git / commitdiff