Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not generate TYP_STRUCT LCL_FLD nodes #66251

Merged
merged 4 commits into from
Mar 30, 2022
Merged

Do not generate TYP_STRUCT LCL_FLD nodes #66251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83a3f3c
Do not create TYP_STRUCT LCL_FLD
SingleAccretion Mar 5, 2022
202fc54
Fix regressions
SingleAccretion Mar 5, 2022
acf22a0
More fixes for regressions
SingleAccretion Mar 7, 2022
dba8765
Add a comment
SingleAccretion Mar 29, 2022
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
29 changes: 20 additions & 9 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7001,16 +7001,9 @@ void Compiler::gtBlockOpInit(GenTree* result, GenTree* dst, GenTree* srcOrFillVa
GenTree* Compiler::gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVolatile, bool isCopyBlock)
{
assert(dst->OperIsBlk() || dst->OperIsLocal());
if (isCopyBlock)
{
if (srcOrFillVal->OperIsIndir() && (srcOrFillVal->gtGetOp1()->gtOper == GT_ADDR))
{
srcOrFillVal = srcOrFillVal->gtGetOp1()->gtGetOp1();
}
}
else

if (!isCopyBlock) // InitBlk
{
// InitBlk
assert(varTypeIsIntegral(srcOrFillVal));
if (varTypeIsStruct(dst))
{
Expand Down Expand Up @@ -15648,8 +15641,15 @@ bool GenTree::IsLocalAddrExpr(Compiler* comp,
{
assert(!comp->compRationalIRForm);
GenTree* addrArg = AsOp()->gtOp1;

if (addrArg->IsLocal()) // Note that this covers "GT_LCL_FLD."
{
FieldSeqNode* zeroOffsetFieldSeq = nullptr;
if (comp->GetZeroOffsetFieldMap()->Lookup(this, &zeroOffsetFieldSeq))
{
*pFldSeq = comp->GetFieldSeqStore()->Append(zeroOffsetFieldSeq, *pFldSeq);
}

*pLclVarTree = addrArg->AsLclVarCommon();
if (addrArg->OperGet() == GT_LCL_FLD)
{
Expand Down Expand Up @@ -22218,6 +22218,17 @@ uint16_t GenTreeLclVarCommon::GetLclOffs() const
}
}

//------------------------------------------------------------------------
// GetFieldSeq: Get the field sequence for this local node.
//
// Return Value:
// The sequence of the node for local fields, empty ("nullptr") otherwise.
//
FieldSeqNode* GenTreeLclVarCommon::GetFieldSeq() const
{
return OperIsLocalField() ? AsLclFld()->GetFieldSeq() : nullptr;
}

#if defined(TARGET_XARCH) && defined(FEATURE_HW_INTRINSICS)
//------------------------------------------------------------------------
// GetResultOpNumForFMA: check if the result is written into one of the operands.
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3330,6 +3330,8 @@ struct GenTreeLclVarCommon : public GenTreeUnOp
return (GetSsaNum() != SsaConfig::RESERVED_SSA_NUM);
}

FieldSeqNode* GetFieldSeq() const;

#if DEBUGGABLE_GENTREE
GenTreeLclVarCommon() : GenTreeUnOp()
{
Expand Down
63 changes: 48 additions & 15 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12440,7 +12440,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
ival1 = 0;

// Don't remove a volatile GT_IND, even if the address points to a local variable.
if ((tree->gtFlags & GTF_IND_VOLATILE) == 0)
// For TYP_STRUCT INDs, we do not know their size, and so will not morph as well.
if (!tree->AsIndir()->IsVolatile() && !tree->TypeIs(TYP_STRUCT))
{
/* Try to Fold *(&X) into X */
if (op1->gtOper == GT_ADDR)
Expand All @@ -12453,13 +12454,9 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)

temp = op1->AsOp()->gtOp1; // X

// In the test below, if they're both TYP_STRUCT, this of course does *not* mean that
// they are the *same* struct type. In fact, they almost certainly aren't. If the
// address has an associated field sequence, that identifies this case; go through
// the "lcl_fld" path rather than this one.
FieldSeqNode* addrFieldSeq = nullptr; // This is an unused out parameter below.
if (typ == temp->TypeGet() && !GetZeroOffsetFieldMap()->Lookup(op1, &addrFieldSeq))
if (typ == temp->TypeGet())
{
assert(typ != TYP_STRUCT);
foldAndReturnTemp = true;
}
else if (temp->OperIsLocal())
Expand Down Expand Up @@ -12635,22 +12632,17 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
// out-of-bounds w.r.t. the local).
if ((temp != nullptr) && !foldAndReturnTemp)
{
assert(temp->OperIsLocal());
assert(temp->OperIsLocalRead());

const unsigned lclNum = temp->AsLclVarCommon()->GetLclNum();
LclVarDsc* const varDsc = lvaGetDesc(lclNum);

const var_types tempTyp = temp->TypeGet();
const bool useExactSize = varTypeIsStruct(tempTyp) || (tempTyp == TYP_BLK) || (tempTyp == TYP_LCLBLK);
const unsigned varSize = useExactSize ? varDsc->lvExactSize : genTypeSize(temp);
unsigned lclNum = temp->AsLclVarCommon()->GetLclNum();

// Make sure we do not enregister this lclVar.
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::LocalField));

// If the size of the load is greater than the size of the lclVar, we cannot fold this access into
// a lclFld: the access represented by an lclFld node must begin at or after the start of the
// lclVar and must not extend beyond the end of the lclVar.
if ((ival1 >= 0) && ((ival1 + genTypeSize(typ)) <= varSize))
if ((ival1 >= 0) && ((ival1 + genTypeSize(typ)) <= lvaLclExactSize(lclNum)))
{
GenTreeLclFld* lclFld;

Expand Down Expand Up @@ -13740,6 +13732,47 @@ GenTree* Compiler::fgOptimizeAddition(GenTreeOp* add)
return op1;
}

// Reduce local addresses: ADD(ADDR(LCL_VAR), OFFSET) => ADDR(LCL_FLD OFFSET).
// TODO-ADDR: do ADD(LCL_FLD/VAR_ADDR, OFFSET) => LCL_FLD_ADDR instead.
//
if (opts.OptimizationEnabled() && fgGlobalMorph && op1->OperIs(GT_ADDR) && op2->IsCnsIntOrI() &&
op1->AsUnOp()->gtGetOp1()->OperIs(GT_LCL_VAR, GT_LCL_FLD))
{
GenTreeUnOp* addrNode = op1->AsUnOp();
GenTreeLclVarCommon* lclNode = addrNode->gtGetOp1()->AsLclVarCommon();
GenTreeIntCon* offsetNode = op2->AsIntCon();
if (FitsIn<uint16_t>(offsetNode->IconValue()))
{
unsigned offset = lclNode->GetLclOffs() + static_cast<uint16_t>(offsetNode->IconValue());

// Note: the emitter does not expect out-of-bounds access for LCL_FLD_ADDR.
if (FitsIn<uint16_t>(offset) && (offset < lvaLclExactSize(lclNode->GetLclNum())))
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
{
// Compose the field sequence: [LCL, ADDR, OFFSET].
FieldSeqNode* fieldSeq = lclNode->GetFieldSeq();
FieldSeqNode* zeroOffsetFieldSeq = nullptr;
if (GetZeroOffsetFieldMap()->Lookup(addrNode, &zeroOffsetFieldSeq))
{
fieldSeq = GetFieldSeqStore()->Append(fieldSeq, zeroOffsetFieldSeq);
GetZeroOffsetFieldMap()->Remove(addrNode);
}
fieldSeq = GetFieldSeqStore()->Append(fieldSeq, offsetNode->gtFieldSeq);

// Types of location nodes under ADDRs do not matter. We arbitrarily choose TYP_UBYTE.
lclNode->ChangeType(TYP_UBYTE);
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
lclNode->SetOper(GT_LCL_FLD);
lclNode->AsLclFld()->SetLclOffs(offset);
lclNode->AsLclFld()->SetFieldSeq(fieldSeq);
lvaSetVarDoNotEnregister(lclNode->GetLclNum() DEBUGARG(DoNotEnregisterReason::LocalField));

DEBUG_DESTROY_NODE(offsetNode);
DEBUG_DESTROY_NODE(add);

return addrNode;
}
}
}

// Note that these transformations are legal for floating-point ADDs as well.
if (opts.OptimizationEnabled())
{
Expand Down