[llvm] r342632 - [IPSCCP] Fix a problem with removing labels in a switch with undef condition
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 20 02:00:17 PDT 2018
Author: bjope
Date: Thu Sep 20 02:00:17 2018
New Revision: 342632
URL: http://llvm.org/viewvc/llvm-project?rev=342632&view=rev
Log:
[IPSCCP] Fix a problem with removing labels in a switch with undef condition
Summary:
Before removing basic blocks that ipsccp has considered as dead
all uses of the basic block label must be removed. That is done
by calling ConstantFoldTerminator on the users. An exception
is when the branch condition is an undef value. In such
scenarios ipsccp is using some internal assumptions regarding
which edge in the control flow that should remain, while
ConstantFoldTerminator don't know how to fold the terminator.
The problem addressed here is related to ConstantFoldTerminator's
ability to rewrite a 'switch' into a conditional 'br'. In such
situations ConstantFoldTerminator returns true indicating that
the terminator has been rewritten. However, ipsccp treated the
true value as if the edge to the dead basic block had been
removed. So the code for resolving an undef branch condition
did not trigger, and we ended up with assertion that there were
uses remaining when deleting the basic block.
The solution is to resolve indeterminate branches before the
call to ConstantFoldTerminator.
Reviewers: efriedma, fhahn, davide
Reviewed By: fhahn
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D52232
Added:
llvm/trunk/test/Transforms/SCCP/switch-undef-constantfoldterminator.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SCCP.cpp?rev=342632&r1=342631&r2=342632&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Thu Sep 20 02:00:17 2018
@@ -1888,6 +1888,42 @@ static void findReturnsToZap(Function &F
}
}
+// 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));
+ }
+ } else {
+ llvm_unreachable("Unexpected terminator instruction");
+ }
+ if (C) {
+ assert(Solver.isEdgeFeasible(I->getParent(), Dest) &&
+ "Didn't find feasible edge?");
+ (void)Dest;
+
+ I->setOperand(0, C);
+ }
+}
+
+
bool llvm::runIPSCCP(
Module &M, const DataLayout &DL, const TargetLibraryInfo *TLI,
function_ref<std::unique_ptr<PredicateInfo>(Function &)> getPredicateInfo) {
@@ -2017,32 +2053,10 @@ bool llvm::runIPSCCP(
// 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);
bool Folded = ConstantFoldTerminator(I->getParent());
- if (!Folded) {
- // If the branch can't be folded, we must have forced an edge
- // for an indeterminate value. Force the terminator to fold
- // to that edge.
- Constant *C;
- BasicBlock *Dest;
- if (SwitchInst *SI = dyn_cast<SwitchInst>(I)) {
- Dest = SI->case_begin()->getCaseSuccessor();
- C = SI->case_begin()->getCaseValue();
- } else if (BranchInst *BI = dyn_cast<BranchInst>(I)) {
- Dest = BI->getSuccessor(1);
- C = ConstantInt::getFalse(BI->getContext());
- } else if (IndirectBrInst *IBR = dyn_cast<IndirectBrInst>(I)) {
- Dest = IBR->getSuccessor(0);
- C = BlockAddress::get(IBR->getSuccessor(0));
- } else {
- llvm_unreachable("Unexpected terminator instruction");
- }
- assert(Solver.isEdgeFeasible(I->getParent(), Dest) &&
- "Didn't find feasible edge?");
- (void)Dest;
-
- I->setOperand(0, C);
- Folded = ConstantFoldTerminator(I->getParent());
- }
assert(Folded &&
"Expect TermInst on constantint or blockaddress to be folded");
(void) Folded;
Added: llvm/trunk/test/Transforms/SCCP/switch-undef-constantfoldterminator.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SCCP/switch-undef-constantfoldterminator.ll?rev=342632&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/SCCP/switch-undef-constantfoldterminator.ll (added)
+++ llvm/trunk/test/Transforms/SCCP/switch-undef-constantfoldterminator.ll Thu Sep 20 02:00:17 2018
@@ -0,0 +1,47 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -ipsccp -S | FileCheck %s
+
+; This test case used to end up like this:
+;
+; While deleting: label %lor.rhs
+; Use still stuck around after Def is destroyed: br i1 undef, label %lor.rhs, label %land.end
+; opt: ../lib/IR/Value.cpp: llvm::Value::~Value(): Assertion `use_empty() && "Uses remain when a value is destroyed!"' failed.
+;
+; due to ConstantFoldTerminator rewriting the switch into
+;
+; br i1 undef, label %lor.rhs, label %land.end
+;
+; while SCCP implementation relied on the terminator to always be folded into
+; an unconditional branch when ConstantFoldTerminator returned true.
+
+define void @f4() {
+; CHECK-LABEL: define void @f4(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CALL:%.*]] = call i16 @f3(i16 undef)
+; CHECK-NEXT: ret void
+;
+entry:
+ %call = call i16 @f3(i16 undef)
+ ret void
+}
+
+define internal i16 @f3(i16 %p1) {
+; CHECK-LABEL: define internal i16 @f3(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[LAND_END:%.*]]
+; CHECK: land.end:
+; CHECK-NEXT: ret i16 undef
+;
+entry:
+ switch i16 %p1, label %land.end [
+ i16 0, label %land.end
+ i16 1, label %lor.rhs
+ ]
+
+lor.rhs:
+ br label %land.end
+
+land.end:
+ ret i16 0
+}
+
More information about the llvm-commits
mailing list