Skip to content

Commit 8a84b28

Browse files
authored
[SimplifyCFG] Eliminate dead edges of switches according to the domain of conditions (#165748)
In simplifycfg/cvp/sccp, we eliminate dead edges of switches according to the knownbits/range info of conditions. However, these approximations may not meet the real-world needs when the domain of condition values is sparse. For example, if the condition can only be either -3 or 3, we cannot prove that the condition never evaluates to 1 (knownbits: ???????1, range: [-3, 4)). This patch adds a helper function `collectPossibleValues` to enumerate all the possible values of V. To fix the motivating issue, `eliminateDeadSwitchCases` will use the result to remove dead edges. Note: In https://discourse.llvm.org/t/missed-optimization-due-to-overflow-check/88700 I proposed a new value lattice kind to represent such values. But I find it hard to apply because the transition becomes much complicated. Compile-time impact looks neutral: https://llvm-compile-time-tracker.com/compare.php?from=32d6b2139a6c8f79e074e8c6cfe0cc9e79c4c0c8&to=e47c26e3f1bf9eb062684dda4fafce58438e994b&stat=instructions:u This patch removes many dead error-handling codes: dtcxzyw/llvm-opt-benchmark#3012 Closes #165179.
1 parent 747050b commit 8a84b28

File tree

8 files changed

+237
-36
lines changed

8 files changed

+237
-36
lines changed

llvm/include/llvm/Analysis/ValueTracking.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,6 +1024,16 @@ findValuesAffectedByCondition(Value *Cond, bool IsAssume,
10241024
LLVM_ABI Value *stripNullTest(Value *V);
10251025
LLVM_ABI const Value *stripNullTest(const Value *V);
10261026

1027+
/// Enumerates all possible values of V and inserts them into the set \p
1028+
/// Constants. If \p AllowUndefOrPoison is false, it fails when V may contain
1029+
/// undef/poison elements. Returns true if the result is complete. Otherwise,
1030+
/// the result is incomplete (more than MaxCount values).
1031+
/// NOTE: The constant values are not distinct.
1032+
LLVM_ABI bool
1033+
collectPossibleValues(const Value *V,
1034+
SmallPtrSetImpl<const Constant *> &Constants,
1035+
unsigned MaxCount, bool AllowUndefOrPoison = true);
1036+
10271037
} // end namespace llvm
10281038

10291039
#endif // LLVM_ANALYSIS_VALUETRACKING_H

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10341,3 +10341,55 @@ const Value *llvm::stripNullTest(const Value *V) {
1034110341
Value *llvm::stripNullTest(Value *V) {
1034210342
return const_cast<Value *>(stripNullTest(const_cast<const Value *>(V)));
1034310343
}
10344+
10345+
bool llvm::collectPossibleValues(const Value *V,
10346+
SmallPtrSetImpl<const Constant *> &Constants,
10347+
unsigned MaxCount, bool AllowUndefOrPoison) {
10348+
SmallPtrSet<const Instruction *, 8> Visited;
10349+
SmallVector<const Instruction *, 8> Worklist;
10350+
auto Push = [&](const Value *V) -> bool {
10351+
if (auto *C = dyn_cast<Constant>(V)) {
10352+
if (!AllowUndefOrPoison && !isGuaranteedNotToBeUndefOrPoison(C))
10353+
return false;
10354+
// Check existence first to avoid unnecessary allocations.
10355+
if (Constants.contains(C))
10356+
return true;
10357+
if (Constants.size() == MaxCount)
10358+
return false;
10359+
Constants.insert(C);
10360+
return true;
10361+
}
10362+
10363+
if (auto *Inst = dyn_cast<Instruction>(V)) {
10364+
if (Visited.insert(Inst).second)
10365+
Worklist.push_back(Inst);
10366+
return true;
10367+
}
10368+
return false;
10369+
};
10370+
if (!Push(V))
10371+
return false;
10372+
while (!Worklist.empty()) {
10373+
const Instruction *CurInst = Worklist.pop_back_val();
10374+
switch (CurInst->getOpcode()) {
10375+
case Instruction::Select:
10376+
if (!Push(CurInst->getOperand(1)))
10377+
return false;
10378+
if (!Push(CurInst->getOperand(2)))
10379+
return false;
10380+
break;
10381+
case Instruction::PHI:
10382+
for (Value *IncomingValue : cast<PHINode>(CurInst)->incoming_values()) {
10383+
// Fast path for recurrence PHI.
10384+
if (IncomingValue == CurInst)
10385+
continue;
10386+
if (!Push(IncomingValue))
10387+
return false;
10388+
}
10389+
break;
10390+
default:
10391+
return false;
10392+
}
10393+
}
10394+
return true;
10395+
}

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6020,6 +6020,8 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
60206020
const DataLayout &DL) {
60216021
Value *Cond = SI->getCondition();
60226022
KnownBits Known = computeKnownBits(Cond, DL, AC, SI);
6023+
SmallPtrSet<const Constant *, 4> KnownValues;
6024+
bool IsKnownValuesValid = collectPossibleValues(Cond, KnownValues, 4);
60236025

60246026
// We can also eliminate cases by determining that their values are outside of
60256027
// the limited range of the condition based on how many significant (non-sign)
@@ -6039,15 +6041,18 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
60396041
UniqueSuccessors.push_back(Successor);
60406042
++It->second;
60416043
}
6042-
const APInt &CaseVal = Case.getCaseValue()->getValue();
6044+
ConstantInt *CaseC = Case.getCaseValue();
6045+
const APInt &CaseVal = CaseC->getValue();
60436046
if (Known.Zero.intersects(CaseVal) || !Known.One.isSubsetOf(CaseVal) ||
6044-
(CaseVal.getSignificantBits() > MaxSignificantBitsInCond)) {
6045-
DeadCases.push_back(Case.getCaseValue());
6047+
(CaseVal.getSignificantBits() > MaxSignificantBitsInCond) ||
6048+
(IsKnownValuesValid && !KnownValues.contains(CaseC))) {
6049+
DeadCases.push_back(CaseC);
60466050
if (DTU)
60476051
--NumPerSuccessorCases[Successor];
60486052
LLVM_DEBUG(dbgs() << "SimplifyCFG: switch case " << CaseVal
60496053
<< " is dead.\n");
6050-
}
6054+
} else if (IsKnownValuesValid)
6055+
KnownValues.erase(CaseC);
60516056
}
60526057

