Skip to content

Commit

Permalink
Fix an over-constrained use of a byte reg
Browse files Browse the repository at this point in the history
Fix #40963
  • Loading branch information
CarolEidt committed Aug 20, 2020
1 parent 3aced82 commit 1375f4b
Showing 1 changed file with 49 additions and 2 deletions.
51 changes: 49 additions & 2 deletions src/coreclr/src/jit/lsraxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,11 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode)
regMaskTP srcRegMask = RBM_NONE;
regMaskTP sizeRegMask = RBM_NONE;

RefPosition* internalIntDef = nullptr;
#ifdef TARGET_X86
bool internalIsByte = false;
#endif

if (blkNode->OperIsInitBlkOp())
{
if (src->OperIs(GT_INIT_VAL))
Expand Down Expand Up @@ -1359,10 +1364,11 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode)
if ((size & 1) != 0)
{
// We'll need to store a byte so a byte register is needed on x86.
regMask = allByteRegs();
regMask = allByteRegs();
internalIsByte = true;
}
#endif
buildInternalIntRegisterDefForNode(blkNode, regMask);
internalIntDef = buildInternalIntRegisterDefForNode(blkNode, regMask);
}

if (size >= XMM_REGSIZE_BYTES)
Expand Down Expand Up @@ -1436,9 +1442,30 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode)
BuildUse(blkNode->AsDynBlk()->gtDynamicSize, sizeRegMask);
}

#ifdef TARGET_X86
// If we require a byte register on x86, we may run into an over-constrained situation
// if we have 4 or more uses (currently, it can be at most 4, if both the source and
// destination have base+index addressing).
// This is because the byteable register requirement doesn't "reserve" a specific register,
// and it would be possible for the incoming sources to all be occupying the byteable
// registers, leaving none free for the internal register.
// In this scenario, we will require rax to ensure that it is reserved and available.
// We need to make that modification prior to building the uses for the internal register,
// so that when we create the use we will also create the RefTypeFixedRef on the RegRecord.
// We don't expect a useCount of more than 3 for the initBlk case, so we haven't set
// internalIsByte in that case above.
assert((useCount < 4) || !blkNode->OperIsInitBlkOp());
if (internalIsByte && (useCount >= 4))
{
noway_assert(internalIntDef != nullptr);
internalIntDef->registerAssignment = RBM_RAX;
}
#endif

buildInternalRegisterUses();
regMaskTP killMask = getKillSetForBlockStore(blkNode);
BuildDefsWithKills(blkNode, 0, RBM_NONE, killMask);

return useCount;
}

Expand Down Expand Up @@ -1594,6 +1621,15 @@ int LinearScan::BuildPutArgStk(GenTreePutArgStk* putArgStk)

srcCount = BuildOperandUses(src);
buildInternalRegisterUses();

#ifdef TARGET_X86
// There are only 4 byteable registers on x86. If we require a byteable internal register,
// we must have less than 4 sources.
// If we have 4 or more sources, and require a byteable internal register, we need to reserve
// one explicitly (see BuildBlockStore()).
assert(srcCount < 4);
#endif

return srcCount;
}
#endif // FEATURE_PUT_STRUCT_ARG_STK
Expand Down Expand Up @@ -2728,6 +2764,7 @@ int LinearScan::BuildIndir(GenTreeIndir* indirTree)
}
}
}

#ifdef FEATURE_SIMD
if (varTypeIsSIMD(indirTree))
{
Expand All @@ -2736,6 +2773,16 @@ int LinearScan::BuildIndir(GenTreeIndir* indirTree)
buildInternalRegisterUses();
#endif // FEATURE_SIMD

#ifdef TARGET_X86
// There are only 4 byteable registers on x86. If we have a source that requires
// such a register, we must have no more than 4 sources.
// If we have more than 4 sources, and require a byteable register, we need to reserve
// one explicitly (see BuildBlockStore()).
// (Note that the assert below doesn't count internal registers because we only have
// floating point internal registers, if any).
assert(srcCount <= 4);
#endif

if (indirTree->gtOper != GT_STOREIND)
{
BuildDef(indirTree);
Expand Down

0 comments on commit 1375f4b

Please sign in to comment.