Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[RyuJIT/armel] Make RefPosition arg regs fixed #13023

Merged
merged 5 commits into from
Aug 8, 2017
Merged

[RyuJIT/armel] Make RefPosition arg regs fixed #13023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5b4c7b
[RyuJIT/armel] Make RefPosition arg regs fixed
wateret Jul 25, 2017
0e0e60d
[RyuJIT/armel] Each reg gets own SpillFlag for MultiRegOp
wateret Aug 1, 2017
45a2404
[RyuJIT/armel] Allocate one more temp for call address
wateret Aug 2, 2017
e36d674
[RyuJIT/armel] arg regs are always TYP_INT
wateret Aug 4, 2017
99e4b01
[RyuJIT/armel] Fix ifdef condition and Formatting Error
wateret Aug 7, 2017
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
42 changes: 42 additions & 0 deletions src/jit/codegenlinear.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,32 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree)
}
}

unspillTree->gtFlags &= ~GTF_SPILLED;
}
else if (unspillTree->OperIsMultiRegOp())
{
GenTreeMultiRegOp* multiReg = unspillTree->AsMultiRegOp();
unsigned regCount = multiReg->gtOtherReg == REG_NA ? 1 : 2;

// In case of split struct argument node, GTF_SPILLED flag on it indicates that
// one or more of its result regs are spilled. Call node needs to be
// queried to know which specific result regs to be unspilled.
for (unsigned i = 0; i < regCount; ++i)
{
unsigned flags = multiReg->GetRegSpillFlagByIdx(i);
if ((flags & GTF_SPILLED) != 0)
{
var_types dstType = multiReg->GetRegType(i);
regNumber dstReg = multiReg->GetRegNumByIdx(i);

TempDsc* t = regSet.rsUnspillInPlace(multiReg, dstReg, i);
getEmitter()->emitIns_R_S(ins_Load(dstType), emitActualTypeSize(dstType), dstReg, t->tdTempNum(),
0);
compiler->tmpRlsTemp(t);
gcInfo.gcMarkRegPtrVal(dstReg, dstType);
}
}

unspillTree->gtFlags &= ~GTF_SPILLED;
}
Copy link

Choose a reason for hiding this comment

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

I think that we should merge these cases, and unify the handling of all the different multi-reg ops on the various targets. I'll open a separate issue for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@CarolEidt Definitely I agree with that.

#endif
Expand Down Expand Up @@ -1656,6 +1682,22 @@ void CodeGen::genProduceReg(GenTree* tree)
}
}
}
else if (tree->OperIsMultiRegOp())
{
GenTreeMultiRegOp* multiReg = tree->AsMultiRegOp();
unsigned regCount = multiReg->gtOtherReg == REG_NA ? 1 : 2;

for (unsigned i = 0; i < regCount; ++i)
{
unsigned flags = multiReg->GetRegSpillFlagByIdx(i);
if ((flags & GTF_SPILL) != 0)
{
regNumber reg = multiReg->GetRegNumByIdx(i);
regSet.rsSpillTree(reg, multiReg, i);
gcInfo.gcMarkRegSetNpt(genRegMask(reg));
}
}
}
#endif // _TARGET_ARM_
else
{
Expand Down
131 changes: 131 additions & 0 deletions src/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,18 @@ struct GenTree
return OperIsPutArgStk() || OperIsPutArgReg() || OperIsPutArgSplit();
}

bool OperIsMultiRegOp() const
{
#if !defined(LEGACY_BACKEND) && defined(_TARGET_ARM_)
if (gtOper == GT_MUL_LONG || gtOper == GT_PUTARG_REG || gtOper == GT_COPY)
{
return true;
}
#endif

return false;
}

