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

JIT: Account for GT_JMP implicit uses in local morph ref counting #80734

Merged
merged 6 commits into from
Jan 18, 2023
Merged

JIT: Account for GT_JMP implicit uses in local morph ref counting #80734

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ba39b89
JIT: Account for GT_JMP implicit uses in local morph ref counting
jakobbotsch Jan 17, 2023
e533199
Remove leftover file
jakobbotsch Jan 17, 2023
59f38c6
Exclude new test under llvmfullaot
jakobbotsch Jan 17, 2023
b6b6993
Use a switch in LocalAddressVisitor::PreOrderVisit
jakobbotsch Jan 17, 2023
5ef4ef2
Check for GT_JMP once per block instead
jakobbotsch Jan 17, 2023
0e6a55d
Oops
jakobbotsch Jan 17, 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
75 changes: 42 additions & 33 deletions src/coreclr/jit/lclmorph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,49 +569,58 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
{
GenTree* const node = *use;

if (node->OperIs(GT_IND, GT_FIELD, GT_FIELD_ADDR))
switch (node->gtOper)
{
MorphStructField(node, user);
}
else if (node->OperIs(GT_LCL_FLD))
{
MorphLocalField(node, user);
}
case GT_IND:
case GT_FIELD:
case GT_FIELD_ADDR:
MorphStructField(node, user);
break;
case GT_LCL_FLD:
MorphLocalField(node, user);
assert(node->OperIsLocal());
__fallthrough;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
__fallthrough;
FALLTHROUGH;

(We use the macro version everywhere else)

Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up reverting this because the case above also can potentially morph to a local, so also needs some more handling. There is a BasicBlock helper already to check for GT_JMP that it uses now.

Copy link
Contributor

@SingleAccretion SingleAccretion Jan 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Just for my understanding: why not use compJmpOpUsed?

(Multiple JMPs?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes (BasicBlock::endsWithJmpMethod does use compJmpOpUsed as an early-out)

case GT_LCL_VAR:
case GT_LCL_VAR_ADDR:
case GT_LCL_FLD_ADDR:
{
unsigned const lclNum = node->AsLclVarCommon()->GetLclNum();
LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum);

if (node->OperIsLocal() || node->OperIsLocalAddr())
{
unsigned const lclNum = node->AsLclVarCommon()->GetLclNum();
LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum);
UpdateEarlyRefCount(lclNum);

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

if (varDsc->lvIsStructField)
{
// Promoted field, increase count for the parent lclVar.
//
assert(!m_compiler->lvaIsImplicitByRefLocal(lclNum));
unsigned parentLclNum = varDsc->lvParentLcl;
UpdateEarlyRefCount(parentLclNum);
if (varDsc->lvPromoted)
{
// Promoted struct, increase count for each promoted field.
//
for (unsigned childLclNum = varDsc->lvFieldLclStart;
childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum)
{
UpdateEarlyRefCount(childLclNum);
}
}

break;
}

if (varDsc->lvPromoted)
case GT_JMP:
{
// Promoted struct, increase count for each promoted field.
//
for (unsigned childLclNum = varDsc->lvFieldLclStart;
childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum)
// GT_JMP has implicit uses of all arguments.
for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++)
{
UpdateEarlyRefCount(childLclNum);
UpdateEarlyRefCount(lclNum);
}
}
}

if (node->OperIs(GT_JMP))
{
// GT_JMP has implicit uses of all arguments.
for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++)
{
UpdateEarlyRefCount(lclNum);
break;
}
}

Expand Down