Skip to content

Commit 4010a7a

Browse files
committed
Reapply [InstCombine] Support switch in phi to cond fold
Reapply with an explicit check for multi-edges, as the expected behavior of multi-edge dominance is unclear (D120811). ----- For conditional branches, we know the value is i1 0 or i1 1 along the outgoing edges. For switches we can apply exactly the same optimization, just with the known values determined by the switch cases.
1 parent 5a62495 commit 4010a7a

File tree

2 files changed

+43
-24
lines changed

2 files changed

+43
-24
lines changed

llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp

+38-16
Original file line numberDiff line numberDiff line change
@@ -1269,9 +1269,6 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN,
12691269
// ... ...
12701270
// \ /
12711271
// phi [true] [false]
1272-
if (!PN.getType()->isIntegerTy(1))
1273-
return nullptr;
1274-
12751272
// Make sure all inputs are constants.
12761273
if (!all_of(PN.operands(), [](Value *V) { return isa<ConstantInt>(V); }))
12771274
return nullptr;
@@ -1281,30 +1278,56 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN,
12811278
if (!DT.isReachableFromEntry(BB))
12821279
return nullptr;
12831280

1284-
// Check that the immediate dominator has a conditional branch.
1281+
// Determine which value the condition of the idom has for which successor.
1282+
LLVMContext &Context = PN.getContext();
12851283
auto *IDom = DT.getNode(BB)->getIDom()->getBlock();
1286-
auto *BI = dyn_cast<BranchInst>(IDom->getTerminator());
1287-
if (!BI || BI->isUnconditional())
1284+
Value *Cond;
1285+
SmallDenseMap<ConstantInt *, BasicBlock *, 8> SuccForValue;
1286+
SmallDenseMap<BasicBlock *, unsigned, 8> SuccCount;
1287+
auto AddSucc = [&](ConstantInt *C, BasicBlock *Succ) {
1288+
SuccForValue[C] = Succ;
1289+
++SuccCount[Succ];
1290+
};
1291+
if (auto *BI = dyn_cast<BranchInst>(IDom->getTerminator())) {
1292+
if (BI->isUnconditional())
1293+
return nullptr;
1294+
1295+
Cond = BI->getCondition();
1296+
AddSucc(ConstantInt::getTrue(Context), BI->getSuccessor(0));
1297+
AddSucc(ConstantInt::getFalse(Context), BI->getSuccessor(1));
1298+
} else if (auto *SI = dyn_cast<SwitchInst>(IDom->getTerminator())) {
1299+
Cond = SI->getCondition();
1300+
for (auto Case : SI->cases())
1301+
AddSucc(Case.getCaseValue(), Case.getCaseSuccessor());
1302+
} else {
1303+
return nullptr;
1304+
}
1305+
1306+
if (Cond->getType() != PN.getType())
12881307
return nullptr;
12891308

12901309
// Check that edges outgoing from the idom's terminators dominate respective
12911310
// inputs of the Phi.
1292-
BasicBlockEdge TrueOutEdge(IDom, BI->getSuccessor(0));
1293-
BasicBlockEdge FalseOutEdge(IDom, BI->getSuccessor(1));
1294-
12951311
Optional<bool> Invert;
12961312
for (auto Pair : zip(PN.incoming_values(), PN.blocks())) {
12971313
auto *Input = cast<ConstantInt>(std::get<0>(Pair));
12981314
BasicBlock *Pred = std::get<1>(Pair);
1299-
BasicBlockEdge Edge(Pred, BB);
1315+
auto IsCorrectInput = [&](ConstantInt *Input) {
1316+
// The input needs to be dominated by the corresponding edge of the idom.
1317+
// This edge cannot be a multi-edge, as that would imply that multiple
1318+
// different condition values follow the same edge.
1319+
auto It = SuccForValue.find(Input);
1320+
return It != SuccForValue.end() && SuccCount[It->second] == 1 &&
1321+
DT.dominates(BasicBlockEdge(IDom, It->second),
1322+
BasicBlockEdge(Pred, BB));
1323+
};
13001324

1301-
// The input needs to be dominated by one of the edges of the idom.
13021325
// Depending on the constant, the condition may need to be inverted.
13031326
bool NeedsInvert;
1304-
if (DT.dominates(TrueOutEdge, Edge))
1305-
NeedsInvert = Input->isZero();
1306-
else if (DT.dominates(FalseOutEdge, Edge))
1307-
NeedsInvert = Input->isOne();
1327+
if (IsCorrectInput(Input))
1328+
NeedsInvert = false;
1329+
else if (IsCorrectInput(cast<ConstantInt>(ConstantExpr::getNot(Input))))
1330+
NeedsInvert = true;
13081331
else
13091332
return nullptr;
13101333

@@ -1315,7 +1338,6 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN,
13151338
Invert = NeedsInvert;
13161339
}
13171340

1318-
auto *Cond = BI->getCondition();
13191341
if (!*Invert)
13201342
return Cond;
13211343

llvm/test/Transforms/InstCombine/simple_phi_condition.ll

+5-8
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,7 @@ define i8 @test_switch(i8 %cond) {
279279
; CHECK: default:
280280
; CHECK-NEXT: ret i8 42
281281
; CHECK: merge:
282-
; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 1, [[SW_1]] ], [ 7, [[SW_7]] ], [ 19, [[SW_19]] ]
283-
; CHECK-NEXT: ret i8 [[RET]]
282+
; CHECK-NEXT: ret i8 [[COND]]
284283
;
285284
entry:
286285
switch i8 %cond, label %default [
@@ -321,8 +320,7 @@ define i8 @test_switch_direct_edge(i8 %cond) {
321320
; CHECK: default:
322321
; CHECK-NEXT: ret i8 42
323322
; CHECK: merge:
324-
; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 1, [[SW_1]] ], [ 7, [[SW_7]] ], [ 19, [[ENTRY:%.*]] ]
325-
; CHECK-NEXT: ret i8 [[RET]]
323+
; CHECK-NEXT: ret i8 [[COND]]
326324
;
327325
entry:
328326
switch i8 %cond, label %default [
@@ -396,8 +394,7 @@ define i8 @test_switch_subset(i8 %cond) {
396394
; CHECK: default:
397395
; CHECK-NEXT: ret i8 42
398396
; CHECK: merge:
399-
; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 1, [[SW_1]] ], [ 7, [[SW_7]] ]
400-
; CHECK-NEXT: ret i8 [[RET]]
397+
; CHECK-NEXT: ret i8 [[COND]]
401398
;
402399
entry:
403400
switch i8 %cond, label %default [
@@ -484,8 +481,8 @@ define i8 @test_switch_inverted(i8 %cond) {
484481
; CHECK: default:
485482
; CHECK-NEXT: ret i8 42
486483
; CHECK: merge:
487-
; CHECK-NEXT: [[RET:%.*]] = phi i8 [ -1, [[SW_0]] ], [ -2, [[SW_1]] ], [ -3, [[SW_2]] ]
488-
; CHECK-NEXT: ret i8 [[RET]]
484+
; CHECK-NEXT: [[TMP0:%.*]] = xor i8 [[COND]], -1
485+
; CHECK-NEXT: ret i8 [[TMP0]]
489486
;
490487
entry:
491488
switch i8 %cond, label %default [

0 commit comments

Comments
 (0)