bool OperIsAddrMode() const
{
return OperIsAddrMode(OperGet());
Expand Down Expand Up @@ -3894,9 +3906,128 @@ struct GenTreeMultiRegOp : public GenTreeOp
{
regNumber gtOtherReg;

// GTF_SPILL or GTF_SPILLED flag on a multi-reg call node indicates that one or
// more of its result regs are in that state. The spill flag of each of the
// return register is stored here. We only need 2 bits per returned register,
// so this is treated as a 2-bit array. No architecture needs more than 8 bits.

static const unsigned PACKED_GTF_SPILL = 1;
static const unsigned PACKED_GTF_SPILLED = 2;
unsigned char gtSpillFlags;

GenTreeMultiRegOp(genTreeOps oper, var_types type, GenTreePtr op1, GenTreePtr op2)
: GenTreeOp(oper, type, op1, op2), gtOtherReg(REG_NA)
{
ClearOtherRegFlags();
}

//---------------------------------------------------------------------------
// GetRegNumByIdx: get ith register allocated to this struct argument.
//
// Arguments:
// idx - index of the register
//
// Return Value:
// Return regNumber of ith register of this register argument
//
regNumber GetRegNumByIdx(unsigned idx) const
{
assert(idx < 2);

if (idx == 0)
{
return gtRegNum;
}

return gtOtherReg;
}

//----------------------------------------------------------------------
// GetRegSpillFlagByIdx: get spill flag associated with the register
// specified by its index.
//
// Arguments:
// idx - Position or index of the register
//
// Return Value:
// Returns GTF_* flags associated with the register. Only GTF_SPILL and GTF_SPILLED are considered.
//
unsigned GetRegSpillFlagByIdx(unsigned idx) const
{
assert(idx < MAX_REG_ARG);

unsigned bits = gtSpillFlags >> (idx * 2); // It doesn't matter that we possibly leave other high bits here.
unsigned spillFlags = 0;
if (bits & PACKED_GTF_SPILL)
{
spillFlags |= GTF_SPILL;
}
if (bits & PACKED_GTF_SPILLED)
{
spillFlags |= GTF_SPILLED;
}

return spillFlags;
}

//----------------------------------------------------------------------
// SetRegSpillFlagByIdx: set spill flags for the register
// specified by its index.
//
// Arguments:
// flags - GTF_* flags. Only GTF_SPILL and GTF_SPILLED are allowed.
// idx - Position or index of the register
//
// Return Value:
// None
//
void SetRegSpillFlagByIdx(unsigned flags, unsigned idx)
{
assert(idx < MAX_REG_ARG);

unsigned bits = 0;
if (flags & GTF_SPILL)
{
bits |= PACKED_GTF_SPILL;
}
if (flags & GTF_SPILLED)
{
bits |= PACKED_GTF_SPILLED;
}

// Clear anything that was already there by masking out the bits before 'or'ing in what we want there.
gtSpillFlags = (unsigned char)((gtSpillFlags & ~(0xffU << (idx * 2))) | (bits << (idx * 2)));
}

//--------------------------------------------------------------------------
// GetRegType: Get var_type of the register specified by index.
//
// Arguments:
// index - Index of the register.
// First register will have an index 0 and so on.
//
// Return Value:
// var_type of the register specified by its index.

var_types GetRegType(unsigned index)
{
assert(index < 2);
var_types result = TYP_INT; // XXX
return result;
}

//-------------------------------------------------------------------
// clearOtherRegFlags: clear GTF_* flags associated with gtOtherRegs
//
// Arguments:
// None
//
// Return Value:
// None
//
void ClearOtherRegFlags()
{
gtSpillFlags = 0;
}

#if DEBUGGABLE_GENTREE
Expand Down
23 changes: 23 additions & 0 deletions src/jit/lsra.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3985,6 +3985,15 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree,
#endif // DEBUG

regMaskTP candidates = getUseCandidates(useNode);
#ifdef ARM_SOFTFP
// If oper is GT_PUTARG_REG, set bits in useCandidates must be in sequential order.
if (useNode->OperIsMultiRegOp())
{
regMaskTP candidate = genFindLowestReg(candidates);
useNode->gtLsraInfo.setSrcCandidates(this, candidates & ~candidate);
candidates = candidate;
}
#endif // ARM_SOFTFP
Copy link

Choose a reason for hiding this comment

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

I think this should more generally handle the case of useNode->IsMultiRegNode(). And it might be better to extract this into a separate method. I'm not crazy about modifying the srcCandidates on useNode, but there is probably not a better way that doesn't suffer from inefficiency.

assert((candidates & allRegs(i->registerType)) != 0);

// For non-localVar uses we record nothing, as nothing needs to be written back to the tree.
Expand Down Expand Up @@ -8907,6 +8916,15 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition)
ReturnTypeDesc* retTypeDesc = treeNode->AsCall()->GetReturnTypeDesc();
typ = retTypeDesc->GetReturnRegType(refPosition->getMultiRegIdx());
}
#ifdef ARM_SOFTFP
else if (treeNode->OperIsPutArgReg())
{
// For double arg regs, the type is changed to long since they must be passed via `r0-r3`.
// However when they get spilled, they should be treated as separated int registers.
var_types typNode = treeNode->TypeGet();
typ = (typNode == TYP_LONG) ? TYP_INT : typNode;
}
#endif // ARM_SOFTFP
else
{
typ = treeNode->TypeGet();
Expand Down Expand Up @@ -9230,6 +9248,11 @@ void LinearScan::resolveRegisters()
GenTreePutArgSplit* splitArg = treeNode->AsPutArgSplit();
splitArg->SetRegSpillFlagByIdx(GTF_SPILL, currentRefPosition->getMultiRegIdx());
}
else if (treeNode->OperIsMultiRegOp())
{
GenTreeMultiRegOp* multiReg = treeNode->AsMultiRegOp();
multiReg->SetRegSpillFlagByIdx(GTF_SPILL, currentRefPosition->getMultiRegIdx());
}
#endif
}

