[InstSimplify] Don't miscompile `X == 0 ? abs(X) : -abs(X) --> -abs(X)` xform
authorRoman Lebedev <lebedev.ri@gmail.com>
Fri, 18 Dec 2020 17:29:27 +0000 (20:29 +0300)
committerRoman Lebedev <lebedev.ri@gmail.com>
Fri, 18 Dec 2020 18:18:13 +0000 (21:18 +0300)
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
llvm/test/Transforms/InstSimplify/abs_intrinsic.ll

index 35c21a0..55f3bc4 100644 (file)
@@ -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;
   }
 
index e0bb170..9d5980f 100644 (file)
@@ -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