Skip to content

Commit 2b5a897

Browse files
aqjuneaqjune
committed
Revert "[SimpleLoopUnswitch] Fix introduction of UB when hoisted condition may be undef or poison"
.. due to performance regression. This patch is reverted until infrastructore for CSE/LICM support for freeze is added. This reverts commit 181628b
1 parent da02575 commit 2b5a897

File tree

7 files changed

+35
-101
lines changed

7 files changed

+35
-101
lines changed

‎llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
#include "llvm/Analysis/LoopPass.h"
2727
#include "llvm/Analysis/MemorySSA.h"
2828
#include "llvm/Analysis/MemorySSAUpdater.h"
29-
#include "llvm/Analysis/MustExecute.h"
30-
#include "llvm/Analysis/ValueTracking.h"
3129
#include "llvm/IR/BasicBlock.h"
3230
#include "llvm/IR/Constant.h"
3331
#include "llvm/IR/Constants.h"
@@ -181,14 +179,11 @@ static void buildPartialUnswitchConditionalBranch(BasicBlock &BB,
181179
ArrayRef<Value *> Invariants,
182180
bool Direction,
183181
BasicBlock &UnswitchedSucc,
184-
BasicBlock &NormalSucc,
185-
bool InsertFreeze) {
182+
BasicBlock &NormalSucc) {
186183
IRBuilder<> IRB(&BB);
187184

188185
Value *Cond = Direction ? IRB.CreateOr(Invariants) :
189186
IRB.CreateAnd(Invariants);
190-
if (InsertFreeze)
191-
Cond = IRB.CreateFreeze(Cond, Cond->getName() + ".fr");
192187
IRB.CreateCondBr(Cond, Direction ? &UnswitchedSucc : &NormalSucc,
193188
Direction ? &NormalSucc : &UnswitchedSucc);
194189
}
@@ -502,7 +497,7 @@ static bool unswitchTrivialBranch(Loop &L, BranchInst &BI, DominatorTree &DT,
502497
Instruction::And &&
503498
"Must have an `and` of `i1`s for the condition!");
504499
buildPartialUnswitchConditionalBranch(*OldPH, Invariants, ExitDirection,
505-
*UnswitchedBB, *NewPH, false);
500+
*UnswitchedBB, *NewPH);
506501
}
507502

508503
// Update the dominator tree with the added edge.
@@ -2013,10 +2008,6 @@ static void unswitchNontrivialInvariants(
20132008
SE->forgetTopmostLoop(&L);
20142009
}
20152010

2016-
ICFLoopSafetyInfo SafetyInfo(&DT);
2017-
SafetyInfo.computeLoopSafetyInfo(&L);
2018-
bool InsertFreeze = !SafetyInfo.isGuaranteedToExecute(TI, &DT, &L);
2019-
20202011
// If the edge from this terminator to a successor dominates that successor,
20212012
// store a map from each block in its dominator subtree to it. This lets us
20222013
// tell when cloning for a particular successor if a block is dominated by
@@ -2074,12 +2065,6 @@ static void unswitchNontrivialInvariants(
20742065
BasicBlock *ClonedPH = ClonedPHs.begin()->second;
20752066
BI->setSuccessor(ClonedSucc, ClonedPH);
20762067
BI->setSuccessor(1 - ClonedSucc, LoopPH);
2077-
if (InsertFreeze) {
2078-
auto Cond = BI->getCondition();
2079-
if (!isGuaranteedNotToBeUndefOrPoison(Cond))
2080-
BI->setCondition(new FreezeInst(Cond, Cond->getName() + ".fr", BI));
2081-
}
2082-
20832068
DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH});
20842069
} else {
20852070
assert(SI && "Must either be a branch or switch!");
@@ -2094,12 +2079,6 @@ static void unswitchNontrivialInvariants(
20942079
else
20952080
Case.setSuccessor(ClonedPHs.find(Case.getCaseSuccessor())->second);
20962081

2097-
if (InsertFreeze) {
2098-
auto Cond = SI->getCondition();
2099-
if (!isGuaranteedNotToBeUndefOrPoison(Cond))
2100-
SI->setCondition(new FreezeInst(Cond, Cond->getName() + ".fr", SI));
2101-
}
2102-
21032082
// We need to use the set to populate domtree updates as even when there
21042083
// are multiple cases pointing at the same successor we only want to
21052084
// remove and insert one edge in the domtree.
@@ -2176,7 +2155,7 @@ static void unswitchNontrivialInvariants(
21762155
// When doing a partial unswitch, we have to do a bit more work to build up
21772156
// the branch in the split block.
21782157
buildPartialUnswitchConditionalBranch(*SplitBB, Invariants, Direction,
2179-
*ClonedPH, *LoopPH, InsertFreeze);
2158+
*ClonedPH, *LoopPH);
21802159
DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH});
21812160