Expand Down
24 changes: 24 additions & 0 deletions src/jit/regset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,7 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 *
var_types treeType;
#if !defined(LEGACY_BACKEND) && defined(_TARGET_ARM_)
GenTreePutArgSplit* splitArg = nullptr;
GenTreeMultiRegOp* multiReg = nullptr;
#endif

#ifndef LEGACY_BACKEND
Expand All @@ -1548,6 +1549,11 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 *
splitArg = tree->AsPutArgSplit();
treeType = splitArg->GetRegType(regIdx);
}
else if (tree->OperIsMultiRegOp())
{
multiReg = tree->AsMultiRegOp();
treeType = multiReg->GetRegType(regIdx); // XXX check
}
#endif // _TARGET_ARM_
else
#endif // !LEGACY_BACKEND
Expand Down Expand Up @@ -1605,6 +1611,12 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 *
assert((regFlags & GTF_SPILL) != 0);
regFlags &= ~GTF_SPILL;
}
else if (multiReg != nullptr)
{
regFlags = multiReg->GetRegSpillFlagByIdx(regIdx);
assert((regFlags & GTF_SPILL) != 0);
regFlags &= ~GTF_SPILL;
}
#endif // _TARGET_ARM_
else
{
Expand Down Expand Up @@ -1759,6 +1771,11 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 *
regFlags |= GTF_SPILLED;
splitArg->SetRegSpillFlagByIdx(regFlags, regIdx);
}
else if (multiReg != nullptr)
{
regFlags |= GTF_SPILLED;
multiReg->SetRegSpillFlagByIdx(regFlags, regIdx);
}
#endif // _TARGET_ARM_
#endif //! LEGACY_BACKEND
}
Expand Down Expand Up @@ -2401,6 +2418,13 @@ TempDsc* RegSet::rsUnspillInPlace(GenTreePtr tree, regNumber oldReg, unsigned re
flags &= ~GTF_SPILLED;
splitArg->SetRegSpillFlagByIdx(flags, regIdx);
}
else if (tree->OperIsMultiRegOp())
{
GenTreeMultiRegOp* multiReg = tree->AsMultiRegOp();
unsigned flags = multiReg->GetRegSpillFlagByIdx(regIdx);
flags &= ~GTF_SPILLED;
multiReg->SetRegSpillFlagByIdx(flags, regIdx);
}
#endif // !LEGACY_BACKEND && _TARGET_ARM_
else
{
Expand Down