Skip to content

Commit

Permalink
Don't mark LCL_VAR that is used in RETURN(IND(ADDR(LCL_VAR)) as addre…
Browse files Browse the repository at this point in the history
…ss taken when possible.

Protect against a promoted struct with a hole like struct<8> {hole 4; int a;};
  • Loading branch information
Sergey Andreenko committed Jun 29, 2020
1 parent 06e7c52 commit f53bb30
Show file tree
Hide file tree
Showing 8 changed files with 287 additions and 106 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ void CodeGen::genCodeForBitCast(GenTreeOp* treeNode)
JITDUMP("Changing type of BITCAST source to load directly.");
genCodeForTreeNode(op1);
}
else if (varTypeIsFloating(treeNode) != varTypeIsFloating(op1))
else if (varTypeUsesFloatReg(treeNode) != varTypeUsesFloatReg(op1))
{
regNumber srcReg = op1->GetRegNum();
assert(genTypeSize(op1->TypeGet()) == genTypeSize(targetType));
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,8 @@ class LclVarDsc
return GetLayout()->GetRegisterType();
}

bool CanBeReplacedWithItsField(Compiler* comp) const;

#ifdef DEBUG
public:
const char* lvReason;
Expand Down Expand Up @@ -5602,6 +5604,7 @@ class Compiler
GenTree* fgMorphCopyBlock(GenTree* tree);
GenTree* fgMorphForRegisterFP(GenTree* tree);
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac = nullptr);
GenTree* fgMorphRetInd(GenTreeUnOp* tree);
GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree);
GenTree* fgMorphSmpOpOptional(GenTreeOp* tree);

Expand Down Expand Up @@ -9179,7 +9182,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

// Returns true if the method returns a value in more than one return register,
// it should replace/be merged with compMethodReturnsMultiRegRetType when #36868 is fixed.
// The difference from original `compMethodReturnsMultiRegRetType` is in ARM64 SIMD16 handling,
// The difference from original `compMethodReturnsMultiRegRetType` is in ARM64 SIMD* handling,
// this method correctly returns false for it (it is passed as HVA), when the original returns true.
bool compMethodReturnsMultiRegRegTypeAlternate()
{
Expand All @@ -9189,8 +9192,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
return varTypeIsLong(info.compRetNativeType);
#else // targets: X64-UNIX, ARM64 or ARM32
#if defined(TARGET_ARM64)
// TYP_SIMD16 is returned in one register.
if (info.compRetNativeType == TYP_SIMD16)
// TYP_SIMD* are returned in one register.
if (varTypeIsSIMD(info.compRetNativeType))
{
return false;
}
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ void Compiler::fgInit()
fgCurBBEpochSize = 0;
fgBBSetCountInSizeTUnits = 0;

genReturnBB = nullptr;
genReturnBB = nullptr;
genReturnLocal = BAD_VAR_NUM;

/* We haven't reached the global morphing phase */
fgGlobalMorph = false;
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/src/jit/lclmorph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -536,12 +536,13 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
// be merged separatly.
GenTreeLclVar* lclVar = retVal->AsLclVar();
unsigned lclNum = lclVar->GetLclNum();
if (!m_compiler->lvaIsImplicitByRefLocal(lclVar->GetLclNum()))
if (!m_compiler->compMethodReturnsMultiRegRegTypeAlternate() &&
!m_compiler->lvaIsImplicitByRefLocal(lclVar->GetLclNum()))
{
LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum);
if (varDsc->lvFieldCnt > 1)
{
m_compiler->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_IsStructArg));
m_compiler->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_BlockOp));
}
}
}
Expand Down
50 changes: 49 additions & 1 deletion src/coreclr/src/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1974,7 +1974,8 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
shouldPromote = false;
}
}
else if (!compiler->compDoOldStructRetyping() && (lclNum == compiler->genReturnLocal) && (structPromotionInfo.fieldCnt > 1))
else if (!compiler->compDoOldStructRetyping() && (lclNum == compiler->genReturnLocal) &&
(structPromotionInfo.fieldCnt > 1))
{
// TODO-1stClassStructs: a temporary solution to keep diffs small, it will be fixed later.
shouldPromote = false;
Expand Down Expand Up @@ -3645,6 +3646,53 @@ var_types LclVarDsc::lvaArgType()
return type;
}

//----------------------------------------------------------------------------------------------
// CanBeReplacedWithItsField: check if a whole struct reference could be replaced by a field.
//
// Arguments:
// comp - the compiler instance;
//
// Return Value:
// true if that can be replaced, false otherwise.
//
// Notes:
// The replacement can be made only for independently promoted structs
// with 1 field without holes.
//
bool LclVarDsc::CanBeReplacedWithItsField(Compiler* comp) const
{
if (!lvPromoted)
{
return false;
}

if (comp->lvaGetPromotionType(this) != Compiler::PROMOTION_TYPE_INDEPENDENT)
{
return false;
}
if (lvFieldCnt != 1)
{
return false;
}
if (lvContainsHoles)
{
return false;
}

#if defined(FEATURE_SIMD)
// If we return `struct A { SIMD16 a; }` we split the struct into several fields.
// In order to do that we have to have its field in memory. Right now lowering cannot
// handle RETURN struct(multiple registers)->SIMD16(one register), but it can be improved.
LclVarDsc* fieldDsc = comp->lvaGetDesc(lvFieldLclStart);
if (fieldDsc->TypeGet() == TYP_SIMD12 || fieldDsc->TypeGet() == TYP_SIMD16)
{
return false;
}
#endif // FEATURE_SIMD

return true;
}

//------------------------------------------------------------------------
// lvaMarkLclRefs: increment local var references counts and more
//
Expand Down
Loading

0 comments on commit f53bb30

Please sign in to comment.