Skip to content

Commit d76ea25

Browse files
artagnonnikic
andauthored
Reland [InstCombine] Teach foldSelectOpOp about samesign (llvm#124320)
Changes: There was a serious bug in the previous patch, leading to a miscompile. See llvm#122723 for the miscompile report from Alexander, and the follow-up investigation by Nikita. The patch has since been reworked, and now includes the testcase from the miscompile. Follow up on 4a0d53a (PatternMatch: migrate to CmpPredicate) to get rid of one of the FIXMEs it introduced by replacing a predicate comparison with CmpPredicate::getMatching. Co-authored-by: Nikita Popov <[email protected]>
1 parent ef92e6b commit d76ea25

File tree

2 files changed

+131
-13
lines changed

2 files changed

+131
-13
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -423,24 +423,34 @@ Instruction *InstCombinerImpl::foldSelectOpOp(SelectInst &SI, Instruction *TI,
423423
}
424424
}
425425

426+
auto CreateCmpSel = [&](std::optional<CmpPredicate> P,
427+
bool Swapped) -> CmpInst * {
428+
if (!P)
429+
return nullptr;
430+
auto *MatchOp = getCommonOp(TI, FI, ICmpInst::isEquality(*P),
431+
ICmpInst::isRelational(*P) && Swapped);
432+
if (!MatchOp)
433+
return nullptr;
434+
Value *NewSel = Builder.CreateSelect(Cond, OtherOpT, OtherOpF,
435+
SI.getName() + ".v", &SI);
436+
return new ICmpInst(MatchIsOpZero ? *P
437+
: ICmpInst::getSwappedCmpPredicate(*P),
438+
MatchOp, NewSel);
439+
};
440+
426441
// icmp with a common operand also can have the common operand
427442
// pulled after the select.
428443
CmpPredicate TPred, FPred;
429444
if (match(TI, m_ICmp(TPred, m_Value(), m_Value())) &&
430445
match(FI, m_ICmp(FPred, m_Value(), m_Value()))) {
431-
// FIXME: Use CmpPredicate::getMatching here.
432-
CmpInst::Predicate T = TPred, F = FPred;
433-
if (T == F || T == ICmpInst::getSwappedCmpPredicate(F)) {
434-
bool Swapped = T != F;
435-
if (Value *MatchOp =
436-
getCommonOp(TI, FI, ICmpInst::isEquality(TPred), Swapped)) {
437-
Value *NewSel = Builder.CreateSelect(Cond, OtherOpT, OtherOpF,
438-
SI.getName() + ".v", &SI);
439-
return new ICmpInst(
440-
MatchIsOpZero ? TPred : ICmpInst::getSwappedCmpPredicate(TPred),
441-
MatchOp, NewSel);
442-
}
443-
}
446+
if (auto *R =
447+
CreateCmpSel(CmpPredicate::getMatching(TPred, FPred), false))
448+
return R;
449+
if (auto *R =
450+
CreateCmpSel(CmpPredicate::getMatching(
451+
TPred, ICmpInst::getSwappedCmpPredicate(FPred)),
452+
true))
453+
return R;
444454
}
445455
}
446456

llvm/test/Transforms/InstCombine/select-cmp.ll

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ define i1 @icmp_ne_common_op00(i1 %c, i6 %x, i6 %y, i6 %z) {
2323
ret i1 %r
2424
}
2525

