Skip to content

Commit cc78af7

Browse files
authored
JIT: Sparse conditional propagation in value numbering (#94563)
Utilize the fact that VN is able to decide whether many non-trivial branches will be taken or not taken to prove basic blocks dynamically unreachable. Take this into account when VN'ing PHIs. Fix #94559
1 parent d1e4fda commit cc78af7

File tree

2 files changed

+69
-12
lines changed

2 files changed

+69
-12
lines changed

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5057,6 +5057,7 @@ class Compiler
50575057

50585058
// The value numbers for this compilation.
50595059
ValueNumStore* vnStore;
5060+
struct ValueNumberState* vnVisitState;
50605061

50615062
public:
50625063
ValueNumStore* GetValueNumStore()

src/coreclr/jit/valuenum.cpp

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9658,6 +9658,8 @@ struct ValueNumberState
96589658
BVB_complete = 0x1,
96599659
BVB_onAllDone = 0x2,
96609660
BVB_onNotAllDone = 0x4,
9661+
// Set for statically reachable blocks that we have proven unreachable.
9662+
BVB_provenUnreachable = 0x8,
96619663
};
96629664

96639665
bool GetVisitBit(unsigned bbNum, BlockVisitBits bvb)
@@ -9787,7 +9789,8 @@ struct ValueNumberState
97879789
JITDUMP(" Not yet completed.\n");
97889790
#endif // DEBUG_VN_VISIT
97899791

9790-
bool allPredsVisited = true;
9792+
bool allPredsVisited = true;
9793+
bool provenUnreachable = true;
97919794
for (FlowEdge* pred = m_comp->BlockPredsWithEH(succ); pred != nullptr; pred = pred->getNextPredEdge())
97929795
{
97939796
BasicBlock* predBlock = pred->getSourceBlock();
@@ -9796,6 +9799,12 @@ struct ValueNumberState
97969799
allPredsVisited = false;
97979800
break;
97989801
}
9802+
9803+
if (provenUnreachable && !GetVisitBit(predBlock->bbNum, BVB_provenUnreachable) &&
9804+
IsReachableFromPred(predBlock, succ))
9805+
{
9806+
provenUnreachable = false;
9807+
}
97999808
}
98009809

98019810
if (allPredsVisited)
@@ -9808,6 +9817,11 @@ struct ValueNumberState
98089817
assert(!GetVisitBit(succ->bbNum, BVB_onAllDone));
98099818
m_toDoAllPredsDone.Push(succ);
98109819
SetVisitBit(succ->bbNum, BVB_onAllDone);
9820+
9821+
if (provenUnreachable)
9822+
{
9823+
SetVisitBit(succ->bbNum, BVB_provenUnreachable);
9824+
}
98119825
}
98129826
else
98139827
{
@@ -9829,6 +9843,31 @@ struct ValueNumberState
98299843
});
98309844
}
98319845

