Skip to content

Commit e9289dc

Browse files
committed
[InstSimplify] Don't miscompile X == 0 ? abs(X) : -abs(X) --> -abs(X) xform
The transform wasn't checking that the LHS of the comparison *is* the `X` in question... This is the miscompile that was holding up D87188. Thanks to Dave Green for producing an actionable reproducer!
1 parent 9b183a1 commit e9289dc

File tree

2 files changed

+12
-6
lines changed

2 files changed

+12
-6
lines changed

llvm/lib/Analysis/InstructionSimplify.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4020,11 +4020,12 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal,
40204020

40214021
// X == 0 ? abs(X) : -abs(X) --> -abs(X)
40224022
// X == 0 ? -abs(X) : abs(X) --> abs(X)
4023-
if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Value(X))) &&
4024-
match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(X)))))
4023+
if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))) &&
4024+
match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))))
40254025
return FalseVal;
4026-
if (match(TrueVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Value(X)))) &&
4027-
match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(X))))
4026+
if (match(TrueVal,
4027+
m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))) &&
4028+
match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))))
40284029
return FalseVal;
40294030
}
40304031

llvm/test/Transforms/InstSimplify/abs_intrinsic.ll

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,10 @@ define i32 @select_abs_of_abs_eq(i32 %x) {
225225
define i32 @select_abs_of_abs_eq_wrong(i32 %x, i32 %y) {
226226
; CHECK-LABEL: @select_abs_of_abs_eq_wrong(
227227
; CHECK-NEXT: [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false)
228-
; CHECK-NEXT: ret i32 [[ABS]]
228+
; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[ABS]]
229+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[Y:%.*]], 0
230+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[ABS]]
231+
; CHECK-NEXT: ret i32 [[SEL]]
229232
;
230233
%abs = call i32 @llvm.abs.i32(i32 %x, i1 false)
231234
%neg = sub i32 0, %abs
@@ -264,7 +267,9 @@ define i32 @select_nabs_of_abs_eq_wrong(i32 %x, i32 %y) {
264267
; CHECK-LABEL: @select_nabs_of_abs_eq_wrong(
265268
; CHECK-NEXT: [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false)
266269
; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[ABS]]
267-
; CHECK-NEXT: ret i32 [[NEG]]
270+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[Y:%.*]], 0
271+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[ABS]], i32 [[NEG]]
272+
; CHECK-NEXT: ret i32 [[SEL]]
268273
;
269274
%abs = call i32 @llvm.abs.i32(i32 %x, i1 false)
270275
%neg = sub i32 0, %abs

0 commit comments

Comments
 (0)