26+
define i1 @icmp_ne_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
27+
; CHECK-LABEL: @icmp_ne_samesign_common(
28+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
29+
; CHECK-NEXT: [[R:%.*]] = icmp ne i6 [[X:%.*]], [[R_V]]
30+
; CHECK-NEXT: ret i1 [[R]]
31+
;
32+
%cmp1 = icmp samesign ne i6 %x, %y
33+
%cmp2 = icmp ne i6 %x, %z
34+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
35+
ret i1 %r
36+
}
37+
2638
define i1 @icmp_ne_common_op01(i1 %c, i3 %x, i3 %y, i3 %z) {
2739
; CHECK-LABEL: @icmp_ne_common_op01(
2840
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i3 [[Y:%.*]], i3 [[Z:%.*]]
@@ -71,6 +83,18 @@ define i1 @icmp_eq_common_op00(i1 %c, i5 %x, i5 %y, i5 %z) {
7183
ret i1 %r
7284
}
7385

86+
define i1 @icmp_eq_samesign_common(i1 %c, i5 %x, i5 %y, i5 %z) {
87+
; CHECK-LABEL: @icmp_eq_samesign_common(
88+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i5 [[Y:%.*]], i5 [[Z:%.*]]
89+
; CHECK-NEXT: [[R:%.*]] = icmp eq i5 [[X:%.*]], [[R_V]]
90+
; CHECK-NEXT: ret i1 [[R]]
91+
;
92+
%cmp1 = icmp eq i5 %x, %y
93+
%cmp2 = icmp samesign eq i5 %x, %z
94+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
95+
ret i1 %r
96+
}
97+
7498
define <5 x i1> @icmp_eq_common_op01(<5 x i1> %c, <5 x i7> %x, <5 x i7> %y, <5 x i7> %z) {
7599
; CHECK-LABEL: @icmp_eq_common_op01(
76100
; CHECK-NEXT: [[R_V:%.*]] = select <5 x i1> [[C:%.*]], <5 x i7> [[Y:%.*]], <5 x i7> [[Z:%.*]]
@@ -134,6 +158,18 @@ define i1 @icmp_slt_common(i1 %c, i6 %x, i6 %y, i6 %z) {
134158
ret i1 %r
135159
}
136160

161+
define i1 @icmp_slt_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
162+
; CHECK-LABEL: @icmp_slt_samesign_common(
163+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
164+
; CHECK-NEXT: [[R:%.*]] = icmp slt i6 [[X:%.*]], [[R_V]]
165+
; CHECK-NEXT: ret i1 [[R]]
166+
;
167+
%cmp1 = icmp samesign ult i6 %x, %y
168+
%cmp2 = icmp slt i6 %x, %z
169+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
170+
ret i1 %r
171+
}
172+
137173
define i1 @icmp_sgt_common(i1 %c, i6 %x, i6 %y, i6 %z) {
138174
; CHECK-LABEL: @icmp_sgt_common(
139175
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
@@ -146,6 +182,18 @@ define i1 @icmp_sgt_common(i1 %c, i6 %x, i6 %y, i6 %z) {
146182
ret i1 %r
147183
}
148184

185+
define i1 @icmp_sgt_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
186+
; CHECK-LABEL: @icmp_sgt_samesign_common(
187+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
188+
; CHECK-NEXT: [[R:%.*]] = icmp sgt i6 [[X:%.*]], [[R_V]]
189+
; CHECK-NEXT: ret i1 [[R]]
190+
;
191+
%cmp1 = icmp samesign ugt i6 %x, %y
192+
%cmp2 = icmp sgt i6 %x, %z
193+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
194+
ret i1 %r
195+
}
196+
149197
define i1 @icmp_sle_common(i1 %c, i6 %x, i6 %y, i6 %z) {
150198
; CHECK-LABEL: @icmp_sle_common(
151199
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
@@ -158,6 +206,18 @@ define i1 @icmp_sle_common(i1 %c, i6 %x, i6 %y, i6 %z) {
158206
ret i1 %r
159207
}
160208

209+
define i1 @icmp_sle_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
210+
; CHECK-LABEL: @icmp_sle_samesign_common(
211+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
212+
; CHECK-NEXT: [[R:%.*]] = icmp sge i6 [[X:%.*]], [[R_V]]
213+
; CHECK-NEXT: ret i1 [[R]]
214+
;
215+
%cmp1 = icmp sle i6 %y, %x
216+
%cmp2 = icmp samesign ule i6 %z, %x
217+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
218+
ret i1 %r
219+
}
220+
161221
define i1 @icmp_sge_common(i1 %c, i6 %x, i6 %y, i6 %z) {
162222
; CHECK-LABEL: @icmp_sge_common(
163223
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
@@ -170,6 +230,18 @@ define i1 @icmp_sge_common(i1 %c, i6 %x, i6 %y, i6 %z) {
170230
ret i1 %r
171231
}
172232

233+
define i1 @icmp_sge_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
234+
; CHECK-LABEL: @icmp_sge_samesign_common(
235+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
236+
; CHECK-NEXT: [[R:%.*]] = icmp sle i6 [[X:%.*]], [[R_V]]
237+
; CHECK-NEXT: ret i1 [[R]]
238+
;
239+
%cmp1 = icmp sge i6 %y, %x
240+
%cmp2 = icmp samesign uge i6 %z, %x
241+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
242+
ret i1 %r
243+
}
244+
173245
define i1 @icmp_slt_sgt_common(i1 %c, i6 %x, i6 %y, i6 %z) {
174246
; CHECK-LABEL: @icmp_slt_sgt_common(
175247
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
@@ -182,6 +254,18 @@ define i1 @icmp_slt_sgt_common(i1 %c, i6 %x, i6 %y, i6 %z) {
182254
ret i1 %r
183255
}
184256

257+
define i1 @icmp_slt_sgt_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
258+
; CHECK-LABEL: @icmp_slt_sgt_samesign_common(
259+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
260+
; CHECK-NEXT: [[R:%.*]] = icmp slt i6 [[X:%.*]], [[R_V]]
261+
; CHECK-NEXT: ret i1 [[R]]
262+
;
263+
%cmp1 = icmp samesign ult i6 %x, %y
264+
%cmp2 = icmp sgt i6 %z, %x
265+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
266+
ret i1 %r
267+
}
268+
185269
define i1 @icmp_sle_sge_common(i1 %c, i6 %x, i6 %y, i6 %z) {
186270
; CHECK-LABEL: @icmp_sle_sge_common(
187271
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
@@ -194,6 +278,18 @@ define i1 @icmp_sle_sge_common(i1 %c, i6 %x, i6 %y, i6 %z) {
194278
ret i1 %r
195279
}
196280

281+
define i1 @icmp_sle_sge_samesign_common(i1 %c, i6 %x, i6 %y, i6 %z) {
282+
; CHECK-LABEL: @icmp_sle_sge_samesign_common(
283+
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
284+
; CHECK-NEXT: [[R:%.*]] = icmp sge i6 [[X:%.*]], [[R_V]]
285+
; CHECK-NEXT: ret i1 [[R]]
286+
;
287+
%cmp1 = icmp sle i6 %y, %x
288+
%cmp2 = icmp samesign uge i6 %x, %z
289+
%r = select i1 %c, i1 %cmp1, i1 %cmp2
290+
ret i1 %r
291+
}
292+
197293
define i1 @icmp_ult_common(i1 %c, i6 %x, i6 %y, i6 %z) {
198294
; CHECK-LABEL: @icmp_ult_common(
199295
; CHECK-NEXT: [[R_V:%.*]] = select i1 [[C:%.*]], i6 [[Y:%.*]], i6 [[Z:%.*]]
@@ -700,5 +796,17 @@ define i1 @sel_icmp_cmp_and_no_simplify_comm(i1 %c, i32 %a1, i32 %a2, i8 %b) {
700796
ret i1 %cmp
701797
}
702798

799+
define i1 @icmp_lt_slt(i1 %c, i32 %arg) {
800+
; CHECK-LABEL: @icmp_lt_slt(
801+
; CHECK-NEXT: [[SELECT_V:%.*]] = select i1 [[C:%.*]], i32 131072, i32 0
802+
; CHECK-NEXT: [[SELECT:%.*]] = icmp slt i32 [[ARG:%.*]], [[SELECT_V]]
803+
; CHECK-NEXT: ret i1 [[SELECT]]
804+
;
805+
%cmp1 = icmp samesign ult i32 %arg, 131072
806+
%cmp2 = icmp slt i32 %arg, 0
807+
%select = select i1 %c, i1 %cmp1, i1 %cmp2
808+
ret i1 %select
809+
}
810+
703811
declare void @use(i1)
704812
declare void @use.i8(i8)

0 commit comments

Comments
 (0)