[llvm] 4637daa - Revert D84264 "[SCCP] Directly remove non-feasible edges" & 5db5b4bc4394ca247c9eb665e03b851848aa2fbf
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 23 17:52:39 PDT 2020
Author: Fangrui Song
Date: 2020-07-23T17:51:48-07:00
New Revision: 4637daa9905cbe12adba85fc88ea95a3be7db85d
URL: https://github.com/llvm/llvm-project/commit/4637daa9905cbe12adba85fc88ea95a3be7db85d
DIFF: https://github.com/llvm/llvm-project/commit/4637daa9905cbe12adba85fc88ea95a3be7db85d.diff
LOG: Revert D84264 "[SCCP] Directly remove non-feasible edges" & 5db5b4bc4394ca247c9eb665e03b851848aa2fbf
It breaks stage-2 build. Clang crashed when compiling
llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
llvm/Support/GenericDomTree.h eraseNode: Node is not a leaf node
Added:
Modified:
llvm/lib/Transforms/Scalar/SCCP.cpp
llvm/test/Transforms/SCCP/conditions-ranges.ll
llvm/test/Transforms/SCCP/predicateinfo-cond.ll
llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
llvm/test/Transforms/SCCP/switch.ll
llvm/test/Transforms/SCCP/widening.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
index 985e88dfa017..32dc14e5ec19 100644
--- a/llvm/lib/Transforms/Scalar/SCCP.cpp
+++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -276,7 +276,7 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
// isEdgeFeasible - Return true if the control flow edge from the 'From' basic
// block to the 'To' basic block is currently feasible.
- bool isEdgeFeasible(BasicBlock *From, BasicBlock *To) const;
+ bool isEdgeFeasible(BasicBlock *From, BasicBlock *To);
std::vector<ValueLatticeElement> getStructLatticeValueFor(Value *V) const {
std::vector<ValueLatticeElement> StructValues;
@@ -705,7 +705,7 @@ void SCCPSolver::getFeasibleSuccessors(Instruction &TI,
// isEdgeFeasible - Return true if the control flow edge from the 'From' basic
// block to the 'To' basic block is currently feasible.
-bool SCCPSolver::isEdgeFeasible(BasicBlock *From, BasicBlock *To) const {
+bool SCCPSolver::isEdgeFeasible(BasicBlock *From, BasicBlock *To) {
// Check if we've called markEdgeExecutable on the edge yet. (We could
// be more aggressive and try to consider edges which haven't been marked
// yet, but there isn't any need.)
@@ -1807,51 +1807,39 @@ static void findReturnsToZap(Function &F,
}
}
-static bool removeNonFeasibleEdges(const SCCPSolver &Solver, BasicBlock *BB,
- DomTreeUpdater &DTU) {
- SmallPtrSet<BasicBlock *, 8> FeasibleSuccessors;
- bool HasNonFeasibleEdges = false;
- for (BasicBlock *Succ : successors(BB)) {
- if (Solver.isEdgeFeasible(BB, Succ))
- FeasibleSuccessors.insert(Succ);
- else
- HasNonFeasibleEdges = true;
- }
-
- // All edges feasible, nothing to do.
- if (!HasNonFeasibleEdges)
- return false;
-
- // SCCP can only determine non-feasible edges for br, switch and indirectbr.
- Instruction *TI = BB->getTerminator();
- assert((isa<BranchInst>(TI) || isa<SwitchInst>(TI) ||
- isa<IndirectBrInst>(TI)) &&
- "Terminator must be a br, switch or indirectbr");
-
- if (FeasibleSuccessors.size() == 1) {
- // Replace with an unconditional branch to the only feasible successor.
- BasicBlock *OnlyFeasibleSuccessor = *FeasibleSuccessors.begin();
- SmallVector<DominatorTree::UpdateType, 8> Updates;
- bool HaveSeenOnlyFeasibleSuccessor = false;
- for (BasicBlock *Succ : successors(BB)) {
- if (Succ == OnlyFeasibleSuccessor && !HaveSeenOnlyFeasibleSuccessor) {
- // Don't remove the edge to the only feasible successor the first time
- // we see it. We still do need to remove any multi-edges to it though.
- HaveSeenOnlyFeasibleSuccessor = true;
- continue;
- }
-
- Succ->removePredecessor(BB);
- Updates.push_back({DominatorTree::Delete, BB, Succ});
+// Update the condition for terminators that are branching on indeterminate
+// values, forcing them to use a specific edge.
+static void forceIndeterminateEdge(Instruction* I, SCCPSolver &Solver) {
+ BasicBlock *Dest = nullptr;
+ Constant *C = nullptr;
+ if (SwitchInst *SI = dyn_cast<SwitchInst>(I)) {
+ if (!isa<ConstantInt>(SI->getCondition())) {
+ // Indeterminate switch; use first case value.
+ Dest = SI->case_begin()->getCaseSuccessor();
+ C = SI->case_begin()->getCaseValue();
+ }
+ } else if (BranchInst *BI = dyn_cast<BranchInst>(I)) {
+ if (!isa<ConstantInt>(BI->getCondition())) {
+ // Indeterminate branch; use false.
+ Dest = BI->getSuccessor(1);
+ C = ConstantInt::getFalse(BI->getContext());
+ }
+ } else if (IndirectBrInst *IBR = dyn_cast<IndirectBrInst>(I)) {
+ if (!isa<BlockAddress>(IBR->getAddress()->stripPointerCasts())) {
+ // Indeterminate indirectbr; use successor 0.
+ Dest = IBR->getSuccessor(0);
+ C = BlockAddress::get(IBR->getSuccessor(0));
}
-
- DTU.applyUpdatesPermissive(Updates);
- BranchInst::Create(OnlyFeasibleSuccessor, BB);
- TI->eraseFromParent();
} else {
- llvm_unreachable("Either all successors are feasible, or exactly one is");
+ llvm_unreachable("Unexpected terminator instruction");
+ }
+ if (C) {
+ assert(Solver.isEdgeFeasible(I->getParent(), Dest) &&
+ "Didn't find feasible edge?");
+ (void)Dest;
+
+ I->setOperand(0, C);
}
- return true;
}
bool llvm::runIPSCCP(
@@ -1964,11 +1952,45 @@ bool llvm::runIPSCCP(
/*UseLLVMTrap=*/false,
/*PreserveLCSSA=*/false, &DTU);
- for (BasicBlock &BB : F)
- MadeChanges |= removeNonFeasibleEdges(Solver, &BB, DTU);
-
- for (BasicBlock *DeadBB : BlocksToErase)
+ // Now that all instructions in the function are constant folded,
+ // use ConstantFoldTerminator to get rid of in-edges, record DT updates and
+ // delete dead BBs.
+ for (BasicBlock *DeadBB : BlocksToErase) {
+ // If there are any PHI nodes in this successor, drop entries for BB now.
+ for (Value::user_iterator UI = DeadBB->user_begin(),
+ UE = DeadBB->user_end();
+ UI != UE;) {
+ // Grab the user and then increment the iterator early, as the user
+ // will be deleted. Step past all adjacent uses from the same user.
+ auto *I = dyn_cast<Instruction>(*UI);
+ do { ++UI; } while (UI != UE && *UI == I);
+
+ // Ignore blockaddress users; BasicBlock's dtor will handle them.
+ if (!I) continue;
+
+ // If we have forced an edge for an indeterminate value, then force the
+ // terminator to fold to that edge.
+ forceIndeterminateEdge(I, Solver);
+ BasicBlock *InstBB = I->getParent();
+ bool Folded = ConstantFoldTerminator(InstBB,
+ /*DeleteDeadConditions=*/false,
+ /*TLI=*/nullptr, &DTU);
+ assert(Folded &&
+ "Expect TermInst on constantint or blockaddress to be folded");
+ (void) Folded;
+ // If we folded the terminator to an unconditional branch to another
+ // dead block, replace it with Unreachable, to avoid trying to fold that
+ // branch again.
+ BranchInst *BI = cast<BranchInst>(InstBB->getTerminator());
+ if (BI && BI->isUnconditional() &&
+ !Solver.isBlockExecutable(BI->getSuccessor(0))) {
+ InstBB->getTerminator()->eraseFromParent();
+ new UnreachableInst(InstBB->getContext(), InstBB);
+ }
+ }
+ // Mark dead BB for deletion.
DTU.deleteBB(DeadBB);
+ }
for (BasicBlock &BB : F) {
for (BasicBlock::iterator BI = BB.begin(), E = BB.end(); BI != E;) {
diff --git a/llvm/test/Transforms/SCCP/conditions-ranges.ll b/llvm/test/Transforms/SCCP/conditions-ranges.ll
index dada59099d81..612a38f008fc 100644
--- a/llvm/test/Transforms/SCCP/conditions-ranges.ll
+++ b/llvm/test/Transforms/SCCP/conditions-ranges.ll
@@ -231,12 +231,12 @@ define void @f7_nested_conds(i32* %a, i32 %b) {
; CHECK-NEXT: [[C_1:%.*]] = icmp ne i32 [[A_V]], 0
; CHECK-NEXT: br i1 [[C_1]], label [[TRUE:%.*]], label [[FALSE:%.*]]
; CHECK: false:
-; CHECK-NEXT: br label [[TRUE_2:%.*]]
+; CHECK-NEXT: br i1 true, label [[TRUE_2:%.*]], label [[TRUE]]
; CHECK: true.2:
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: ret void
; CHECK: true:
-; CHECK-NEXT: store i32 [[B:%.*]], i32* [[A]], align 4
+; CHECK-NEXT: store i32 [[B:%.*]], i32* [[A]]
; CHECK-NEXT: ret void
;
entry:
diff --git a/llvm/test/Transforms/SCCP/predicateinfo-cond.ll b/llvm/test/Transforms/SCCP/predicateinfo-cond.ll
index 1443cc72c2ef..8ed96ec9301f 100644
--- a/llvm/test/Transforms/SCCP/predicateinfo-cond.ll
+++ b/llvm/test/Transforms/SCCP/predicateinfo-cond.ll
@@ -105,7 +105,7 @@ define void @pr46814(i32 %a) {
; CHECK-NEXT: [[C3:%.*]] = and i1 [[C1]], [[C2]]
; CHECK-NEXT: br i1 [[C3]], label [[IF_1:%.*]], label [[EXIT:%.*]]
; CHECK: if.1:
-; CHECK-NEXT: br label [[IF_2:%.*]]
+; CHECK-NEXT: br i1 true, label [[IF_2:%.*]], label [[EXIT]]
; CHECK: if.2:
; CHECK-NEXT: br i1 true, label [[EXIT]], label [[EXIT]]
; CHECK: exit:
diff --git a/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll b/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
index 9e9d1256c4cc..e1c7b3d5662d 100644
--- a/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
+++ b/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
@@ -136,12 +136,13 @@ define internal i1 @test2_g(%t1* %h, i32 %i) {
; CHECK-LABEL: define {{[^@]+}}@test2_g
; CHECK-SAME: (%t1* [[H:%.*]], i32 [[I:%.*]])
; CHECK-NEXT: entry:
-; CHECK-NEXT: br label [[LAND_RHS:%.*]]
+; CHECK-NEXT: br i1 true, label [[LAND_RHS:%.*]], label [[LAND_END:%.*]]
; CHECK: land.rhs:
; CHECK-NEXT: [[CALL:%.*]] = call i32 (...) @test2_j()
; CHECK-NEXT: [[TOBOOL1:%.*]] = icmp ne i32 [[CALL]], 0
-; CHECK-NEXT: br label [[LAND_END:%.*]]
+; CHECK-NEXT: br label [[LAND_END]]
; CHECK: land.end:
+; CHECK-NEXT: [[TMP0:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[TOBOOL1]], [[LAND_RHS]] ]
; CHECK-NEXT: ret i1 undef
;
entry:
@@ -195,9 +196,10 @@ define internal i32 @test3_k(i8 %h, i32 %i) {
; CHECK-NEXT: [[TMP1:%.*]] = inttoptr i64 [[CONV]] to %t1*
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
+; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ undef, [[ENTRY:%.*]] ], [ false, [[LOOP]] ]
; CHECK-NEXT: [[CALL:%.*]] = call i1 @test3_g(%t1* [[TMP1]], i32 0)
; CHECK-NEXT: call void @use.1(i1 false)
-; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK-NEXT: br i1 false, label [[LOOP]], label [[EXIT:%.*]]
; CHECK: exit:
; CHECK-NEXT: ret i32 undef
;
diff --git a/llvm/test/Transforms/SCCP/switch-constantfold-crash.ll b/llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
index 17b37f000407..7596a56b8122 100644
--- a/llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
+++ b/llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
@@ -5,11 +5,11 @@
define void @barney() {
; CHECK-LABEL: @barney(
; CHECK-NEXT: bb:
-; CHECK-NEXT: br label [[BB9:%.*]]
+; CHECK-NEXT: br label %bb9
; CHECK: bb6:
; CHECK-NEXT: unreachable
; CHECK: bb9:
-; CHECK-NEXT: br label [[BB6:%.*]]
+; CHECK-NEXT: unreachable
;
bb:
br label %bb9
@@ -29,9 +29,9 @@ bb9: ; preds = %bb
define void @blam() {
; CHECK-LABEL: @blam(
; CHECK-NEXT: bb:
-; CHECK-NEXT: br label [[BB16:%.*]]
+; CHECK-NEXT: br label %bb16
; CHECK: bb16:
-; CHECK-NEXT: br label [[BB38:%.*]]
+; CHECK-NEXT: br label %bb38
; CHECK: bb38:
; CHECK-NEXT: unreachable
;
@@ -62,9 +62,9 @@ bb38: ; preds = %bb16
define void @hoge() {
; CHECK-LABEL: @hoge(
; CHECK-NEXT: bb:
-; CHECK-NEXT: br label [[BB2:%.*]]
+; CHECK-NEXT: br label %bb2
; CHECK: bb2:
-; CHECK-NEXT: br label [[BB3:%.*]]
+; CHECK-NEXT: unreachable
; CHECK: bb3:
; CHECK-NEXT: unreachable
;
diff --git a/llvm/test/Transforms/SCCP/switch.ll b/llvm/test/Transforms/SCCP/switch.ll
index 3587587bcb91..5f607a3afd89 100644
--- a/llvm/test/Transforms/SCCP/switch.ll
+++ b/llvm/test/Transforms/SCCP/switch.ll
@@ -23,11 +23,15 @@ define i32 @test_duplicate_successors_phi(i1 %c, i32 %x) {
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[C:%.*]], label [[SWITCH:%.*]], label [[END:%.*]]
; CHECK: switch:
-; CHECK-NEXT: br label [[SWITCH_DEFAULT:%.*]]
+; CHECK-NEXT: switch i32 -1, label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT: i32 0, label [[END]]
+; CHECK-NEXT: i32 1, label [[END]]
+; CHECK-NEXT: ]
; CHECK: switch.default:
; CHECK-NEXT: ret i32 -1
; CHECK: end:
-; CHECK-NEXT: ret i32 [[X:%.*]]
+; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[X:%.*]], [[ENTRY:%.*]] ], [ 1, [[SWITCH]] ], [ 1, [[SWITCH]] ]
+; CHECK-NEXT: ret i32 [[PHI]]
;
entry:
br i1 %c, label %switch, label %end
diff --git a/llvm/test/Transforms/SCCP/widening.ll b/llvm/test/Transforms/SCCP/widening.ll
index 23a88c35a93e..2703bdb27dff 100644
--- a/llvm/test/Transforms/SCCP/widening.ll
+++ b/llvm/test/Transforms/SCCP/widening.ll
@@ -216,11 +216,11 @@ define void @rotated_loop_2(i32 %x) {
; IPSCCP: bb3:
; IPSCCP-NEXT: br label [[EXIT]]
; IPSCCP: exit:
-; IPSCCP-NEXT: [[P:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ 3, [[BB1]] ], [ 2, [[BB2]] ], [ 5, [[BB3]] ]
-; IPSCCP-NEXT: [[A:%.*]] = add i32 [[P]], 1
+; IPSCCP-NEXT: [[P:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ 3, [[BB1]] ], [ 2, [[BB2]] ], [ 5, [[BB3]] ], [ [[A:%.*]], [[EXIT]] ]
+; IPSCCP-NEXT: [[A]] = add i32 [[P]], 1
; IPSCCP-NEXT: call void @use(i1 true)
; IPSCCP-NEXT: call void @use(i1 false)
-; IPSCCP-NEXT: br label [[EXIT_1:%.*]]
+; IPSCCP-NEXT: br i1 false, label [[EXIT]], label [[EXIT_1:%.*]]
; IPSCCP: exit.1:
; IPSCCP-NEXT: ret void
;
@@ -451,10 +451,10 @@ define void @foo(i64* %arg) {
; SCCP-NEXT: [[TMP7:%.*]] = sub i64 3, [[TMP6]]
; SCCP-NEXT: [[TMP8:%.*]] = shl i64 [[TMP7]], 1
; SCCP-NEXT: [[TMP9:%.*]] = trunc i64 [[TMP8]] to i32
-; SCCP-NEXT: [[TMP0:%.*]] = zext i32 [[TMP9]] to i64
+; SCCP-NEXT: [[TMP10:%.*]] = zext i32 [[TMP9]] to i64
; SCCP-NEXT: br label [[BB11:%.*]]
; SCCP: bb11:
-; SCCP-NEXT: [[TMP12:%.*]] = phi i64 [ [[TMP0]], [[BB4]] ], [ [[TMP17:%.*]], [[BB18:%.*]] ]
+; SCCP-NEXT: [[TMP12:%.*]] = phi i64 [ [[TMP10]], [[BB4]] ], [ [[TMP17:%.*]], [[BB18:%.*]] ]
; SCCP-NEXT: br label [[BB13:%.*]]
; SCCP: bb13:
; SCCP-NEXT: [[C_1:%.*]] = icmp eq i64 [[TMP12]], 6
@@ -489,10 +489,10 @@ define void @foo(i64* %arg) {
; IPSCCP-NEXT: [[TMP7:%.*]] = sub i64 3, [[TMP6]]
; IPSCCP-NEXT: [[TMP8:%.*]] = shl i64 [[TMP7]], 1
; IPSCCP-NEXT: [[TMP9:%.*]] = trunc i64 [[TMP8]] to i32
-; IPSCCP-NEXT: [[TMP0:%.*]] = zext i32 [[TMP9]] to i64
+; IPSCCP-NEXT: [[TMP10:%.*]] = zext i32 [[TMP9]] to i64
; IPSCCP-NEXT: br label [[BB11:%.*]]
; IPSCCP: bb11:
-; IPSCCP-NEXT: [[TMP12:%.*]] = phi i64 [ [[TMP0]], [[BB4]] ], [ [[TMP17:%.*]], [[BB18:%.*]] ]
+; IPSCCP-NEXT: [[TMP12:%.*]] = phi i64 [ [[TMP10]], [[BB4]] ], [ [[TMP17:%.*]], [[BB18:%.*]] ]
; IPSCCP-NEXT: br label [[BB13:%.*]]
; IPSCCP: bb13:
; IPSCCP-NEXT: [[C_1:%.*]] = icmp eq i64 [[TMP12]], 6
More information about the llvm-commits
mailing list