21822161
if (MSSAU) {

‎llvm/test/Transforms/SimpleLoopUnswitch/exponential-nontrivial-unswitch-nested.ll

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,8 @@
8585

8686
declare void @bar()
8787

88-
define void @loop_nested3_conds5(i32* %addr, i1 %c1i, i1 %c2i, i1 %c3i, i1 %c4i, i1 %c5i) {
88+
define void @loop_nested3_conds5(i32* %addr, i1 %c1, i1 %c2, i1 %c3, i1 %c4, i1 %c5) {
8989
entry:
90-
; c1 ~ c5 are guaranteed to be never undef or poison.
91-
%c1 = freeze i1 %c1i
92-
%c2 = freeze i1 %c2i
93-
%c3 = freeze i1 %c3i
94-
%c4 = freeze i1 %c4i
95-
%c5 = freeze i1 %c5i
9690
%addr1 = getelementptr i32, i32* %addr, i64 0
9791
%addr2 = getelementptr i32, i32* %addr, i64 1
9892
%addr3 = getelementptr i32, i32* %addr, i64 2

‎llvm/test/Transforms/SimpleLoopUnswitch/exponential-nontrivial-unswitch-nested2.ll

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,8 @@
9797

9898
declare void @bar()
9999

100-
define void @loop_nested3_conds5(i32* %addr, i1 %c1i, i1 %c2i, i1 %c3i, i1 %c4i, i1 %c5i) {
100+
define void @loop_nested3_conds5(i32* %addr, i1 %c1, i1 %c2, i1 %c3, i1 %c4, i1 %c5) {
101101
entry:
102-
; c1 ~ c5 are guaranteed to be never undef or poison.
103-
%c1 = freeze i1 %c1i
104-
%c2 = freeze i1 %c2i
105-
%c3 = freeze i1 %c3i
106-
%c4 = freeze i1 %c4i
107-
%c5 = freeze i1 %c5i
108102
%addr1 = getelementptr i32, i32* %addr, i64 0
109103
%addr2 = getelementptr i32, i32* %addr, i64 1
110104
%addr3 = getelementptr i32, i32* %addr, i64 2

‎llvm/test/Transforms/SimpleLoopUnswitch/exponential-switch-unswitch.ll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,8 @@
8686
; LOOP-MAX-COUNT-111: Loop at depth 2 containing:
8787
; LOOP-MAX-NOT: Loop at depth 2 containing:
8888

89-
define i32 @loop_switch(i32* %addr, i32 %c1i, i32 %c2i) {
89+
define i32 @loop_switch(i32* %addr, i32 %c1, i32 %c2) {
9090
entry:
91-
; c1, c2 are guaranteed to be never undef or poison.
92-
%c1 = freeze i32 %c1i
93-
%c2 = freeze i32 %c2i
9491
%addr1 = getelementptr i32, i32* %addr, i64 0
9592
%addr2 = getelementptr i32, i32* %addr, i64 1
9693
%check0 = icmp eq i32 %c2, 0

‎llvm/test/Transforms/SimpleLoopUnswitch/guards.ll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ exit:
8181
define void @test_conditional_guards(i1 %cond, i32 %N) {
8282
; CHECK-LABEL: @test_conditional_guards(
8383
; CHECK-NEXT: entry:
84-
; CHECK-NEXT: [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]
85-
; CHECK-NEXT: br i1 [[COND_FR]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
84+
; CHECK-NEXT: br i1 [[COND:%.*]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
8685
; CHECK: entry.split.us:
8786
; CHECK-NEXT: br label [[LOOP_US:%.*]]
8887
; CHECK: loop.us:

‎llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-cost.ll

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ define void @test_unswitch(i1* %ptr, i1 %cond) {
5757
entry:
5858
br label %loop_begin
5959
; CHECK-NEXT: entry:
60-
; CHECK-NEXT: %[[COND_FR:.*]] = freeze i1 %cond
61-
; CHECK-NEXT: br i1 %[[COND_FR]], label %entry.split.us, label %entry.split
60+
; CHECK-NEXT: br i1 %cond, label %entry.split.us, label %entry.split
6261

6362
loop_begin:
6463
call void @x()
@@ -128,8 +127,7 @@ define void @test_unswitch_non_dup_code(i1* %ptr, i1 %cond) {
128127
entry:
129128
br label %loop_begin
130129
; CHECK-NEXT: entry:
131-
; CHECK-NEXT: %[[COND_FR:.*]] = freeze i1 %cond
132-
; CHECK-NEXT: br i1 %[[COND_FR]], label %entry.split.us, label %entry.split
130+
; CHECK-NEXT: br i1 %cond, label %entry.split.us, label %entry.split
133131

134132
loop_begin:
135133
call void @x()
@@ -210,8 +208,7 @@ define void @test_unswitch_non_dup_code_in_cfg(i1* %ptr, i1 %cond) {
210208
entry:
211209
br label %loop_begin
212210
; CHECK-NEXT: entry:
213-
; CHECK-NEXT: %[[COND_FR:.*]] = freeze i1 %cond
214-
; CHECK-NEXT: br i1 %[[COND_FR]], label %entry.split.us, label %entry.split
211+
; CHECK-NEXT: br i1 %cond, label %entry.split.us, label %entry.split
215212

216213
loop_begin:
217214
call void @x()
@@ -367,8 +364,7 @@ define void @test_unswitch_large_exit(i1* %ptr, i1 %cond) {
367364
entry:
368365
br label %loop_begin
369366
; CHECK-NEXT: entry:
370-
; CHECK-NEXT: %[[COND_FR:.*]] = freeze i1 %cond
371-
; CHECK-NEXT: br i1 %[[COND_FR]], label %entry.split.us, label %entry.split
367+
; CHECK-NEXT: br i1 %cond, label %entry.split.us, label %entry.split
372368

373369
loop_begin:
374370
call void @x()
@@ -445,8 +441,7 @@ define void @test_unswitch_dedicated_exiting(i1* %ptr, i1 %cond) {
445441
entry:
446442
br label %loop_begin
447443
; CHECK-NEXT: entry:
448-
; CHECK-NEXT: %[[COND_FR:.*]] = freeze i1 %cond
449-
; CHECK-NEXT: br i1 %[[COND_FR]], label %entry.split.us, label %entry.split
444+
; CHECK-NEXT: br i1 %cond, label %entry.split.us, label %entry.split
450445

451446
loop_begin:
452447
call void @x()

0 commit comments

Comments
 (0)