9846+
bool IsReachableFromPred(BasicBlock* predBlock, BasicBlock* block)
9847+
{
9848+
if (!predBlock->KindIs(BBJ_COND) || predBlock->JumpsToNext())
9849+
{
9850+
return true;
9851+
}
9852+
9853+
GenTree* lastTree = predBlock->lastStmt()->GetRootNode();
9854+
assert(lastTree->OperIs(GT_JTRUE));
9855+
9856+
GenTree* cond = lastTree->gtGetOp1();
9857+
// TODO-Cleanup: Using liberal VNs here is a bit questionable as it
9858+
// adds a cross-phase dependency on RBO to definitely fold this branch
9859+
// away.
9860+
ValueNum normalVN = m_comp->vnStore->VNNormalValue(cond->GetVN(VNK_Liberal));
9861+
if (!m_comp->vnStore->IsVNConstant(normalVN))
9862+
{
9863+
return true;
9864+
}
9865+
9866+
bool isTaken = normalVN != m_comp->vnStore->VNZeroForType(TYP_INT);
9867+
BasicBlock* unreachableSucc = isTaken ? predBlock->Next() : predBlock->GetJumpDest();
9868+
return block != unreachableSucc;
9869+
}
9870+
98329871
bool ToDoExists()
98339872
{
98349873
return m_toDoAllPredsDone.Size() > 0 || m_toDoNotAllPredsDone.Size() > 0;
@@ -9965,6 +10004,8 @@ PhaseStatus Compiler::fgValueNumber()
996510004

996610005
ValueNumberState vs(this);
996710006

10007+
vnVisitState = &vs;
10008+
996810009
// Push the first block. This has no preds.
996910010
vs.m_toDoAllPredsDone.Push(fgFirstBB);
997010011

@@ -9973,7 +10014,13 @@ PhaseStatus Compiler::fgValueNumber()
997310014
while (vs.m_toDoAllPredsDone.Size() > 0)
997410015
{
997510016
BasicBlock* toDo = vs.m_toDoAllPredsDone.Pop();
10017+
10018+
// TODO-TP: We can skip VN'ing blocks we have proven unreachable
10019+
// here. However, currently downstream phases are not prepared to
10020+
// handle the fact that some blocks that are seemingly reachable
10021+
// have not been VN'd.
997610022
fgValueNumberBlock(toDo);
10023+
997710024
// Record that we've visited "toDo", and add successors to the right sets.
997810025
vs.FinishVisit(toDo);
997910026
}
@@ -10010,8 +10057,7 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
1001010057

1001110058
Statement* stmt = blk->firstStmt();
1001210059

10013-
// First: visit phi's. If "newVNForPhis", give them new VN's. If not,
10014-
// first check to see if all phi args have the same value.
10060+
// First: visit phis and check to see if all phi args have the same value.
1001510061
for (; (stmt != nullptr) && stmt->IsPhiDefnStmt(); stmt = stmt->GetNextStmt())
1001610062
{
1001710063
GenTreeLclVar* newSsaDef = stmt->GetRootNode()->AsLclVar();
@@ -10021,9 +10067,22 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
1002110067

1002210068
for (GenTreePhi::Use& use : phiNode->Uses())
1002310069
{
10024-
GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg();
10025-
ValueNum phiArgSsaNumVN = vnStore->VNForIntCon(phiArg->GetSsaNum());
10026-
ValueNumPair phiArgVNP = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair;
10070+
GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg();
10071+
if (vnVisitState->GetVisitBit(phiArg->gtPredBB->bbNum, ValueNumberState::BVB_provenUnreachable))
10072+
{
10073+
JITDUMP(" Phi arg [%06u] refers to proven unreachable pred " FMT_BB "\n", dspTreeID(phiArg),
10074+
phiArg->gtPredBB->bbNum);
10075+
10076+
if ((use.GetNext() != nullptr) || (phiVNP.GetLiberal() != ValueNumStore::NoVN))
10077+
{
10078+
continue;
10079+
}
10080+
10081+
JITDUMP(" ..but it looks like all preds are unreachable, so we are using it anyway\n");
10082+
}
10083+
10084+
ValueNum phiArgSsaNumVN = vnStore->VNForIntCon(phiArg->GetSsaNum());
10085+
ValueNumPair phiArgVNP = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair;
1002710086

1002810087
phiArg->gtVNPair = phiArgVNP;
1002910088

@@ -10049,12 +10108,9 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
1004910108
}
1005010109
}
1005110110

10052-
#ifdef DEBUG
10053-
// There should be at least to 2 PHI arguments so phiVN's VNs should always be VNF_Phi functions.
10054-
VNFuncApp phiFunc;
10055-
assert(vnStore->GetVNFunc(phiVNP.GetLiberal(), &phiFunc) && (phiFunc.m_func == VNF_Phi));
10056-
assert(vnStore->GetVNFunc(phiVNP.GetConservative(), &phiFunc) && (phiFunc.m_func == VNF_Phi));
10057-
#endif
10111+
// We should have visited at least one phi arg in the loop above
10112+
assert(phiVNP.GetLiberal() != ValueNumStore::NoVN);
10113+
assert(phiVNP.GetConservative() != ValueNumStore::NoVN);
1005810114

1005910115
ValueNumPair newSsaDefVNP;
1006010116

0 commit comments

Comments
 (0)