Skip to content

Commit

Permalink
Handle complex local addresses in block morphing
Browse files Browse the repository at this point in the history
In block morphing, "addrSpill" is used when the destination or source
represent indirections of "complex" addresses. Unfortunately, some trees
in the form of "IND(ADDR(LCL))" fall into this category.

If such an "ADDR(LCL)" is used as an "addrSpill", the underlying local
*must* be marked as address-exposed. Block morphing was using a very
simplistic test for when that needs to happen, essentially only recognizing
"ADDR(LCL_VAR/FLD)". But it is possible to have a more complicated pattern
as "PrepareDst/Src" uses "IsLocalAddrExpr" to recognize indirect stores
to locals.

Currently it appears impossible to get a mismatch here as morph transforms
"IND(ADD(ADDR(LCL_VAR), OFFSET))" into "LCL_FLD" (including for TYP_STRUCT
indirections), but this is a very fragile invariant. Transforming TYP_STRUCT
GT_FIELDs into GT_OBJs instead of GT_INDs breaks it, for example.

Fix this by address-exposing the local obtained via "IsLocalAddrExpr".
  • Loading branch information
SingleAccretion committed Jan 13, 2022
1 parent 64c05a3 commit 51fd539
Showing 1 changed file with 11 additions and 31 deletions.
42 changes: 11 additions & 31 deletions src/coreclr/jit/morphblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1048,9 +1048,9 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()
{
GenTreeOp* asgFields = nullptr;

GenTree* addrSpill = nullptr;
unsigned addrSpillTemp = BAD_VAR_NUM;
bool addrSpillIsStackDest = false; // true if 'addrSpill' represents the address in our local stack frame
GenTree* addrSpill = nullptr;
unsigned addrSpillSrcLclNum = BAD_VAR_NUM;
unsigned addrSpillTemp = BAD_VAR_NUM;

GenTree* addrSpillAsg = nullptr;

Expand Down Expand Up @@ -1095,7 +1095,8 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()
// We will spill m_srcAddr (i.e. assign to a temp "BlockOp address local")
// no need to clone a new copy as it is only used once
//
addrSpill = m_srcAddr; // addrSpill represents the 'm_srcAddr'
addrSpill = m_srcAddr; // addrSpill represents the 'm_srcAddr'
addrSpillSrcLclNum = m_srcLclNum;
}
}
}
Expand Down Expand Up @@ -1144,28 +1145,13 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()
// We will spill m_dstAddr (i.e. assign to a temp "BlockOp address local")
// no need to clone a new copy as it is only used once
//
addrSpill = m_dstAddr; // addrSpill represents the 'm_dstAddr'
addrSpill = m_dstAddr; // addrSpill represents the 'm_dstAddr'
addrSpillSrcLclNum = m_dstLclNum;
}
}
}
}

// TODO-CQ: this should be based on a more general
// "BaseAddress" method, that handles fields of structs, before or after
// morphing.
if ((addrSpill != nullptr) && addrSpill->OperIs(GT_ADDR))
{
GenTree* addrSpillOp = addrSpill->AsOp()->gtGetOp1();
if (addrSpillOp->IsLocal())
{
// We will *not* consider this to define the local, but rather have each individual field assign
// be a definition.
addrSpillOp->gtFlags &= ~(GTF_LIVENESS_MASK);
addrSpillIsStackDest = true; // addrSpill represents the address of LclVar[varNum] in our
// local stack frame
}
}

if (addrSpill != nullptr)
{
// 'addrSpill' is already morphed
Expand All @@ -1178,8 +1164,9 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()

addrSpillDsc->lvType = TYP_BYREF;

if (addrSpillIsStackDest)
if (addrSpillSrcLclNum != BAD_VAR_NUM)
{
// addrSpill represents the address of LclVar[varNum] in our local stack frame.
addrSpillDsc->lvStackByref = true;
}

Expand All @@ -1193,23 +1180,16 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()
// that we don't delete the definition for this LclVar
// as a dead store later on.
//
if (addrSpill->OperGet() == GT_ADDR)
if (addrSpillSrcLclNum != BAD_VAR_NUM)
{
GenTree* addrOp = addrSpill->AsOp()->gtOp1;
if (addrOp->IsLocal())
{
unsigned lclVarNum = addrOp->AsLclVarCommon()->GetLclNum();
m_comp->lvaGetDesc(lclVarNum)->SetAddressExposed(true DEBUGARG(AddressExposedReason::COPY_FLD_BY_FLD));
m_comp->lvaSetVarDoNotEnregister(lclVarNum DEBUGARG(DoNotEnregisterReason::AddrExposed));
}
m_comp->lvaSetVarAddrExposed(addrSpillSrcLclNum DEBUGARG(AddressExposedReason::COPY_FLD_BY_FLD));
}
}

// We may have allocated a temp above, and that may have caused the lvaTable to be expanded.
// So, beyond this point we cannot rely on the old values of 'm_srcVarDsc' and 'm_dstVarDsc'.
for (unsigned i = 0; i < fieldCnt; ++i)
{

GenTree* dstFld;
if (m_dstDoFldAsg)
{
Expand Down

0 comments on commit 51fd539

Please sign in to comment.