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

Address-expose locals under complex local addresses in block morphing #63100

Merged
merged 2 commits into from
Jan 21, 2022
Merged

Address-expose locals under complex local addresses in block morphing #63100

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51fd539
Handle complex local addresses in block morphing
SingleAccretion Dec 23, 2021
772a91c
Add a TODO-CQ for LCL_FLD usage
SingleAccretion Jan 18, 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
53 changes: 17 additions & 36 deletions src/coreclr/jit/morphblock.cpp
View file
Open in desktop
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was deleted as it was redundant with the code above.

addrSpillIsStackDest = true; // addrSpill represents the address of LclVar[varNum] in our
// local stack frame
}
}

if (addrSpill != nullptr)
{
// 'addrSpill' is already morphed
Expand All @@ -1178,38 +1164,33 @@ 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;
}

GenTreeLclVar* addrSpillNode = m_comp->gtNewLclvNode(addrSpillTemp, TYP_BYREF);
addrSpillAsg = m_comp->gtNewAssignNode(addrSpillNode, addrSpill);

// If we are assigning the address of a LclVar here
// liveness does not account for this kind of address taken use.
// If we are assigning the address of a LclVar here liveness will not
// account for this kind of address taken use. Mark the local as
// address-exposed so that we don't do illegal optimizations with it.
//
// We have to mark this local as address exposed so
// that we don't delete the definition for this LclVar
// as a dead store later on.
// TODO-CQ: usage of "addrSpill" for local addresses is a workaround
// for cases where we fail to use LCL_FLD nodes instead. Fix them and
// delete this code.
//
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));
Copy link
Contributor Author

@SingleAccretion SingleAccretion Dec 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

lvaSetVarAddrExposed calls lvaSetVarDoNotEnregister.

}
}

// 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