Skip to content

JIT: also run local assertion prop in postorder during morph #115626

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
merged 4 commits into from
May 16, 2025
Merged

JIT: also run local assertion prop in postorder during morph #115626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
686e628
proof of concept
AndyAyersMS May 15, 2025
5d445ad
only run during global morph
AndyAyersMS May 15, 2025
26e3fd9
disable local ap after global morph
AndyAyersMS May 15, 2025
ef31d97
init apLocal; add config
AndyAyersMS May 16, 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 @@ -7708,6 +7708,7 @@ class Compiler
BitVecTraits* apTraits;
ASSERT_TP apFull;
ASSERT_TP apLocal;
ASSERT_TP apLocalPostorder;
ASSERT_TP apLocalIfTrue;

enum optAssertionKind : uint8_t
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,9 @@ RELEASE_CONFIG_INTEGER(JitEnablePhysicalPromotion, "JitEnablePhysicalPromotion",
// Enable cross-block local assertion prop
RELEASE_CONFIG_INTEGER(JitEnableCrossBlockLocalAssertionProp, "JitEnableCrossBlockLocalAssertionProp", 1)

// Enable postorder local assertion prop
RELEASE_CONFIG_INTEGER(JitEnablePostorderLocalAssertionProp, "JitEnablePostorderLocalAssertionProp", 1)

// Enable strength reduction
RELEASE_CONFIG_INTEGER(JitEnableStrengthReduction, "JitEnableStrengthReduction", 1)

Expand Down
73 changes: 60 additions & 13 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7106,6 +7106,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
if (optLocalAssertionProp)
{
isQmarkColon = true;
BitVecOps::ClearD(apTraits, apLocalPostorder);
}
break;

Expand Down Expand Up @@ -7746,6 +7747,40 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
qmarkOp2 = oldTree->AsOp()->gtOp2->AsOp()->gtOp2;
}

// During global morph, give assertion prop another shot at this tree.
//
// We need to use the "postorder" assertion set here, because apLocal
// may reflect results from subtrees that have since been reordered.
//
// apLocalPostorder only includes live assertions from prior statements.
//
if (fgGlobalMorph && optLocalAssertionProp && (optAssertionCount > 0))
{
GenTree* optimizedTree = tree;
bool again = JitConfig.JitEnablePostorderLocalAssertionProp() > 0;
bool didOptimize = false;

if (!again)
{
JITDUMP("*** Postorder assertion prop disabled by config\n");
}

while (again)
{
tree = optimizedTree;
optimizedTree = optAssertionProp(apLocalPostorder, tree, nullptr, nullptr);
again = (optimizedTree != nullptr);
didOptimize |= again;
}

assert(tree != nullptr);

if (didOptimize)
{
gtUpdateNodeSideEffects(tree);
}
}

// Try to fold it, maybe we get lucky,
tree = gtFoldExpr(tree);

Expand Down Expand Up @@ -11718,13 +11753,12 @@ void Compiler::fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree*
{
// Active dependent assertions are killed here
//
ASSERT_TP killed = BitVecOps::MakeCopy(apTraits, GetAssertionDep(lclNum));
BitVecOps::IntersectionD(apTraits, killed, apLocal);

if (killed)
{
ASSERT_TP killed = GetAssertionDep(lclNum);

#ifdef DEBUG
bool hasKills = !BitVecOps::IsEmptyIntersection(apTraits, apLocal, killed);
if (hasKills)
{
AssertionIndex index = optAssertionCount;
while (killed && (index > 0))
{
Expand All @@ -11744,10 +11778,11 @@ void Compiler::fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree*

index--;
}
}
#endif

BitVecOps::DiffD(apTraits, apLocal, killed);
}
BitVecOps::DiffD(apTraits, apLocal, killed);
BitVecOps::DiffD(apTraits, apLocalPostorder, killed);
}

//------------------------------------------------------------------------
Expand All @@ -11764,7 +11799,7 @@ void Compiler::fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree*
//
void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree))
{
if (BitVecOps::IsEmpty(apTraits, apLocal))
if (BitVecOps::IsEmpty(apTraits, apLocal) && BitVecOps::IsEmpty(apTraits, apLocalPostorder))
{
return;
}
Expand Down Expand Up @@ -12450,6 +12485,11 @@ void Compiler::fgMorphStmts(BasicBlock* block)
compCurStmt = stmt;
GenTree* oldTree = stmt->GetRootNode();

if (optLocalAssertionProp)
{
BitVecOps::Assign(apTraits, apLocalPostorder, apLocal);
}

#ifdef DEBUG

unsigned oldHash = verbose ? gtHashValue(oldTree) : DUMMY_INIT(~0);
Expand Down Expand Up @@ -12670,7 +12710,8 @@ void Compiler::fgMorphBlock(BasicBlock* block, MorphUnreachableInfo* unreachable
// Each block starts with an empty table, and no available assertions
//
optAssertionReset(0);
apLocal = BitVecOps::MakeEmpty(apTraits);
BitVecOps::ClearD(apTraits, apLocal);
BitVecOps::ClearD(apTraits, apLocalPostorder);
}
else
{
Expand Down Expand Up @@ -12808,6 +12849,8 @@ void Compiler::fgMorphBlock(BasicBlock* block, MorphUnreachableInfo* unreachable
apLocal = BitVecOps::MakeEmpty(apTraits);
}

BitVecOps::Assign(apTraits, apLocalPostorder, apLocal);

JITDUMPEXEC(optDumpAssertionIndices("Assertions in: ", apLocal));
}
}
Expand Down Expand Up @@ -12876,6 +12919,8 @@ PhaseStatus Compiler::fgMorphBlocks()
// Local assertion prop is enabled if we are optimizing.
//
optAssertionInit(/* isLocalProp*/ true);
apLocal = BitVecOps::MakeEmpty(apTraits);
apLocalPostorder = BitVecOps::MakeEmpty(apTraits);
}
else
{
Expand Down Expand Up @@ -13011,10 +13056,12 @@ PhaseStatus Compiler::fgMorphBlocks()

if (optLocalAssertionProp)
{
Metrics.LocalAssertionCount = optAssertionCount;
Metrics.LocalAssertionOverflow = optAssertionOverflow;
Metrics.MorphTrackedLocals = lvaTrackedCount;
Metrics.MorphLocals = lvaCount;
Metrics.LocalAssertionCount = optAssertionCount;
Metrics.LocalAssertionOverflow = optAssertionOverflow;
Metrics.MorphTrackedLocals = lvaTrackedCount;
Metrics.MorphLocals = lvaCount;
optLocalAssertionProp = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

curious - is this something you noticed or it was prevention from getting into weird state, now that we will do another round of assertions with apLocalPostorder?

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 wanted to make sure calls into morph after the global morph phase didn't try and use local assertions. Most uses of local assertions in morph are guarded by fgGlobalMorph but it is easy to forget...

optCrossBlockLocalAssertionProp = false;
}

// We may have converted a tailcall into a loop, in which case the first BB
Expand Down
Loading