Skip to content

JIT: Model partial defs less conservatively in liveness when possible #115081

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
jakobbotsch merged 3 commits into from
Apr 30, 2025
+43 −19
Merged

JIT: Model partial defs less conservatively in liveness when possible #115081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8958f61
Port old change
jakobbotsch Apr 26, 2025
012e914
Clean up
jakobbotsch Apr 26, 2025
1e42d69
Update src/coreclr/jit/liveness.cpp
jakobbotsch Apr 27, 2025
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
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6720,6 +6720,7 @@ class Compiler

PhaseStatus fgEarlyLiveness();

template<bool ssaLiveness>
Copy link

Copilot AI Apr 27, 2025

Choose a reason for hiding this comment

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

Consider adding inline documentation to clarify the expected values of ssaLiveness and its impact on liveness modeling, ensuring that its usage is clear to future maintainers.

Copilot uses AI. Check for mistakes.
void fgMarkUseDef(GenTreeLclVarCommon* tree);

//-------------------------------------------------------------------------
Expand Down
61 changes: 42 additions & 19 deletions src/coreclr/jit/liveness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,27 @@
#endif
#include "lower.h" // for LowerRange()

/*****************************************************************************
*
* Helper for Compiler::fgPerBlockLocalVarLiveness().
* The goal is to compute the USE and DEF sets for a basic block.
*/
//------------------------------------------------------------------------
// fgMarkUseDef:
// Mark a local in the current def/use set.
//
// Parameters:
// tree - The local
//
// Template parameters:
// ssaLiveness - Whether the liveness computed is for SSA and should follow
// same modelling rules as SSA. SSA models partial defs like (v.x = 123) as
// (v = v with x = 123), which also implies that these partial definitions
// become uses. For dead-code elimination this is more conservative than
// needed, so outside SSA we do not model partial defs in this way:
//
// * In SSA: Partial defs are full defs but are also uses. They impact both
// bbVarUse and bbVarDef.
//
// * Outside SSA: Partial defs are _not_ full defs and are also not
// considered uses. They do not get included in bbVarUse/bbVarDef.
//
template <bool ssaLiveness>
void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree)
{
assert((tree->OperIsLocal() && (tree->OperGet() != GT_PHI_ARG)) || tree->OperIs(GT_LCL_ADDR));
Expand All @@ -35,8 +51,9 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree)
varDsc->setLvRefCnt(1);
}

const bool isDef = (tree->gtFlags & GTF_VAR_DEF) != 0;
const bool isUse = !isDef || ((tree->gtFlags & GTF_VAR_USEASG) != 0);
const bool isDef = ((tree->gtFlags & GTF_VAR_DEF) != 0);
const bool isFullDef = isDef && ((tree->gtFlags & GTF_VAR_USEASG) == 0);
const bool isUse = ssaLiveness ? !isFullDef : !isDef;
Copy link

Copilot AI Apr 27, 2025

Choose a reason for hiding this comment

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

Verify that the logic for computing the use set aligns with the intended liveness semantics: in SSA mode, only non-full defs are considered uses, whereas in non-SSA mode any def should not be treated as a use.

Copilot uses AI. Check for mistakes.

if (varDsc->lvTracked)
{
Expand All @@ -60,7 +77,7 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree)
VarSetOps::AddElemD(this, fgCurUseSet, varDsc->lvVarIndex);
}

if (isDef)
if (ssaLiveness ? isDef : isFullDef)
{
// This is a def, add it to the set of defs.
VarSetOps::AddElemD(this, fgCurDefSet, varDsc->lvVarIndex);
Expand Down Expand Up @@ -106,7 +123,7 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree)
VarSetOps::AddElemD(this, fgCurUseSet, varIndex);
}

if (isDef)
if (ssaLiveness ? isDef : isFullDef)
{
VarSetOps::AddElemD(this, fgCurDefSet, varIndex);
}
Expand All @@ -116,7 +133,10 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree)
}
}

