Skip to content

Delete OBJ(LCL_ADDR) args handling from local morph ref counting #85544

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

Merged
AndyAyersMS merged 2 commits into from
Apr 30, 2023
Merged

Delete OBJ(LCL_ADDR) args handling from local morph ref counting #85544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9e7a42
Delete OBJ args handling from local morph
SingleAccretion Apr 28, 2023
4443cef
Delete outdated comments
SingleAccretion Apr 30, 2023
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
55 changes: 9 additions & 46 deletions src/coreclr/jit/lclmorph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
{
DoPreOrder = true,
DoPostOrder = true,
ComputeStack = true,
DoLclVarsOnly = false,
UseExecutionOrder = true,
};
Expand Down Expand Up @@ -569,15 +568,15 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
unsigned const lclNum = node->AsLclVarCommon()->GetLclNum();
LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum);

UpdateEarlyRefCount(lclNum);
UpdateEarlyRefCount(lclNum, node, user);

if (varDsc->lvIsStructField)
{
// Promoted field, increase count for the parent lclVar.
//
assert(!m_compiler->lvaIsImplicitByRefLocal(lclNum));
unsigned parentLclNum = varDsc->lvParentLcl;
UpdateEarlyRefCount(parentLclNum);
UpdateEarlyRefCount(parentLclNum, node, user);
}

if (varDsc->lvPromoted)
Expand All @@ -587,7 +586,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
for (unsigned childLclNum = varDsc->lvFieldLclStart;
childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum)
{
UpdateEarlyRefCount(childLclNum);
UpdateEarlyRefCount(childLclNum, node, user);
}
}
}
Expand Down Expand Up @@ -1535,14 +1534,14 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
//
// Arguments:
// lclNum - the local number to update the count for.
// node - local node representing the reference
// user - "node"'s user
//
// Notes:
// fgMakeOutgoingStructArgCopy checks the ref counts for implicit byref params when it decides
// if it's legal to elide certain copies of them;
// fgRetypeImplicitByRefArgs checks the ref counts when it decides to undo promotions.
// fgForwardSub uses ref counts to decide when to forward sub.
// fgForwardSub may use ref counts to decide when to forward sub.
//
void UpdateEarlyRefCount(unsigned lclNum)
void UpdateEarlyRefCount(unsigned lclNum, GenTree* node, GenTree* user)
{
LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum);

Expand All @@ -1566,43 +1565,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
// But the pattern should at least subset the implicit byref cases that are
// handled in fgCanFastTailCall and fgMakeOutgoingStructArgCopy.
//
// CALL(OBJ(LCL_VAR_ADDR...))
// -or-
// CALL(LCL_VAR)

// TODO-1stClassStructs: We've removed most, but not all, cases where OBJ(LCL_VAR_ADDR)
// is introduced (it was primarily from impNormStructVal). But until all cases are gone
// we still want to handle it as well.

if (m_ancestors.Height() < 2)
{
return;
}

GenTree* node = m_ancestors.Top(0);

if (node->OperIs(GT_LCL_VAR))
{
node = m_ancestors.Top(1);
}
else if (node->IsLclVarAddr())
{
if (m_ancestors.Height() < 3)
{
return;
}

node = m_ancestors.Top(1);

if (!node->OperIs(GT_BLK))
{
return;
}

node = m_ancestors.Top(2);
}

if (node->IsCall())
if ((node != nullptr) && node->OperIs(GT_LCL_VAR) && (user != nullptr) && user->IsCall())
{
JITDUMP("LocalAddressVisitor incrementing weighted ref count from " FMT_WT " to " FMT_WT
" for implicit by-ref V%02d arg passed to call\n",
Expand Down Expand Up @@ -1714,7 +1677,7 @@ PhaseStatus Compiler::fgMarkAddressExposedLocals()
// GT_JMP has implicit uses of all arguments.
for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++)
{
visitor.UpdateEarlyRefCount(lclNum);
visitor.UpdateEarlyRefCount(lclNum, nullptr, nullptr);
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3947,9 +3947,8 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)

if (lcl != nullptr)
{
const unsigned varNum = lcl->GetLclNum();
LclVarDsc* const varDsc = lvaGetDesc(varNum);
const unsigned short totalAppearances = varDsc->lvRefCnt(RCS_EARLY);
const unsigned varNum = lcl->GetLclNum();
LclVarDsc* const varDsc = lvaGetDesc(varNum);

// We generally use liveness to figure out if we can omit creating
// this copy. However, even without liveness (e.g. due to too many
Expand All @@ -3963,9 +3962,6 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)
// alias analysis of the call's parameters, or checking if the call
// site is not within some try region.
//
// * (may not copy) if there is exactly one use of the local in the method,
// and the call is not in loop, this is a last use.
//
bool omitCopy = call->IsTailCall();

if (!omitCopy && fgGlobalMorph)
Expand Down