Skip to content

Commit 8362cae

Browse files
committed
[SimpleLoopUnswitch] Fix exponential unswitch
When unswitching via invariant condition injection, we currently mark the condition in the old loop, so that it does not get unswitched again. However, if there are multiple branches for which conditions can be injected, then we can do that for both the old and new loop. This means that the number of unswitches increases exponentially. Change the handling to be more similar to partial unswitching, where we instead mark the whole loop, rather than a single condition. This means that we will only generate a linear number of loops. TBH I think even that is still highly undesirable, and we should probably be unswitching all candidates at the same time, so that we end up with only two loops. But at least this mitigates the worst case. The test case is a reduced variant that generates 1700 lines of IR without this patch and 290 with it. Fixes #66868.
1 parent ca3ed7b commit 8362cae

File tree

3 files changed

+329
-47
lines changed

3 files changed

+329
-47
lines changed

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

+34-27
Original file line numberDiff line numberDiff line change
@@ -2127,9 +2127,10 @@ static void unswitchNontrivialInvariants(
21272127
Loop &L, Instruction &TI, ArrayRef<Value *> Invariants,
21282128
IVConditionInfo &PartialIVInfo, DominatorTree &DT, LoopInfo &LI,
21292129
AssumptionCache &AC,
2130-
function_ref<void(bool, bool, ArrayRef<Loop *>)> UnswitchCB,
2130+
function_ref<void(bool, bool, bool, ArrayRef<Loop *>)> UnswitchCB,
21312131
ScalarEvolution *SE, MemorySSAUpdater *MSSAU,
2132-
function_ref<void(Loop &, StringRef)> DestroyLoopCB, bool InsertFreeze) {
2132+
function_ref<void(Loop &, StringRef)> DestroyLoopCB, bool InsertFreeze,
2133+
bool InjectedCondition) {
21332134
auto *ParentBB = TI.getParent();
21342135
BranchInst *BI = dyn_cast<BranchInst>(&TI);
21352136
SwitchInst *SI = BI ? nullptr : cast<SwitchInst>(&TI);
@@ -2582,7 +2583,7 @@ static void unswitchNontrivialInvariants(
25822583
for (Loop *UpdatedL : llvm::concat<Loop *>(NonChildClonedLoops, HoistedLoops))
25832584
if (UpdatedL->getParentLoop() == ParentL)
25842585
SibLoops.push_back(UpdatedL);
2585-
UnswitchCB(IsStillLoop, PartiallyInvariant, SibLoops);
2586+
UnswitchCB(IsStillLoop, PartiallyInvariant, InjectedCondition, SibLoops);
25862587

25872588
if (MSSAU && VerifyMemorySSA)
25882589
MSSAU->getMemorySSA()->verifyMemorySSA();
@@ -2980,13 +2981,6 @@ static bool shouldTryInjectInvariantCondition(
29802981
/// the metadata.
29812982
bool shouldTryInjectBasingOnMetadata(const BranchInst *BI,
29822983
const BasicBlock *TakenSucc) {
2983-
// Skip branches that have already been unswithed this way. After successful
2984-
// unswitching of injected condition, we will still have a copy of this loop
2985-
// which looks exactly the same as original one. To prevent the 2nd attempt
2986-
// of unswitching it in the same pass, mark this branch as "nothing to do
2987-
// here".
2988-
if (BI->hasMetadata("llvm.invariant.condition.injection.disabled"))
2989-
return false;
29902984
SmallVector<uint32_t> Weights;
29912985
if (!extractBranchWeights(*BI, Weights))
29922986
return false;
@@ -3069,13 +3063,9 @@ injectPendingInvariantConditions(NonTrivialUnswitchCandidate Candidate, Loop &L,
30693063
Builder.CreateCondBr(InjectedCond, InLoopSucc, CheckBlock);
30703064

30713065
Builder.SetInsertPoint(CheckBlock);
3072-
auto *NewTerm = Builder.CreateCondBr(TI->getCondition(), TI->getSuccessor(0),
3073-
TI->getSuccessor(1));
3074-
3066+
Builder.CreateCondBr(TI->getCondition(), TI->getSuccessor(0),
3067+
TI->getSuccessor(1));
30753068
TI->eraseFromParent();
3076-
// Prevent infinite unswitching.
3077-
NewTerm->setMetadata("llvm.invariant.condition.injection.disabled",
3078-
MDNode::get(BB->getContext(), {}));
30793069

30803070
// Fixup phis.
30813071
for (auto &I : *InLoopSucc) {
@@ -3443,7 +3433,7 @@ static bool shouldInsertFreeze(Loop &L, Instruction &TI, DominatorTree &DT,
34433433
static bool unswitchBestCondition(
34443434
Loop &L, DominatorTree &DT, LoopInfo &LI, AssumptionCache &AC,
34453435
AAResults &AA, TargetTransformInfo &TTI,
3446-
function_ref<void(bool, bool, ArrayRef<Loop *>)> UnswitchCB,
3436+
function_ref<void(bool, bool, bool, ArrayRef<Loop *>)> UnswitchCB,
34473437
ScalarEvolution *SE, MemorySSAUpdater *MSSAU,
34483438
function_ref<void(Loop &, StringRef)> DestroyLoopCB) {
34493439
// Collect all invariant conditions within this loop (as opposed to an inner
@@ -3453,9 +3443,10 @@ static bool unswitchBestCondition(
34533443
Instruction *PartialIVCondBranch = nullptr;
34543444
collectUnswitchCandidates(UnswitchCandidates, PartialIVInfo,
34553445
PartialIVCondBranch, L, LI, AA, MSSAU);
3456-
collectUnswitchCandidatesWithInjections(UnswitchCandidates, PartialIVInfo,
3457-
PartialIVCondBranch, L, DT, LI, AA,
3458-
MSSAU);
3446+
if (!findOptionMDForLoop(&L, "llvm.loop.unswitch.injection.disable"))
3447+
collectUnswitchCandidatesWithInjections(UnswitchCandidates, PartialIVInfo,
3448+
PartialIVCondBranch, L, DT, LI, AA,
3449+
MSSAU);
34593450
// If we didn't find any candidates, we're done.
34603451
if (UnswitchCandidates.empty())
34613452
return false;
@@ -3476,8 +3467,11 @@ static bool unswitchBestCondition(
34763467
return false;
34773468
}
34783469

3479-
if (Best.hasPendingInjection())
3470+
bool InjectedCondition = false;
3471+
if (Best.hasPendingInjection()) {
34803472
Best = injectPendingInvariantConditions(Best, L, DT, LI, AC, MSSAU);
3473+
InjectedCondition = true;
3474+
}
34813475
assert(!Best.hasPendingInjection() &&
34823476
"All injections should have been done by now!");
34833477

@@ -3505,7 +3499,7 @@ static bool unswitchBestCondition(
35053499
<< ") terminator: " << *Best.TI << "\n");
35063500
unswitchNontrivialInvariants(L, *Best.TI, Best.Invariants, PartialIVInfo, DT,
35073501
LI, AC, UnswitchCB, SE, MSSAU, DestroyLoopCB,
3508-
InsertFreeze);
3502+
InsertFreeze, InjectedCondition);
35093503
return true;
35103504
}
35113505

@@ -3534,7 +3528,7 @@ static bool
35343528
unswitchLoop(Loop &L, DominatorTree &DT, LoopInfo &LI, AssumptionCache &AC,
35353529
AAResults &AA, TargetTransformInfo &TTI, bool Trivial,
35363530
bool NonTrivial,
3537-
function_ref<void(bool, bool, ArrayRef<Loop *>)> UnswitchCB,
3531+
function_ref<void(bool, bool, bool, ArrayRef<Loop *>)> UnswitchCB,
35383532
ScalarEvolution *SE, MemorySSAUpdater *MSSAU,
35393533
ProfileSummaryInfo *PSI, BlockFrequencyInfo *BFI,
35403534
function_ref<void(Loop &, StringRef)> DestroyLoopCB) {
@@ -3549,7 +3543,8 @@ unswitchLoop(Loop &L, DominatorTree &DT, LoopInfo &LI, AssumptionCache &AC,
35493543
if (Trivial && unswitchAllTrivialConditions(L, DT, LI, SE, MSSAU)) {
35503544
// If we unswitched successfully we will want to clean up the loop before
35513545
// processing it further so just mark it as unswitched and return.
3552-
UnswitchCB(/*CurrentLoopValid*/ true, false, {});
3546+
UnswitchCB(/*CurrentLoopValid*/ true, /*PartiallyInvariant*/ false,
3547+
/*InjectedCondition*/ false, {});
35533548
return true;
35543549
}
35553550

@@ -3645,6 +3640,7 @@ PreservedAnalyses SimpleLoopUnswitchPass::run(Loop &L, LoopAnalysisManager &AM,
36453640

36463641
auto UnswitchCB = [&L, &U, &LoopName](bool CurrentLoopValid,
36473642
bool PartiallyInvariant,
3643+
bool InjectedCondition,
36483644
ArrayRef<Loop *> NewLoops) {
36493645
// If we did a non-trivial unswitch, we have added new (cloned) loops.
36503646
if (!NewLoops.empty())
@@ -3664,6 +3660,16 @@ PreservedAnalyses SimpleLoopUnswitchPass::run(Loop &L, LoopAnalysisManager &AM,
36643660
Context, L.getLoopID(), {"llvm.loop.unswitch.partial"},
36653661
{DisableUnswitchMD});
36663662
L.setLoopID(NewLoopID);
3663+
} else if (InjectedCondition) {
3664+
// Do the same for injection of invariant conditions.
3665+
auto &Context = L.getHeader()->getContext();
3666+
MDNode *DisableUnswitchMD = MDNode::get(
3667+
Context,
3668+
MDString::get(Context, "llvm.loop.unswitch.injection.disable"));
3669+
MDNode *NewLoopID = makePostTransformationMetadata(
3670+
Context, L.getLoopID(), {"llvm.loop.unswitch.injection"},
3671+
{DisableUnswitchMD});
3672+
L.setLoopID(NewLoopID);
36673673
} else
36683674
U.revisitCurrentLoop();
36693675
} else
@@ -3756,6 +3762,7 @@ bool SimpleLoopUnswitchLegacyPass::runOnLoop(Loop *L, LPPassManager &LPM) {
37563762
auto *SE = SEWP ? &SEWP->getSE() : nullptr;
37573763

37583764
auto UnswitchCB = [&L, &LPM](bool CurrentLoopValid, bool PartiallyInvariant,
3765+
bool InjectedCondition,
37593766
ArrayRef<Loop *> NewLoops) {
37603767
// If we did a non-trivial unswitch, we have added new (cloned) loops.
37613768
for (auto *NewL : NewLoops)
@@ -3766,9 +3773,9 @@ bool SimpleLoopUnswitchLegacyPass::runOnLoop(Loop *L, LPPassManager &LPM) {
37663773
// but it is the best we can do in the old PM.
37673774
if (CurrentLoopValid) {
37683775
// If the current loop has been unswitched using a partially invariant
3769-
// condition, we should not re-add the current loop to avoid unswitching
3770-
// on the same condition again.
3771-
if (!PartiallyInvariant)
3776+
// condition or injected invariant condition, we should not re-add the
3777+
// current loop to avoid unswitching on the same condition again.
3778+
if (!PartiallyInvariant && !InjectedCondition)
37723779
LPM.addLoop(*L);
37733780
} else
37743781
LPM.markLoopAsDeleted(*L);

0 commit comments

Comments
 (0)