[InstCombine] Fix canonicalization of (-X s/ Y) to -(X s/ Y).

[hliao]

Summary

• The previous patch seems quite messy.

[hliao]

Hi Chen

Your change https://reviews.llvm.org/D60395 seems incorrect and cause regressions. Please check whether this does the canonicalization correctly.

Thanks

• Michael

[lebedev.ri]

Was reverted in rL358026 (that entire rL358017 seems faulty, that is not what i have reviewed.).
Let's continue disscussion back in D60395.

[shchenz]

Sorry for the faulty commit and thanks for reverting this @lebedev.ri

[lebedev.ri]

In D60478#1460662, @shchenz wrote:

Sorry for the faulty commit and thanks for reverting this @lebedev.ri

@nikic reverted it

llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

Instruction *InstCombiner::visitSDiv(BinaryOperator &I) {
   Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
   Value *X;
   // sdiv Op0, -1 --> -Op0
   // sdiv Op0, (sext i1 X) --> -Op0 (because if X is 0, the op is undefined)
   if (match(Op1, m_AllOnes()) ||
       (match(Op1, m_SExt(m_Value(X))) && X->getType()->isIntOrIntVectorTy(1)))
     return BinaryOperator::CreateNeg(Op0);
 
+  // -X / Y --> -(X / Y)
+  Value *Y;
+  if (match(&I, m_SDiv(m_OneUse(m_NSWSub(m_Zero(), m_Value(X))), m_Value(Y))))
+    return BinaryOperator::CreateNSWNeg(
+        Builder.CreateSDiv(X, Y, I.getName(), I.isExact()));
+
   // X / INT_MIN --> X == INT_MIN
   if (match(Op1, m_SignMask()))
     return new ZExtInst(Builder.CreateICmpEQ(Op0, Op1), I.getType());
 
   const APInt *Op1C;
   if (match(Op1, m_APInt(Op1C))) {
     // sdiv exact X, C  -->  ashr exact X, log2(C)
     if (I.isExact() && Op1C->isNonNegative() && Op1C->isPowerOf2()) {
if (match(Op0, m_OneUse(m_SExt(m_Value(Op0Src)))) &&
       // minimum signed value because dividing that by -1 is UB. But here, we
       // know that the -1 divisor case is already handled above.
 
       Constant *NarrowDivisor =
           ConstantExpr::getTrunc(cast<Constant>(Op1), Op0Src->getType());
       Value *NarrowOp = Builder.CreateSDiv(Op0Src, NarrowDivisor);
       return new SExtInst(NarrowOp, Op0->getType());
     }
-  
-  // -X / Y --> -(X / Y)
-  Value *Y;
-  if (match(&I, m_SDiv(m_OneUse(m_NSWSub(m_Zero(), m_Value(X))), m_Value(Y))))
-    return BinaryOperator::CreateNSWNeg(
-        Builder.CreateSDiv(X, Y, I.getName(), I.isExact()));
-
     // -X / C --> X / -C (if the negation doesn't overflow).
     // TODO: This could be enhanced to handle arbitrary vector constants by
     //       checking if all elements are not the min-signed-val.
     if (!Op1C->isMinSignedValue() &&
         match(Op0, m_NSWSub(m_Zero(), m_Value(X)))) {
       Constant *NegC = ConstantInt::get(I.getType(), -(*Op1C));
       Instruction *BO = BinaryOperator::CreateSDiv(X, NegC);
       BO->setIsExact(I.isExact());

llvm/test/Transforms/InstCombine/div.ll

;
   %neg = sub nsw <2 x i8> zeroinitializer, %x
   %d = sdiv exact <2 x i8> %neg, <i8 -42, i8 -42>
   ret <2 x i8> %d
 }
 
 define i8 @sdiv_negated_dividend_constant_divisor_smin(i8 %x) {
 ; CHECK-LABEL: @sdiv_negated_dividend_constant_divisor_smin(
 ; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8 [[X:%.*]], -128
-; CHECK-NEXT:    [[D:%.*]] = zext i1 [[TMP1]] to i8
+; CHECK-NEXT:    [[D:%.*]] = sext i1 [[TMP1]] to i8
 ; CHECK-NEXT:    ret i8 [[D]]
 ;
   %neg = sub nsw i8 0, %x
   %d = sdiv i8 %neg, -128
   ret i8 %d
 }
 
 define <2 x i8> @sdiv_negated_dividend_constant_divisor_vec_splat_smin(<2 x i8> %x) {
 ; CHECK-LABEL: @sdiv_negated_dividend_constant_divisor_vec_splat_smin(
 ; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 -128, i8 -128>
-; CHECK-NEXT:    [[D:%.*]] = zext <2 x i1> [[TMP1]] to <2 x i8>
+; CHECK-NEXT:    [[D:%.*]] = sext <2 x i1> [[TMP1]] to <2 x i8>
 ; CHECK-NEXT:    ret <2 x i8> [[D]]
 ;
   %neg = sub nsw <2 x i8> zeroinitializer, %x
   %d = sdiv <2 x i8> %neg, <i8 -128, i8 -128>
   ret <2 x i8> %d
 }
 
 define <2 x i8> @sdiv_negated_dividend_constant_divisor_vec_undef(<2 x i8> %x) {
 ; CHECK-LABEL: @sdiv_negated_dividend_constant_divisor_vec_undef(
 ; CHECK-NEXT:    ret <2 x i8> undef
 ;
   %neg = sub nsw <2 x i8> zeroinitializer, %x
   %d = sdiv <2 x i8> %neg, <i8 -128, i8 undef>
   ret <2 x i8> %d
 }
 
 define <2 x i64> @sdiv_negated_dividend_constant_divisor_vec(<2 x i64> %x) {
 ; CHECK-LABEL: @sdiv_negated_dividend_constant_divisor_vec(
-; CHECK-NEXT:    [[NEG:%.*]] = sub nsw <2 x i64> zeroinitializer, [[X:%.*]]
-; CHECK-NEXT:    [[DIV:%.*]] = sdiv <2 x i64> [[NEG]], <i64 3, i64 4>
-; CHECK-NEXT:    ret <2 x i64> [[DIV]]
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv <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]]
 ;
   %neg = sub nsw <2 x i64> zeroinitializer, %x
   %div = sdiv <2 x i64> %neg, <i64 3, i64 4>
   ret <2 x i64> %div
 }
 
 define <2 x i64> @sdiv_exact_negated_dividend_constant_divisor_vec(<2 x i64> %x) {
 ; CHECK-LABEL: @sdiv_exact_negated_dividend_constant_divisor_vec(
-; CHECK-NEXT:    [[NEG:%.*]] = sub nsw <2 x i64> zeroinitializer, [[X:%.*]]
-; CHECK-NEXT:    [[DIV:%.*]] = sdiv exact <2 x i64> [[NEG]], <i64 3, i64 4>
-; CHECK-NEXT:    ret <2 x i64> [[DIV]]
+; 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]]
 ;
   %neg = sub nsw <2 x i64> zeroinitializer, %x
   %div = sdiv exact <2 x i64> %neg, <i64 3, i64 4>
   ret <2 x i64> %div
 }
 
 ; Can't negate signed min vector element.
 
 define <2 x i8> @sdiv_exact_negated_dividend_constant_divisor_vec_overflow(<2 x i8> %x) {
 ; CHECK-LABEL: @sdiv_exact_negated_dividend_constant_divisor_vec_overflow(
-; CHECK-NEXT:    [[NEG:%.*]] = sub nsw <2 x i8> zeroinitializer, [[X:%.*]]
-; CHECK-NEXT:    [[DIV:%.*]] = sdiv exact <2 x i8> [[NEG]], <i8 -128, i8 42>
-; CHECK-NEXT:    ret <2 x i8> [[DIV]]
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv exact <2 x i8> [[X:%.*]], <i8 -128, i8 42>
+; CHECK-NEXT:    [[NEG:%.*]] = sub nsw <2 x i8> zeroinitializer, [[DIV]]
+; CHECK-NEXT:    ret <2 x i8> [[NEG]]
 ;
   %neg = sub nsw <2 x i8> zeroinitializer, %x
   %div = sdiv exact <2 x i8> %neg, <i8 -128, i8 42>
   ret <2 x i8> %div
 }
 
 define i32 @test35(i32 %A) {
 ; CHECK-LABEL: @test35(