Skip to content

Commit 9f45c5e

Browse files
authored
[InstCombine] Fix infinite loop in select equivalence fold (#84036)
When replacing with a non-constant, it's possible that the result of the simplification is actually more complicated than the original, and may result in an infinite combine loop. Mitigate the issue by requiring that either the replacement or simplification result is constant, which should ensure that it's simpler. While this check is crude, it does not appear to cause optimization regressions in real-world code in practice. Fixes #83127.
1 parent 7a0accc commit 9f45c5e

File tree

2 files changed

+44
-3
lines changed

2 files changed

+44
-3
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1285,7 +1285,11 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
12851285
isGuaranteedNotToBeUndefOrPoison(CmpRHS, SQ.AC, &Sel, &DT)) {
12861286
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, SQ,
12871287
/* AllowRefinement */ true))
1288-
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1288+
// Require either the replacement or the simplification result to be a
1289+
// constant to avoid infinite loops.
1290+
// FIXME: Make this check more precise.
1291+
if (isa<Constant>(CmpRHS) || isa<Constant>(V))
1292+
return replaceOperand(Sel, Swapped ? 2 : 1, V);
12891293

12901294
// Even if TrueVal does not simplify, we can directly replace a use of
12911295
// CmpLHS with CmpRHS, as long as the instruction is not used anywhere
@@ -1303,7 +1307,8 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
13031307
isGuaranteedNotToBeUndefOrPoison(CmpLHS, SQ.AC, &Sel, &DT))
13041308
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, SQ,
13051309
/* AllowRefinement */ true))
1306-
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1310+
if (isa<Constant>(CmpLHS) || isa<Constant>(V))
1311+
return replaceOperand(Sel, Swapped ? 2 : 1, V);
13071312

13081313
auto *FalseInst = dyn_cast<Instruction>(FalseVal);
13091314
if (!FalseInst)

llvm/test/Transforms/InstCombine/select.ll

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2849,12 +2849,14 @@ define i8 @select_replacement_sub(i8 %x, i8 %y, i8 %z) {
28492849
ret i8 %sel
28502850
}
28512851

2852+
; FIXME: This is safe to fold.
28522853
define i8 @select_replacement_shift_noundef(i8 %x, i8 %y, i8 %z) {
28532854
; CHECK-LABEL: @select_replacement_shift_noundef(
28542855
; CHECK-NEXT: [[SHR:%.*]] = lshr exact i8 [[X:%.*]], 1
28552856
; CHECK-NEXT: call void @use_i8(i8 noundef [[SHR]])
28562857
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[SHR]], [[Y:%.*]]
2857-
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[X]], i8 [[Z:%.*]]
2858+
; CHECK-NEXT: [[SHL:%.*]] = shl i8 [[Y]], 1
2859+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[SHL]], i8 [[Z:%.*]]
28582860
; CHECK-NEXT: ret i8 [[SEL]]
28592861
;
28602862
%shr = lshr exact i8 %x, 1
@@ -2904,6 +2906,40 @@ define i32 @select_replacement_loop2(i32 %arg, i32 %arg2) {
29042906
ret i32 %sel
29052907
}
29062908

2909+
define i8 @select_replacement_loop3(i32 noundef %x) {
2910+
; CHECK-LABEL: @select_replacement_loop3(
2911+
; CHECK-NEXT: [[TRUNC:%.*]] = trunc i32 [[X:%.*]] to i8
2912+
; CHECK-NEXT: [[REV:%.*]] = call i8 @llvm.bitreverse.i8(i8 [[TRUNC]])
2913+
; CHECK-NEXT: [[EXT:%.*]] = zext i8 [[REV]] to i32
2914+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[EXT]], [[X]]
2915+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[TRUNC]], i8 0
2916+
; CHECK-NEXT: ret i8 [[SEL]]
2917+
;
2918+
%trunc = trunc i32 %x to i8
2919+
%rev = call i8 @llvm.bitreverse.i8(i8 %trunc)
2920+
%ext = zext i8 %rev to i32
2921+
%cmp = icmp eq i32 %ext, %x
2922+
%sel = select i1 %cmp, i8 %trunc, i8 0
2923+
ret i8 %sel
2924+
}
2925+
2926+
define i16 @select_replacement_loop4(i16 noundef %p_12) {
2927+
; CHECK-LABEL: @select_replacement_loop4(
2928+
; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i16 [[P_12:%.*]], 2
2929+
; CHECK-NEXT: [[AND1:%.*]] = and i16 [[P_12]], 1
2930+
; CHECK-NEXT: [[AND2:%.*]] = select i1 [[CMP1]], i16 [[AND1]], i16 0
2931+
; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i16 [[AND2]], [[P_12]]
2932+
; CHECK-NEXT: [[AND3:%.*]] = select i1 [[CMP2]], i16 [[AND1]], i16 0
2933+
; CHECK-NEXT: ret i16 [[AND3]]
2934+
;
2935+
%cmp1 = icmp ult i16 %p_12, 2
2936+
%and1 = and i16 %p_12, 1
2937+
%and2 = select i1 %cmp1, i16 %and1, i16 0
2938+
%cmp2 = icmp eq i16 %and2, %p_12
2939+
%and3 = select i1 %cmp2, i16 %and1, i16 0
2940+
ret i16 %and3
2941+
}
2942+
29072943
define ptr @select_replacement_gep_inbounds(ptr %base, i64 %offset) {
29082944
; CHECK-LABEL: @select_replacement_gep_inbounds(
29092945
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds i8, ptr [[BASE:%.*]], i64 [[OFFSET:%.*]]

0 commit comments

Comments
 (0)