[llvm-branch-commits] [llvm] e9289dc - [InstSimplify] Don't miscompile `X == 0 ? abs(X) : -abs(X) --> -abs(X)` xform

[Roman Lebedev via llvm-branch-commits]

Author: Roman Lebedev
Date: 2020-12-18T21:18:13+03:00
New Revision: e9289dc25f7923abff9ac762bd3f6819c0605f20

URL: https://github.com/llvm/llvm-project/commit/e9289dc25f7923abff9ac762bd3f6819c0605f20
DIFF: https://github.com/llvm/llvm-project/commit/e9289dc25f7923abff9ac762bd3f6819c0605f20.diff

LOG: [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!

Added: 
    

Modified: 
    llvm/lib/Analysis/InstructionSimplify.cpp
    llvm/test/Transforms/InstSimplify/abs_intrinsic.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 35c21a023bae..55f3bc4f2923 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<Intrinsic::abs>(m_Value(X))) &&
-        match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(X)))))
+    if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))) &&
+        match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))))
       return FalseVal;
-    if (match(TrueVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Value(X)))) &&
-        match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(X))))
+    if (match(TrueVal,
+              m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))) &&
+        match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))))
       return FalseVal;
   }
 

diff  --git a/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll b/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll
index e0bb1704bc48..9d5980fbf782 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