Skip to content

Commit

Permalink
Refactor HasMultiRegRetVal and impFixupCallStructReturn. (#36465)
Browse files Browse the repository at this point in the history 
* Fix target definitions.

They were used in asserts only, no changes.

* Fix failures after a recent HW changes.

* Add a const getter for `ReturnTypeDesc` from a call.

Used to make some new methods const as well.

* Refactor `HasMultiRegRetVal` and `impFixupCallStructReturn`.

Delete an unnecessary nested condition and make checks more straightforward.

* Delete an extra `.` in some dumps.

* Add an additional check that `ReturnTypeDesc` is initialized.

* Remove old `const_cast` around `GetReturnTypeDesc`.

* Replace non-const `GetReturnTypeDesc` with other methods.

* Fix uninitialized `gtSpillFlags, gtOtherRegs, gtReturnTypeDesc` in `fgMorphIntoHelperCall`.
  • Loading branch information
Sergey Andreenko authored May 15, 2020
1 parent 85777ee commit f758569
Show file tree
Hide file tree
Showing 15 changed files with 204 additions and 200 deletions.
19 changes: 8 additions & 11 deletions src/coreclr/src/jit/codegenarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1394,8 +1394,8 @@ void CodeGen::genMultiRegCallStoreToLocal(GenTree* treeNode)

genConsumeRegs(op1);

ReturnTypeDesc* pRetTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = pRetTypeDesc->GetReturnRegCount();
const ReturnTypeDesc* pRetTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = pRetTypeDesc->GetReturnRegCount();

if (treeNode->GetRegNum() != REG_NA)
{
Expand Down Expand Up @@ -2610,9 +2610,9 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
}

// Determine return value size(s).
ReturnTypeDesc* pRetTypeDesc = call->GetReturnTypeDesc();
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;
const ReturnTypeDesc* pRetTypeDesc = call->GetReturnTypeDesc();
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;

if (call->HasMultiRegRetVal())
{
Expand Down Expand Up @@ -3880,12 +3880,9 @@ void CodeGen::genStructReturn(GenTree* treeNode)
GenTree* actualOp1 = op1->gtSkipReloadOrCopy();
GenTreeCall* call = actualOp1->AsCall();

ReturnTypeDesc* pRetTypeDesc;
unsigned regCount;
unsigned matchingCount = 0;

pRetTypeDesc = call->GetReturnTypeDesc();
regCount = pRetTypeDesc->GetReturnRegCount();
const ReturnTypeDesc* pRetTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = pRetTypeDesc->GetReturnRegCount();
unsigned matchingCount = 0;

var_types regType[MAX_RET_REG_COUNT];
regNumber returnReg[MAX_RET_REG_COUNT];
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11554,7 +11554,7 @@ void CodeGen::genReturn(GenTree* treeNode)
{
if (varTypeIsLong(compiler->info.compRetNativeType))
{
retTypeDesc.InitializeLongReturnType(compiler);
retTypeDesc.InitializeLongReturnType();
}
else // we must have a struct return type
{
Expand Down
28 changes: 14 additions & 14 deletions src/coreclr/src/jit/codegenlinear.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1081,10 +1081,10 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree)
}
else if (unspillTree->IsMultiRegCall())
{
GenTreeCall* call = unspillTree->AsCall();
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
GenTreeCopyOrReload* reloadTree = nullptr;
GenTreeCall* call = unspillTree->AsCall();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = retTypeDesc->GetReturnRegCount();
GenTreeCopyOrReload* reloadTree = nullptr;
if (tree->OperGet() == GT_RELOAD)
{
reloadTree = tree->AsCopyOrReload();
Expand Down Expand Up @@ -1904,9 +1904,9 @@ void CodeGen::genProduceReg(GenTree* tree)
// know which of its result regs needs to be spilled.
if (tree->IsMultiRegCall())
{
GenTreeCall* call = tree->AsCall();
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
GenTreeCall* call = tree->AsCall();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = retTypeDesc->GetReturnRegCount();

for (unsigned i = 0; i < regCount; ++i)
{
Expand Down Expand Up @@ -1987,9 +1987,9 @@ void CodeGen::genProduceReg(GenTree* tree)
// Mark all the regs produced by call node.
if (tree->IsMultiRegCall())
{
GenTreeCall* call = tree->AsCall();
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
const GenTreeCall* call = tree->AsCall();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = retTypeDesc->GetReturnRegCount();

for (unsigned i = 0; i < regCount; ++i)
{
Expand All @@ -2006,10 +2006,10 @@ void CodeGen::genProduceReg(GenTree* tree)

// A multi-reg GT_COPY node produces those regs to which
// copy has taken place.
GenTreeCopyOrReload* copy = tree->AsCopyOrReload();
GenTreeCall* call = copy->gtGetOp1()->AsCall();
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
const GenTreeCopyOrReload* copy = tree->AsCopyOrReload();
const GenTreeCall* call = copy->gtGetOp1()->AsCall();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = retTypeDesc->GetReturnRegCount();

for (unsigned i = 0; i < regCount; ++i)
{
Expand Down
28 changes: 14 additions & 14 deletions src/coreclr/src/jit/codegenxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,14 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg)
ReturnTypeDesc retTypeDesc;
if (varTypeIsLong(compiler->info.compRetNativeType))
{
retTypeDesc.InitializeLongReturnType(compiler);
retTypeDesc.InitializeLongReturnType();
}
else // we must have a struct return type
{
retTypeDesc.InitializeStructReturnType(compiler, compiler->info.compMethodInfo->args.retTypeClass);
}

unsigned regCount = retTypeDesc.GetReturnRegCount();
const unsigned regCount = retTypeDesc.GetReturnRegCount();

// Only x86 and x64 Unix ABI allows multi-reg return and
// number of result regs should be equal to MAX_RET_REG_COUNT.
Expand Down Expand Up @@ -1179,7 +1179,7 @@ void CodeGen::genStructReturn(GenTree* treeNode)

ReturnTypeDesc retTypeDesc;
retTypeDesc.InitializeStructReturnType(compiler, varDsc->lvVerTypeInfo.GetClassHandle());
unsigned regCount = retTypeDesc.GetReturnRegCount();
const unsigned regCount = retTypeDesc.GetReturnRegCount();
assert(regCount == MAX_RET_REG_COUNT);

if (varTypeIsEnregisterable(op1))
Expand Down Expand Up @@ -1243,10 +1243,10 @@ void CodeGen::genStructReturn(GenTree* treeNode)

genConsumeRegs(op1);

GenTree* actualOp1 = op1->gtSkipReloadOrCopy();
GenTreeCall* call = actualOp1->AsCall();
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
const GenTree* actualOp1 = op1->gtSkipReloadOrCopy();
const GenTreeCall* call = actualOp1->AsCall();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
const unsigned regCount = retTypeDesc->GetReturnRegCount();
assert(regCount == MAX_RET_REG_COUNT);

// Handle circular dependency between call allocated regs and ABI return regs.
Expand Down Expand Up @@ -2043,7 +2043,7 @@ void CodeGen::genMultiRegCallStoreToLocal(GenTree* treeNode)

genConsumeRegs(op1);

ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
assert(retTypeDesc->GetReturnRegCount() == MAX_RET_REG_COUNT);
unsigned regCount = retTypeDesc->GetReturnRegCount();

Expand Down Expand Up @@ -2154,8 +2154,8 @@ void CodeGen::genMultiRegCallStoreToLocal(GenTree* treeNode)

genConsumeRegs(op1);

ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
unsigned regCount = retTypeDesc->GetReturnRegCount();
assert(regCount == MAX_RET_REG_COUNT);

// Stack store
Expand Down Expand Up @@ -5490,9 +5490,9 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
}

// Determine return value size(s).
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;
const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;

if (call->HasMultiRegRetVal())
{
Expand Down Expand Up @@ -5755,7 +5755,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
if (call->HasMultiRegRetVal())
{
assert(retTypeDesc != nullptr);
unsigned regCount = retTypeDesc->GetReturnRegCount();
const unsigned regCount = retTypeDesc->GetReturnRegCount();

// If regs allocated to call node are different from ABI return
// regs in which the call has returned its result, move the result
Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/src/jit/decomposelongs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1362,19 +1362,15 @@ GenTree* DecomposeLongs::DecomposeShift(LIR::Use& use)

GenTreeCall::Use* argList = m_compiler->gtNewCallArgs(loOp1, hiOp1, shiftByOp);

GenTree* call = m_compiler->gtNewHelperCallNode(helper, TYP_LONG, argList);
GenTreeCall* call = m_compiler->gtNewHelperCallNode(helper, TYP_LONG, argList);
call->gtFlags |= shift->gtFlags & GTF_ALL_EFFECT;

if (shift->IsUnusedValue())
{
call->SetUnusedValue();
}

GenTreeCall* callNode = call->AsCall();
ReturnTypeDesc* retTypeDesc = callNode->GetReturnTypeDesc();
retTypeDesc->InitializeLongReturnType(m_compiler);

call = m_compiler->fgMorphArgs(callNode);
call = m_compiler->fgMorphArgs(call);
Range().InsertAfter(shift, LIR::SeqTree(m_compiler, call));

Range().Remove(shift);
Expand Down
83 changes: 29 additions & 54 deletions src/coreclr/src/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,15 +622,12 @@ void GenTree::CopyReg(GenTree* from)
//
bool GenTree::gtHasReg() const
{
bool hasReg;
bool hasReg = false;

if (IsMultiRegCall())
{
// Have to cast away const-ness because GetReturnTypeDesc() is a non-const method
GenTree* tree = const_cast<GenTree*>(this);
GenTreeCall* call = tree->AsCall();
unsigned regCount = call->GetReturnTypeDesc()->GetReturnRegCount();
hasReg = false;
const GenTreeCall* call = AsCall();
const unsigned regCount = call->GetReturnTypeDesc()->GetReturnRegCount();

// A Multi-reg call node is said to have regs, if it has
// reg assigned to each of its result registers.
Expand All @@ -645,11 +642,9 @@ bool GenTree::gtHasReg() const
}
else if (IsCopyOrReloadOfMultiRegCall())
{
GenTree* tree = const_cast<GenTree*>(this);
GenTreeCopyOrReload* copyOrReload = tree->AsCopyOrReload();
GenTreeCall* call = copyOrReload->gtGetOp1()->AsCall();
unsigned regCount = call->GetReturnTypeDesc()->GetReturnRegCount();
hasReg = false;
const GenTreeCopyOrReload* copyOrReload = AsCopyOrReload();
const GenTreeCall* call = copyOrReload->gtGetOp1()->AsCall();
const unsigned regCount = call->GetReturnTypeDesc()->GetReturnRegCount();

// A Multi-reg copy or reload node is said to have regs,
// if it has valid regs in any of the positions.
Expand Down Expand Up @@ -693,9 +688,7 @@ int GenTree::GetRegisterDstCount() const
}
else if (IsMultiRegCall())
{
// temporarily cast away const-ness as AsCall() method is not declared const
GenTree* temp = const_cast<GenTree*>(this);
return temp->AsCall()->GetReturnTypeDesc()->GetReturnRegCount();
return AsCall()->GetReturnTypeDesc()->GetReturnRegCount();
}
else if (IsCopyOrReload())
{
Expand Down Expand Up @@ -750,20 +743,18 @@ regMaskTP GenTree::gtGetRegMask() const
if (IsMultiRegCall())
{
// temporarily cast away const-ness as AsCall() method is not declared const
resultMask = genRegMask(GetRegNum());
GenTree* temp = const_cast<GenTree*>(this);
resultMask |= temp->AsCall()->GetOtherRegMask();
resultMask = genRegMask(GetRegNum());
resultMask |= AsCall()->GetOtherRegMask();
}
else if (IsCopyOrReloadOfMultiRegCall())
{
// A multi-reg copy or reload, will have valid regs for only those
// positions that need to be copied or reloaded. Hence we need
// to consider only those registers for computing reg mask.

GenTree* tree = const_cast<GenTree*>(this);
GenTreeCopyOrReload* copyOrReload = tree->AsCopyOrReload();
GenTreeCall* call = copyOrReload->gtGetOp1()->AsCall();
unsigned regCount = call->GetReturnTypeDesc()->GetReturnRegCount();
const GenTreeCopyOrReload* copyOrReload = AsCopyOrReload();
const GenTreeCall* call = copyOrReload->gtGetOp1()->AsCall();
const unsigned regCount = call->GetReturnTypeDesc()->GetReturnRegCount();

resultMask = RBM_NONE;
for (unsigned i = 0; i < regCount; ++i)
Expand All @@ -778,9 +769,8 @@ regMaskTP GenTree::gtGetRegMask() const
#if FEATURE_ARG_SPLIT
else if (OperIsPutArgSplit())
{
GenTree* tree = const_cast<GenTree*>(this);
GenTreePutArgSplit* splitArg = tree->AsPutArgSplit();
unsigned regCount = splitArg->gtNumRegs;
const GenTreePutArgSplit* splitArg = AsPutArgSplit();
const unsigned regCount = splitArg->gtNumRegs;

resultMask = RBM_NONE;
for (unsigned i = 0; i < regCount; ++i)
Expand Down Expand Up @@ -6182,21 +6172,14 @@ GenTreeCall* Compiler::gtNewCallNode(
// Initialize spill flags of gtOtherRegs
node->ClearOtherRegFlags();

#if defined(TARGET_X86) || defined(TARGET_ARM)
// Initialize the multi-reg long return info if necessary
#if !defined(TARGET_64BIT)
if (varTypeIsLong(node))
{
// The return type will remain as the incoming long type
node->gtReturnType = node->gtType;

assert(node->gtReturnType == node->gtType);
// Initialize Return type descriptor of call node
ReturnTypeDesc* retTypeDesc = node->GetReturnTypeDesc();
retTypeDesc->InitializeLongReturnType(this);

// must be a long returned in two registers
assert(retTypeDesc->GetReturnRegCount() == 2);
node->InitializeLongReturnType();
}
#endif // defined(TARGET_X86) || defined(TARGET_ARM)
#endif // !defined(TARGET_64BIT)

return node;
}
Expand Down Expand Up @@ -10217,18 +10200,18 @@ void Compiler::gtDispRegVal(GenTree* tree)
{
// 0th reg is GettRegNum(), which is already printed above.
// Print the remaining regs of a multi-reg call node.
GenTreeCall* call = tree->AsCall();
unsigned regCount = call->GetReturnTypeDesc()->TryGetReturnRegCount();
const GenTreeCall* call = tree->AsCall();
const unsigned regCount = call->GetReturnTypeDesc()->TryGetReturnRegCount();
for (unsigned i = 1; i < regCount; ++i)
{
printf(",%s", compRegVarName(call->GetRegNumByIdx(i)));
}
}
else if (tree->IsCopyOrReloadOfMultiRegCall())
{
GenTreeCopyOrReload* copyOrReload = tree->AsCopyOrReload();
GenTreeCall* call = tree->gtGetOp1()->AsCall();
unsigned regCount = call->GetReturnTypeDesc()->TryGetReturnRegCount();
const GenTreeCopyOrReload* copyOrReload = tree->AsCopyOrReload();
const GenTreeCall* call = tree->gtGetOp1()->AsCall();
const unsigned regCount = call->GetReturnTypeDesc()->TryGetReturnRegCount();
for (unsigned i = 1; i < regCount; ++i)
{
printf(",%s", compRegVarName(copyOrReload->GetRegNumByIdx(i)));
Expand Down Expand Up @@ -15315,9 +15298,7 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr,
#if FEATURE_MULTIREG_RET
if (varTypeIsStruct(call))
{
// Initialize Return type descriptor of call node.
ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc();
retTypeDesc->InitializeStructReturnType(this, structType);
call->InitializeStructReturnType(this, structType);
}
#endif // FEATURE_MULTIREG_RET

Expand Down Expand Up @@ -16529,7 +16510,7 @@ bool GenTree::isContained() const
else if (OperKind() & GTK_RELOP)
{
// We have to cast away const-ness since AsOp() method is non-const.
GenTree* childNode = const_cast<GenTree*>(this)->AsOp()->gtOp1;
const GenTree* childNode = AsOp()->gtGetOp1();
assert((isMarkedContained == false) || childNode->IsSIMDEqualityOrInequality());
}

Expand Down Expand Up @@ -18909,16 +18890,10 @@ void ReturnTypeDesc::InitializeStructReturnType(Compiler* comp, CORINFO_CLASS_HA
// InitializeLongReturnType:
// Initialize the Return Type Descriptor for a method that returns a TYP_LONG
//
// Arguments
// comp - Compiler Instance
//
// Return Value
// None
//
void ReturnTypeDesc::InitializeLongReturnType(Compiler* comp)
void ReturnTypeDesc::InitializeLongReturnType()
{
assert(!m_inited);
#if defined(TARGET_X86) || defined(TARGET_ARM)

// Setups up a ReturnTypeDesc for returning a long using two registers
//
assert(MAX_RET_REG_COUNT >= 2);
Expand Down Expand Up @@ -18950,7 +18925,7 @@ void ReturnTypeDesc::InitializeLongReturnType(Compiler* comp)
// x86 and ARM return long in multiple registers.
// ARM and ARM64 return HFA struct in multiple registers.
//
regNumber ReturnTypeDesc::GetABIReturnReg(unsigned idx)
regNumber ReturnTypeDesc::GetABIReturnReg(unsigned idx) const
{
unsigned count = GetReturnRegCount();
assert(idx < count);
Expand Down Expand Up @@ -19079,7 +19054,7 @@ regNumber ReturnTypeDesc::GetABIReturnReg(unsigned idx)
// of return registers and wants to know the set of return registers.
//
// static
regMaskTP ReturnTypeDesc::GetABIReturnRegs()
regMaskTP ReturnTypeDesc::GetABIReturnRegs() const
{
regMaskTP resultMask = RBM_NONE;

Expand Down
Loading

0 comments on commit f758569

Please sign in to comment.