Skip to content

Commit 94d8f15

Browse files
nikictstellar
authored andcommitted
[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. (cherry picked from commit 9f45c5e)
1 parent 4c36ecb commit 94d8f15

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
@@ -1284,7 +1284,11 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
12841284
isGuaranteedNotToBeUndefOrPoison(CmpRHS, SQ.AC, &Sel, &DT)) {
12851285
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, SQ,
12861286
/* AllowRefinement */ true))
1287-
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1287+
// Require either the replacement or the simplification result to be a
1288+
// constant to avoid infinite loops.
1289+
// FIXME: Make this check more precise.
1290+
if (isa<Constant>(CmpRHS) || isa<Constant>(V))
1291+
return replaceOperand(Sel, Swapped ? 2 : 1, V);
12881292

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

13071312
auto *FalseInst = dyn_cast<Instruction>(FalseVal);
13081313
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)