[llvm] r361881 - Revert "[CorrelatedValuePropagation] Fix prof branch_weights metadata handling for SwitchInst"
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue May 28 14:28:25 PDT 2019
Author: nikic
Date: Tue May 28 14:28:24 2019
New Revision: 361881
URL: http://llvm.org/viewvc/llvm-project?rev=361881&view=rev
Log:
Revert "[CorrelatedValuePropagation] Fix prof branch_weights metadata handling for SwitchInst"
This reverts commit 53f2f3286572cb879b3861d7c15480e4d830dd3b.
As reported on D62126, this causes assertion failures if the switch
has incorrect branch_weights metadata, which may happen as a result
of other transforms not handling it correctly yet.
Removed:
llvm/trunk/test/Transforms/CorrelatedValuePropagation/profmd.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
Modified: llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp?rev=361881&r1=361880&r2=361881&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp Tue May 28 14:28:24 2019
@@ -306,11 +306,11 @@ static bool processCmp(CmpInst *Cmp, Laz
/// that cannot fire no matter what the incoming edge can safely be removed. If
/// a case fires on every incoming edge then the entire switch can be removed
/// and replaced with a branch to the case destination.
-static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI,
+static bool processSwitch(SwitchInst *SI, LazyValueInfo *LVI,
DominatorTree *DT) {
DomTreeUpdater DTU(*DT, DomTreeUpdater::UpdateStrategy::Lazy);
- Value *Cond = I->getCondition();
- BasicBlock *BB = I->getParent();
+ Value *Cond = SI->getCondition();
+ BasicBlock *BB = SI->getParent();
// If the condition was defined in same block as the switch then LazyValueInfo
// currently won't say anything useful about it, though in theory it could.
@@ -327,72 +327,67 @@ static bool processSwitch(SwitchInst *I,
for (auto *Succ : successors(BB))
SuccessorsCount[Succ]++;
- { // Scope for SwitchInstProfUpdateWrapper. It must not live during
- // ConstantFoldTerminator() as the underlying SwitchInst can be changed.
- SwitchInstProfUpdateWrapper SI(*I);
-
- for (auto CI = SI->case_begin(), CE = SI->case_end(); CI != CE;) {
- ConstantInt *Case = CI->getCaseValue();
-
- // Check to see if the switch condition is equal to/not equal to the case
- // value on every incoming edge, equal/not equal being the same each time.
- LazyValueInfo::Tristate State = LazyValueInfo::Unknown;
- for (pred_iterator PI = PB; PI != PE; ++PI) {
- // Is the switch condition equal to the case value?
- LazyValueInfo::Tristate Value = LVI->getPredicateOnEdge(CmpInst::ICMP_EQ,
- Cond, Case, *PI,
- BB, SI);
- // Give up on this case if nothing is known.
- if (Value == LazyValueInfo::Unknown) {
- State = LazyValueInfo::Unknown;
- break;
- }
-
- // If this was the first edge to be visited, record that all other edges
- // need to give the same result.
- if (PI == PB) {
- State = Value;
- continue;
- }
-
- // If this case is known to fire for some edges and known not to fire for
- // others then there is nothing we can do - give up.
- if (Value != State) {
- State = LazyValueInfo::Unknown;
- break;
- }
+ for (auto CI = SI->case_begin(), CE = SI->case_end(); CI != CE;) {
+ ConstantInt *Case = CI->getCaseValue();
+
+ // Check to see if the switch condition is equal to/not equal to the case
+ // value on every incoming edge, equal/not equal being the same each time.
+ LazyValueInfo::Tristate State = LazyValueInfo::Unknown;
+ for (pred_iterator PI = PB; PI != PE; ++PI) {
+ // Is the switch condition equal to the case value?
+ LazyValueInfo::Tristate Value = LVI->getPredicateOnEdge(CmpInst::ICMP_EQ,
+ Cond, Case, *PI,
+ BB, SI);
+ // Give up on this case if nothing is known.
+ if (Value == LazyValueInfo::Unknown) {
+ State = LazyValueInfo::Unknown;
+ break;
}
- if (State == LazyValueInfo::False) {
- // This case never fires - remove it.
- BasicBlock *Succ = CI->getCaseSuccessor();
- Succ->removePredecessor(BB);
- CI = SI.removeCase(CI);
- CE = SI->case_end();
-
- // The condition can be modified by removePredecessor's PHI simplification
- // logic.
- Cond = SI->getCondition();
-
- ++NumDeadCases;
- Changed = true;
- if (--SuccessorsCount[Succ] == 0)
- DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB, Succ}});
+ // If this was the first edge to be visited, record that all other edges
+ // need to give the same result.
+ if (PI == PB) {
+ State = Value;
continue;
}
- if (State == LazyValueInfo::True) {
- // This case always fires. Arrange for the switch to be turned into an
- // unconditional branch by replacing the switch condition with the case
- // value.
- SI->setCondition(Case);
- NumDeadCases += SI->getNumCases();
- Changed = true;
+
+ // If this case is known to fire for some edges and known not to fire for
+ // others then there is nothing we can do - give up.
+ if (Value != State) {
+ State = LazyValueInfo::Unknown;
break;
}
+ }
- // Increment the case iterator since we didn't delete it.
- ++CI;
+ if (State == LazyValueInfo::False) {
+ // This case never fires - remove it.
+ BasicBlock *Succ = CI->getCaseSuccessor();
+ Succ->removePredecessor(BB);
+ CI = SI->removeCase(CI);
+ CE = SI->case_end();
+
+ // The condition can be modified by removePredecessor's PHI simplification
+ // logic.
+ Cond = SI->getCondition();
+
+ ++NumDeadCases;
+ Changed = true;
+ if (--SuccessorsCount[Succ] == 0)
+ DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB, Succ}});
+ continue;
+ }
+ if (State == LazyValueInfo::True) {
+ // This case always fires. Arrange for the switch to be turned into an
+ // unconditional branch by replacing the switch condition with the case
+ // value.
+ SI->setCondition(Case);
+ NumDeadCases += SI->getNumCases();
+ Changed = true;
+ break;
}
+
+ // Increment the case iterator since we didn't delete it.
+ ++CI;
}
if (Changed)
Removed: llvm/trunk/test/Transforms/CorrelatedValuePropagation/profmd.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CorrelatedValuePropagation/profmd.ll?rev=361880&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/CorrelatedValuePropagation/profmd.ll (original)
+++ llvm/trunk/test/Transforms/CorrelatedValuePropagation/profmd.ll (removed)
@@ -1,119 +0,0 @@
-; RUN: opt < %s -correlated-propagation -S | FileCheck %s
-
-; Removed several cases from switch.
-define i32 @switch1(i32 %s) {
-; CHECK-LABEL: @switch1(
-; CHECK-NEXT: entry:
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[S:%.*]], 0
-; CHECK-NEXT: br i1 [[CMP]], label [[NEGATIVE:%.*]], label [[OUT:%.*]]
-;
-entry:
- %cmp = icmp slt i32 %s, 0
- br i1 %cmp, label %negative, label %out
-
-negative:
-; CHECK: negative:
-; CHECK-NEXT: switch i32 [[S]], label [[OUT]] [
-; CHECK-NEXT: i32 -2, label [[NEXT:%.*]]
-; CHECK-NEXT: i32 -1, label [[NEXT]]
- switch i32 %s, label %out [
- i32 0, label %out
- i32 1, label %out
- i32 -1, label %next
- i32 -2, label %next
- i32 2, label %out
- i32 3, label %out
-; CHECK-NEXT: !prof ![[MD0:[0-9]+]]
- ], !prof !{!"branch_weights", i32 99, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6}
-
-out:
- %p = phi i32 [ 1, %entry ], [ -1, %negative ], [ -1, %negative ], [ -1, %negative ], [ -1, %negative ], [ -1, %negative ]
- ret i32 %p
-
-next:
- %q = phi i32 [ 0, %negative ], [ 0, %negative ]
- ret i32 %q
-}
-
-; Removed all cases from switch.
-define i32 @switch2(i32 %s) {
-; CHECK-LABEL: @switch2(
-; CHECK-NEXT: entry:
-; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[S:%.*]], 0
-; CHECK-NEXT: br i1 [[CMP]], label [[POSITIVE:%.*]], label [[OUT:%.*]]
-;
-entry:
- %cmp = icmp sgt i32 %s, 0
- br i1 %cmp, label %positive, label %out
-
-positive:
- switch i32 %s, label %out [
- i32 0, label %out
- i32 -1, label %next
- i32 -2, label %next
- ], !prof !{!"branch_weights", i32 99, i32 1, i32 2, i32 3}
-
-out:
- %p = phi i32 [ -1, %entry ], [ 1, %positive ], [ 1, %positive ]
- ret i32 %p
-
-next:
- %q = phi i32 [ 0, %positive ], [ 0, %positive ]
- ret i32 %q
-}
-
-; Change switch into conditional branch.
-define i32 @switch3(i32 %s) {
-; CHECK-LABEL: @switch3(
-;
-entry:
- %cmp = icmp sgt i32 %s, 0
- br i1 %cmp, label %positive, label %out
-
-positive:
-; CHECK: positive:
-; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 %s, 1
-; CHECK-NEXT: br i1 [[CMP]], label [[NEXT:%.*]], label [[OUT:%.*]], !prof ![[MD1:[0-9]+]]
- switch i32 %s, label %out [
- i32 1, label %next
- i32 -1, label %next
- i32 -2, label %next
- ], !prof !{!"branch_weights", i32 99, i32 1, i32 2, i32 3}
-
-out:
- %p = phi i32 [ -1, %entry ], [ 1, %positive ]
- ret i32 %p
-
-next:
- %q = phi i32 [ 0, %positive ], [ 0, %positive ], [ 0, %positive ]
- ret i32 %q
-}
-
-; Removed all cases from switch.
-define i32 @switch4(i32 %s) {
-; CHECK-LABEL: @switch4(
-; CHECK-NEXT: entry:
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[S:%.*]], 0
-; CHECK-NEXT: br i1 [[CMP]], label [[NEGATIVE:%.*]], label [[OUT:%.*]]
-;
-entry:
- %cmp = icmp slt i32 %s, 0
- br i1 %cmp, label %negative, label %out
-
-negative:
-; CHECK: negative:
-; CHECK-NEXT: br label %out
- switch i32 %s, label %out [
- i32 0, label %out
- i32 1, label %out
- i32 2, label %out
- i32 3, label %out
- ], !prof !{!"branch_weights", i32 99, i32 1, i32 2, i32 3, i32 4}
-
-out:
- %p = phi i32 [ 1, %entry ], [ -1, %negative ], [ -1, %negative ], [ -1, %negative ], [ -1, %negative ], [ -1, %negative ]
- ret i32 %p
-}
-
-; CHECK: ![[MD0]] = !{!"branch_weights", i32 99, i32 4, i32 3}
-; CHECK: ![[MD1]] = !{!"branch_weights", i32 1, i32 99}
More information about the llvm-commits
mailing list