From e9289dc25f7923abff9ac762bd3f6819c0605f20 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Fri, 18 Dec 2020 20:29:27 +0300 Subject: [PATCH] [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! --- llvm/lib/Analysis/InstructionSimplify.cpp | 9 +++++---- llvm/test/Transforms/InstSimplify/abs_intrinsic.ll | 9 +++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp index 35c21a0..55f3bc4 100644 --- a/llvm/lib/Analysis/InstructionSimplify.cpp +++ b/llvm/lib/Analysis/InstructionSimplify.cpp @@ -4020,11 +4020,12 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal, // X == 0 ? abs(X) : -abs(X) --> -abs(X) // X == 0 ? -abs(X) : abs(X) --> abs(X) - if (match(TrueVal, m_Intrinsic(m_Value(X))) && - match(FalseVal, m_Neg(m_Intrinsic(m_Specific(X))))) + if (match(TrueVal, m_Intrinsic(m_Specific(CmpLHS))) && + match(FalseVal, m_Neg(m_Intrinsic(m_Specific(CmpLHS))))) return FalseVal; - if (match(TrueVal, m_Neg(m_Intrinsic(m_Value(X)))) && - match(FalseVal, m_Intrinsic(m_Specific(X)))) + if (match(TrueVal, + m_Neg(m_Intrinsic(m_Specific(CmpLHS)))) && + match(FalseVal, m_Intrinsic(m_Specific(CmpLHS)))) return FalseVal; } diff --git a/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll b/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll index e0bb170..9d5980f 100644 --- a/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll +++ b/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll @@ -225,7 +225,10 @@ define i32 @select_abs_of_abs_eq(i32 %x) { define i32 @select_abs_of_abs_eq_wrong(i32 %x, i32 %y) { ; CHECK-LABEL: @select_abs_of_abs_eq_wrong( ; CHECK-NEXT: [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false) -; CHECK-NEXT: ret i32 [[ABS]] +; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[ABS]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[Y:%.*]], 0 +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[ABS]] +; CHECK-NEXT: ret i32 [[SEL]] ; %abs = call i32 @llvm.abs.i32(i32 %x, i1 false) %neg = sub i32 0, %abs @@ -264,7 +267,9 @@ define i32 @select_nabs_of_abs_eq_wrong(i32 %x, i32 %y) { ; CHECK-LABEL: @select_nabs_of_abs_eq_wrong( ; CHECK-NEXT: [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false) ; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[ABS]] -; CHECK-NEXT: ret i32 [[NEG]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[Y:%.*]], 0 +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[ABS]], i32 [[NEG]] +; CHECK-NEXT: ret i32 [[SEL]] ; %abs = call i32 @llvm.abs.i32(i32 %x, i1 false) %neg = sub i32 0, %abs -- 2.7.4