/*****************************************************************************/
//------------------------------------------------------------------------
// fgLocalVarLiveness:
// Compute block def/use sets, liveness, and do dead code elimination.
//
void Compiler::fgLocalVarLiveness()
{
#ifdef DEBUG
Expand Down Expand Up @@ -216,7 +236,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree)
case GT_LCL_FLD:
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
fgMarkUseDef(tree->AsLclVarCommon());
fgMarkUseDef<!lowered>(tree->AsLclVarCommon());
Copy link

Copilot AI Apr 27, 2025

Choose a reason for hiding this comment

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

Ensure that the expression '!lowered' correctly reflects the intended SSA versus non-SSA evaluation and that 'lowered' is reliably defined and meaningful in this context.

Copilot uses AI. Check for mistakes.
break;

case GT_LCL_ADDR:
Expand All @@ -229,7 +249,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree)
break;
}

fgMarkUseDef(tree->AsLclVarCommon());
fgMarkUseDef<!lowered>(tree->AsLclVarCommon());
}
break;

Expand Down Expand Up @@ -325,7 +345,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree)
GenTreeLclVarCommon* definedLcl = gtCallGetDefinedRetBufLclAddr(call);
if (definedLcl != nullptr)
{
fgMarkUseDef(definedLcl);
fgMarkUseDef<!lowered>(definedLcl);
}
break;
}
Expand Down Expand Up @@ -356,7 +376,10 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTreeHWIntrinsic* hwintrinsic)
}
#endif // FEATURE_HW_INTRINSICS

/*****************************************************************************/
//------------------------------------------------------------------------
// fgPerBlockLocalVarLiveness:
// Compute def and use sets for the IR.
//
void Compiler::fgPerBlockLocalVarLiveness()
{
#ifdef DEBUG
Expand Down Expand Up @@ -419,7 +442,7 @@ void Compiler::fgPerBlockLocalVarLiveness()
{
for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList())
{
fgMarkUseDef(lcl);
fgMarkUseDef<false>(lcl);
}
}
else
Expand All @@ -436,12 +459,12 @@ void Compiler::fgPerBlockLocalVarLiveness()
// qmark arms.
for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList())
{
bool isUse = ((lcl->gtFlags & GTF_VAR_DEF) == 0) || ((lcl->gtFlags & GTF_VAR_USEASG) != 0);
bool isUse = (lcl->gtFlags & GTF_VAR_DEF) == 0;
// We can still handle the pure def at the top level.
bool conditional = lcl != dst;
if (isUse || !conditional)
{
fgMarkUseDef(lcl);
fgMarkUseDef<false>(lcl);
}
}
}
Expand All @@ -453,7 +476,7 @@ void Compiler::fgPerBlockLocalVarLiveness()
{
for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList())
{
fgMarkUseDef(lcl);
fgMarkUseDef<false>(lcl);
}
}
}
Expand Down Expand Up @@ -2087,7 +2110,7 @@ void Compiler::fgInterBlockLocalVarLiveness()
for (GenTree* cur = stmt->GetTreeListEnd(); cur != nullptr;)
{
assert(cur->OperIsAnyLocal());
bool isDef = ((cur->gtFlags & GTF_VAR_DEF) != 0) && ((cur->gtFlags & GTF_VAR_USEASG) == 0);
bool isDef = (cur->gtFlags & GTF_VAR_DEF) != 0;
Copy link

Copilot AI Apr 27, 2025

Choose a reason for hiding this comment

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

Confirm that removing the additional check for GTF_VAR_USEASG when computing isDef in fgInterBlockLocalVarLiveness is intentional and consistent with the new non-SSA liveness semantics.

Copilot uses AI. Check for mistakes.
bool conditional = cur != dst;
// Ignore conditional defs that would otherwise
// (incorrectly) interfere with liveness in other
Expand Down
Loading