From 5cf06d10f8ebd736d5c4bbabd3d6f9e4b10c9291 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 2 Mar 2022 12:48:54 +0100 Subject: [PATCH] Revert "[InstCombine] Support switch in phi to cond fold" This reverts commit 0817ce86b540f909eade6a8d7370e1b47e863a70. Seeing some ppc64le stage2 failures, reverting to investigate. --- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 47 ++++++++-------------- .../Transforms/InstCombine/simple_phi_condition.ll | 13 +++--- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 85efd3e..f1a9128 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -1261,6 +1261,9 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, // ... ... // \ / // phi [true] [false] + if (!PN.getType()->isIntegerTy(1)) + return nullptr; + // Make sure all inputs are constants. if (!all_of(PN.operands(), [](Value *V) { return isa(V); })) return nullptr; @@ -1270,49 +1273,30 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, if (!DT.isReachableFromEntry(BB)) return nullptr; - // Determine which value the condition of the idom has for which successor. - LLVMContext &Context = PN.getContext(); + // Check that the immediate dominator has a conditional branch. auto *IDom = DT.getNode(BB)->getIDom()->getBlock(); - Value *Cond; - SmallDenseMap SuccForValue; - if (auto *BI = dyn_cast(IDom->getTerminator())) { - if (BI->isUnconditional()) - return nullptr; - - Cond = BI->getCondition(); - SuccForValue[ConstantInt::getTrue(Context)] = BI->getSuccessor(0); - SuccForValue[ConstantInt::getFalse(Context)] = BI->getSuccessor(1); - } else if (auto *SI = dyn_cast(IDom->getTerminator())) { - Cond = SI->getCondition(); - for (auto Case : SI->cases()) - SuccForValue[Case.getCaseValue()] = Case.getCaseSuccessor(); - } else { - return nullptr; - } - - if (Cond->getType() != PN.getType()) + auto *BI = dyn_cast(IDom->getTerminator()); + if (!BI || BI->isUnconditional()) return nullptr; // Check that edges outgoing from the idom's terminators dominate respective // inputs of the Phi. + BasicBlockEdge TrueOutEdge(IDom, BI->getSuccessor(0)); + BasicBlockEdge FalseOutEdge(IDom, BI->getSuccessor(1)); + Optional Invert; for (auto Pair : zip(PN.incoming_values(), PN.blocks())) { auto *Input = cast(std::get<0>(Pair)); BasicBlock *Pred = std::get<1>(Pair); - auto IsCorrectInput = [&](ConstantInt *Input) { - // The input needs to be dominated by the corresponding edge of the idom. - auto It = SuccForValue.find(Input); - return It != SuccForValue.end() && - DT.dominates(BasicBlockEdge(IDom, It->second), - BasicBlockEdge(Pred, BB)); - }; + BasicBlockEdge Edge(Pred, BB); + // The input needs to be dominated by one of the edges of the idom. // Depending on the constant, the condition may need to be inverted. bool NeedsInvert; - if (IsCorrectInput(Input)) - NeedsInvert = false; - else if (IsCorrectInput(cast(ConstantExpr::getNot(Input)))) - NeedsInvert = true; + if (DT.dominates(TrueOutEdge, Edge)) + NeedsInvert = Input->isZero(); + else if (DT.dominates(FalseOutEdge, Edge)) + NeedsInvert = Input->isOne(); else return nullptr; @@ -1323,6 +1307,7 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, Invert = NeedsInvert; } + auto *Cond = BI->getCondition(); if (!*Invert) return Cond; diff --git a/llvm/test/Transforms/InstCombine/simple_phi_condition.ll b/llvm/test/Transforms/InstCombine/simple_phi_condition.ll index cfbd386..a247a2e 100644 --- a/llvm/test/Transforms/InstCombine/simple_phi_condition.ll +++ b/llvm/test/Transforms/InstCombine/simple_phi_condition.ll @@ -279,7 +279,8 @@ define i8 @test_switch(i8 %cond) { ; CHECK: default: ; CHECK-NEXT: ret i8 42 ; CHECK: merge: -; CHECK-NEXT: ret i8 [[COND]] +; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 1, [[SW_1]] ], [ 7, [[SW_7]] ], [ 19, [[SW_19]] ] +; CHECK-NEXT: ret i8 [[RET]] ; entry: switch i8 %cond, label %default [ @@ -320,7 +321,8 @@ define i8 @test_switch_direct_edge(i8 %cond) { ; CHECK: default: ; CHECK-NEXT: ret i8 42 ; CHECK: merge: -; CHECK-NEXT: ret i8 [[COND]] +; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 1, [[SW_1]] ], [ 7, [[SW_7]] ], [ 19, [[ENTRY:%.*]] ] +; CHECK-NEXT: ret i8 [[RET]] ; entry: switch i8 %cond, label %default [ @@ -360,7 +362,8 @@ define i8 @test_switch_subset(i8 %cond) { ; CHECK: default: ; CHECK-NEXT: ret i8 42 ; CHECK: merge: -; CHECK-NEXT: ret i8 [[COND]] +; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 1, [[SW_1]] ], [ 7, [[SW_7]] ] +; CHECK-NEXT: ret i8 [[RET]] ; entry: switch i8 %cond, label %default [ @@ -447,8 +450,8 @@ define i8 @test_switch_inverted(i8 %cond) { ; CHECK: default: ; CHECK-NEXT: ret i8 42 ; CHECK: merge: -; CHECK-NEXT: [[TMP0:%.*]] = xor i8 [[COND]], -1 -; CHECK-NEXT: ret i8 [[TMP0]] +; CHECK-NEXT: [[RET:%.*]] = phi i8 [ -1, [[SW_0]] ], [ -2, [[SW_1]] ], [ -3, [[SW_2]] ] +; CHECK-NEXT: ret i8 [[RET]] ; entry: switch i8 %cond, label %default [ -- 2.7.4