[PATCH] D60426: [InstCombine] prevent possible miscompile with negate+sdiv of vector op

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: lebedev.ri, shchenz, RKSimon.
Herald added subscribers: hiraditya, mcrosier.
Herald added a project: LLVM.

  // 0 - (X sdiv C)  -> (X sdiv -C)  provided the negation doesn't overflow.

This fold has been around for many years and nobody noticed the potential vector miscompile from overflow until today...
So it seems unlikely that there's much demand for a vector sdiv optimization on arbitrary vector constants, so just limit the matching to splat constants to avoid the possible bug.


https://reviews.llvm.org/D60426

Files:
  llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  llvm/test/Transforms/InstCombine/div.ll


Index: llvm/test/Transforms/InstCombine/div.ll
===================================================================
--- llvm/test/Transforms/InstCombine/div.ll
+++ llvm/test/Transforms/InstCombine/div.ll
@@ -772,7 +772,8 @@
 
 define <2 x i64> @test_exact_vec(<2 x i64> %x) {
 ; CHECK-LABEL: @test_exact_vec(
-; CHECK-NEXT:    [[NEG:%.*]] = sdiv exact <2 x i64> [[X:%.*]], <i64 -3, i64 -4>
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv exact <2 x i64> [[X:%.*]], <i64 3, i64 4>
+; CHECK-NEXT:    [[NEG:%.*]] = sub nsw <2 x i64> zeroinitializer, [[DIV]]
 ; CHECK-NEXT:    ret <2 x i64> [[NEG]]
 ;
   %div = sdiv exact <2 x i64> %x, <i64 3, i64 4>
@@ -812,7 +813,8 @@
 
 define <2 x i8> @negate_sdiv_vec_one_element(<2 x i8> %x) {
 ; CHECK-LABEL: @negate_sdiv_vec_one_element(
-; CHECK-NEXT:    [[NEG:%.*]] = sdiv <2 x i8> [[X:%.*]], <i8 1, i8 -1>
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv <2 x i8> [[X:%.*]], <i8 -1, i8 1>
+; CHECK-NEXT:    [[NEG:%.*]] = sub <2 x i8> zeroinitializer, [[DIV]]
 ; CHECK-NEXT:    ret <2 x i8> [[NEG]]
 ;
   %div = sdiv <2 x i8> %x, <i8 -1, i8 1>
Index: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1692,9 +1692,12 @@
                                   Builder.CreateNot(Y, Y->getName() + ".not"));
 
     // 0 - (X sdiv C)  -> (X sdiv -C)  provided the negation doesn't overflow.
-    if (match(Op1, m_SDiv(m_Value(X), m_Constant(C))) && match(Op0, m_Zero()) &&
-        C->isNotMinSignedValue() && !C->isOneValue()) {
-      auto *BO = BinaryOperator::CreateSDiv(X, ConstantExpr::getNeg(C));
+    // TODO: This could be extended to match arbitrary vector constants.
+    const APInt *DivC;
+    if (match(Op0, m_Zero()) && match(Op1, m_SDiv(m_Value(X), m_APInt(DivC))) &&
+        !DivC->isMinSignedValue() && *DivC != 1) {
+      Constant *NegDivC = ConstantInt::get(I.getType(), -(*DivC));
+      Instruction *BO = BinaryOperator::CreateSDiv(X, NegDivC);
       BO->setIsExact(cast<BinaryOperator>(Op1)->isExact());
       return BO;
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60426.194218.patch
Type: text/x-patch
Size: 2156 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190408/736fc6d8/attachment.bin>