60536058
// If we can prove that the cases must cover all possible values, the
@@ -6058,33 +6063,41 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
60586063
const unsigned NumUnknownBits =
60596064
Known.getBitWidth() - (Known.Zero | Known.One).popcount();
60606065
assert(NumUnknownBits <= Known.getBitWidth());
6061-
if (HasDefault && DeadCases.empty() &&
6062-
NumUnknownBits < 64 /* avoid overflow */) {
6063-
uint64_t AllNumCases = 1ULL << NumUnknownBits;
6064-
if (SI->getNumCases() == AllNumCases) {
6066+
if (HasDefault && DeadCases.empty()) {
6067+
if (IsKnownValuesValid && all_of(KnownValues, IsaPred<UndefValue>)) {
60656068
createUnreachableSwitchDefault(SI, DTU);
60666069
return true;
60676070
}
6068-
// When only one case value is missing, replace default with that case.
6069-
// Eliminating the default branch will provide more opportunities for
6070-
// optimization, such as lookup tables.
6071-
if (SI->getNumCases() == AllNumCases - 1) {
6072-
assert(NumUnknownBits > 1 && "Should be canonicalized to a branch");
6073-
IntegerType *CondTy = cast<IntegerType>(Cond->getType());
6074-
if (CondTy->getIntegerBitWidth() > 64 ||
6075-
!DL.fitsInLegalInteger(CondTy->getIntegerBitWidth()))
6076-
return false;
60776071

6078-
uint64_t MissingCaseVal = 0;
6079-
for (const auto &Case : SI->cases())
6080-
MissingCaseVal ^= Case.getCaseValue()->getValue().getLimitedValue();
6081-
auto *MissingCase =
6082-
cast<ConstantInt>(ConstantInt::get(Cond->getType(), MissingCaseVal));
6083-
SwitchInstProfUpdateWrapper SIW(*SI);
6084-
SIW.addCase(MissingCase, SI->getDefaultDest(), SIW.getSuccessorWeight(0));
6085-
createUnreachableSwitchDefault(SI, DTU, /*RemoveOrigDefaultBlock*/ false);
6086-
SIW.setSuccessorWeight(0, 0);
6087-
return true;
6072+
if (NumUnknownBits < 64 /* avoid overflow */) {
6073+
uint64_t AllNumCases = 1ULL << NumUnknownBits;
6074+
if (SI->getNumCases() == AllNumCases) {
6075+
createUnreachableSwitchDefault(SI, DTU);
6076+
return true;
6077+
}
6078+
// When only one case value is missing, replace default with that case.
6079+
// Eliminating the default branch will provide more opportunities for
6080+
// optimization, such as lookup tables.
6081+
if (SI->getNumCases() == AllNumCases - 1) {
6082+
assert(NumUnknownBits > 1 && "Should be canonicalized to a branch");
6083+
IntegerType *CondTy = cast<IntegerType>(Cond->getType());
6084+
if (CondTy->getIntegerBitWidth() > 64 ||
6085+
!DL.fitsInLegalInteger(CondTy->getIntegerBitWidth()))
6086+
return false;
6087+
6088+
uint64_t MissingCaseVal = 0;
6089+
for (const auto &Case : SI->cases())
6090+
MissingCaseVal ^= Case.getCaseValue()->getValue().getLimitedValue();
6091+
auto *MissingCase = cast<ConstantInt>(
6092+
ConstantInt::get(Cond->getType(), MissingCaseVal));
6093+
SwitchInstProfUpdateWrapper SIW(*SI);
6094+
SIW.addCase(MissingCase, SI->getDefaultDest(),
6095+
SIW.getSuccessorWeight(0));
6096+
createUnreachableSwitchDefault(SI, DTU,
6097+
/*RemoveOrigDefaultBlock*/ false);
6098+
SIW.setSuccessorWeight(0, 0);
6099+
return true;
6100+
}
60886101
}
60896102
}
60906103

llvm/test/CodeGen/AArch64/arm64-ccmp.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,12 +430,12 @@ declare i32 @foo()
430430

431431
; Test case distilled from 126.gcc.
432432
; The phi in sw.bb.i.i gets multiple operands for the %entry predecessor.
433-
define void @build_modify_expr() nounwind ssp {
433+
define void @build_modify_expr(i32 %cond) nounwind ssp {
434434
; CHECK-LABEL: build_modify_expr:
435435
; CHECK: ; %bb.0: ; %entry
436436
; CHECK-NEXT: ret
437437
entry:
438-
switch i32 undef, label %sw.bb.i.i [
438+
switch i32 %cond, label %sw.bb.i.i [
439439
i32 69, label %if.end85
440440
i32 70, label %if.end85
441441
i32 71, label %if.end85

llvm/test/CodeGen/Hexagon/vect/zext-v4i1.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
66
target datalayout = "e-m:e-p:32:32:32-a:0-n16:32-i64:64:64-i32:32:32-i16:16:16-i1:8:8-f32:32:32-f64:64:64-v32:32:32-v64:64:64-v512:512:512-v1024:1024:1024-v2048:2048:2048"
77
target triple = "hexagon"
88

9-
define i32 @fred(ptr %a0) #0 {
9+
define i32 @fred(ptr %a0, i32 %cond) #0 {
1010
; CHECK-LABEL: fred:
1111
; CHECK: // %bb.0: // %b0
1212
; CHECK-NEXT: {
13-
; CHECK-NEXT: if (p0) jump:nt .LBB0_2
13+
; CHECK-NEXT: p0 = cmp.eq(r1,#5); if (!p0.new) jump:t .LBB0_2
1414
; CHECK-NEXT: }
1515
; CHECK-NEXT: // %bb.1: // %b2
1616
; CHECK-NEXT: {
@@ -40,7 +40,7 @@ define i32 @fred(ptr %a0) #0 {
4040
; CHECK-NEXT: jumpr r31
4141
; CHECK-NEXT: }
4242
b0:
43-
switch i32 undef, label %b14 [
43+
switch i32 %cond, label %b14 [
4444
i32 5, label %b2
4545
i32 3, label %b1
4646
]

llvm/test/Transforms/SimplifyCFG/switch-on-const-select.ll renamed to llvm/test/Transforms/SimplifyCFG/switch-on-const.ll

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,132 @@ bees:
154154
unreachable
155155
}
156156

157+
define void @pr165179(i1 %cond) {
158+
; CHECK-LABEL: @pr165179(
159+
; CHECK-NEXT: entry:
160+
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
161+
; CHECK: if.then:
162+
; CHECK-NEXT: tail call void @bees.a() #[[ATTR0]]
163+
; CHECK-NEXT: br label [[SWITCHBB:%.*]]
164+
; CHECK: if.else:
165+
; CHECK-NEXT: tail call void @bees.b() #[[ATTR0]]
166+
; CHECK-NEXT: br label [[SWITCHBB]]
167+
; CHECK: exit:
168+
; CHECK-NEXT: tail call void @bees.a() #[[ATTR0]]
169+
; CHECK-NEXT: ret void
170+
;
171+
entry:
172+
br i1 %cond, label %if.then, label %if.else
173+
174+
if.then:
175+
tail call void @bees.a() nounwind
176+
br label %switchbb
177+
178+
if.else:
179+
tail call void @bees.b() nounwind
180+
br label %switchbb
181+
182+
switchbb:
183+
%cond1 = phi i32 [ 1, %if.else ], [ -1, %if.then ]
184+
switch i32 %cond1, label %default [
185+
i32 1, label %exit
186+
i32 -1, label %exit
187+
]
188+
189+
exit:
190+
tail call void @bees.a() nounwind
191+
ret void
192+
193+
default:
194+
tail call void @bees.b() nounwind
195+
ret void
196+
}
197+
198+
define void @switch_remove_dead_case_phi(i1 %cond1, i1 %cond2) {
199+
; CHECK-LABEL: @switch_remove_dead_case_phi(
200+
; CHECK-NEXT: entry:
201+
; CHECK-NEXT: br i1 [[COND1:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
202+
; CHECK: if.then:
203+
; CHECK-NEXT: tail call void @bees.a() #[[ATTR0]]
204+
; CHECK-NEXT: br i1 [[COND2:%.*]], label [[SWITCHBB:%.*]], label [[IF_ELSE]]
205+
; CHECK: if.else:
206+
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ 3, [[ENTRY:%.*]] ], [ -1, [[IF_THEN]] ]
207+
; CHECK-NEXT: tail call void @bees.b() #[[ATTR0]]
208+
; CHECK-NEXT: br label [[SWITCHBB]]
209+
; CHECK: switchbb:
210+
; CHECK-NEXT: [[COND:%.*]] = phi i32 [ [[PHI]], [[IF_ELSE]] ], [ 5, [[IF_THEN]] ]
211+
; CHECK-NEXT: [[COND3:%.*]] = icmp eq i32 [[COND]], -1
212+
; CHECK-NEXT: br i1 [[COND3]], label [[EXIT:%.*]], label [[DEFAULT:%.*]]
213+
; CHECK: common.ret:
214+
; CHECK-NEXT: ret void
215+
; CHECK: exit:
216+
; CHECK-NEXT: tail call void @bees.a() #[[ATTR0]]
217+
; CHECK-NEXT: br label [[COMMON_RET:%.*]]
218+
; CHECK: default:
219+
; CHECK-NEXT: tail call void @bees.b() #[[ATTR0]]
220+
; CHECK-NEXT: br label [[COMMON_RET]]
221+
;
222+
entry:
223+
br i1 %cond1, label %if.then, label %if.else
224+
225+
if.then:
226+
tail call void @bees.a() nounwind
227+
br i1 %cond2, label %switchbb, label %if.else
228+
229+
if.else:
230+
%phi = phi i32 [ 3, %entry ], [ -1, %if.then ]
231+
tail call void @bees.b() nounwind
232+
br label %switchbb
233+
234+
switchbb:
235+
%cond = phi i32 [ %phi, %if.else ], [ 5, %if.then ]
236+
switch i32 %cond, label %default [
237+
i32 1, label %exit
238+
i32 -1, label %exit
239+
]
240+
241+
exit:
242+
tail call void @bees.a() nounwind
243+
ret void
244+
245+
default:
246+
tail call void @bees.b() nounwind
247+
ret void
248+
}
249+
250+
define void @switch_remove_dead_case_select(i1 %cond1, i1 %cond2) {
251+
; CHECK-LABEL: @switch_remove_dead_case_select(
252+
; CHECK-NEXT: entry:
253+
; CHECK-NEXT: [[X:%.*]] = select i1 [[COND1:%.*]], i32 -1, i32 3
254+
; CHECK-NEXT: [[Y:%.*]] = select i1 [[COND2:%.*]], i32 [[X]], i32 5
255+
; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[Y]], -1
256+
; CHECK-NEXT: br i1 [[COND]], label [[EXIT:%.*]], label [[DEFAULT:%.*]]
257+
; CHECK: common.ret:
258+
; CHECK-NEXT: ret void
259+
; CHECK: exit:
260+
; CHECK-NEXT: tail call void @bees.a() #[[ATTR0]]
261+
; CHECK-NEXT: br label [[COMMON_RET:%.*]]
262+
; CHECK: default:
263+
; CHECK-NEXT: tail call void @bees.b() #[[ATTR0]]
264+
; CHECK-NEXT: br label [[COMMON_RET]]
265+
;
266+
entry:
267+
%x = select i1 %cond1, i32 -1, i32 3
268+
%y = select i1 %cond2, i32 %x, i32 5
269+
switch i32 %y, label %default [
270+
i32 1, label %exit
271+
i32 -1, label %exit
272+
]
273+
274+
exit:
275+
tail call void @bees.a() nounwind
276+
ret void
277+
278+
default:
279+
tail call void @bees.b() nounwind
280+
ret void
281+
}
282+
157283
declare void @llvm.trap() nounwind noreturn
158284
declare void @bees.a() nounwind
159285
declare void @bees.b() nounwind

llvm/test/Transforms/SimplifyCFG/switch_mask.ll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ define i1 @pr88607() {
221221
; CHECK-NEXT: entry:
222222
; CHECK-NEXT: [[COND:%.*]] = select i1 false, i32 4, i32 1
223223
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 false, i32 2, i32 [[COND]]
224+
; CHECK-NEXT: [[COND1:%.*]] = icmp eq i32 [[SPEC_SELECT]], 1
224225
; CHECK-NEXT: ret i1 false
225226
;
226227
entry:

llvm/test/Transforms/SimplifyCFG/switch_undef.ll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@
55
define void @f6() #0 {
66
; CHECK-LABEL: @f6(
77
; CHECK-NEXT: entry:
8-
; CHECK-NEXT: br label [[FOR_COND_I:%.*]]
9-
; CHECK: for.cond.i:
8+
; CHECK-NEXT: br label [[F1_EXIT_I:%.*]]
9+
; CHECK: f1.exit.i:
1010
; CHECK-NEXT: [[TOBOOL7_I:%.*]] = icmp ne i16 1, 0
11-
; CHECK-NEXT: br label [[FOR_COND_I]]
11+
; CHECK-NEXT: br label [[F1_EXIT_I]]
1212
;
13-
1413
entry:
1514
br label %for.cond.i
1615

0 commit comments

Comments
 (0)