[InstCombine] Optimize multiplication where both operands are negated

[zjaffal]

Summary

Handle the case where both operands are negated in matrix multiplication

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1870–1875

I commented earlier that we prefer creating a new instruction in transforms like this, but we might as well follow the code structure of this related transform. It doesn't seem to bother the worklist iteration.

llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll

126

Fix test comment.
Add FMF to at least one of these tests, so we can see that those are propagated.

138–150

Fix test comment.
Add extra uses to at least one of these tests (they should not change the transform).

[thegameg]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1837

Do we need to handle integers too? (m_Neg)

[zjaffal]

update transformations

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1834

I read "~" as "bitwise not". I expect this comment to be something like this:
// -[A] * -[B] --> [A] * [B]

1838–1839

The casting is unnecessary, and the code doesn't match the code comment. Capture the source values as "A" and "B" inside the match() statement (the same way it is done in the existing variants of this transform).

[zjaffal]

use patterns

[fhahn]

LGTM, with an additional nit. Let's wait a bit with landing in case there are any further comments.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1833

nit: comment should be full sentences: capitalize first letter, end with period.

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1833

IMO, the words are just noise. The formula is clear enough.

1837

Again - it's easier to read when the code matches the comments:
Value *A, *B;

[zjaffal]

rename variables

[zjaffal]

rename variables

[fhahn]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1833

According to other comments in the file, that should just be -A * -B -> A * B

[zjaffal]

Variable names match comments

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1833

The "->" suggestion was not addressed, and we're back to "Optimise" which was requested to be changed/deleted.

Also, renaming the operands "NegA" and "NegB" doesn't make things clearer. In the planned next step to this patch, one of those names will be known to be incorrect. The extra words/code are not helping things.

Other than that, I have no more comments for this patch.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

if ((match(Arg0, m_OneUse(m_FNeg(m_Value(X)))) && Arg1 == X) ||
       Value *R = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, II);
       if (IID == Intrinsic::minimum || IID == Intrinsic::minnum)
         R = Builder.CreateFNegFMF(R, II);
       return replaceInstUsesWith(*II, R);
     }
 
     break;
   }
+  case Intrinsic::matrix_multiply: {
+    // -A * -B -> A * B
+    Value *A, *B;
+    if (match(II->getArgOperand(0), m_FNeg(m_Value(A))) &&
+        match(II->getArgOperand(1), m_FNeg(m_Value(B)))) {
+      replaceOperand(*II, 0, A);
+      replaceOperand(*II, 1, B);
+      return II;
+    }
+    break;
+  }
   case Intrinsic::fmuladd: {
     // Canonicalize fast fmuladd to the separate fmul + fadd.
     if (II->isFast()) {
       BuilderTy::FastMathFlagGuard Guard(Builder);
       Builder.setFastMathFlags(II->getFastMathFlags());
       Value *Mul = Builder.CreateFMul(II->getArgOperand(0),
                                       II->getArgOperand(1));
       Value *Add = Builder.CreateFAdd(Mul, II->getArgOperand(2));
case Intrinsic::fmuladd: {
     }
 
     [[fallthrough]];
   }
   case Intrinsic::fma: {
     // fma fneg(x), fneg(y), z -> fma x, y, z
     Value *Src0 = II->getArgOperand(0);
     Value *Src1 = II->getArgOperand(1);
     Value *X, *Y;
     if (match(Src0, m_FNeg(m_Value(X))) && match(Src1, m_FNeg(m_Value(Y)))) {
       replaceOperand(*II, 0, X);
       replaceOperand(*II, 1, Y);
       return II;
     }
 
     // fma fabs(x), fabs(x), z -> fma x, x, z
     if (match(Src0, m_FAbs(m_Value(X))) &&
         match(Src1, m_FAbs(m_Specific(X)))) {
       replaceOperand(*II, 0, X);
       replaceOperand(*II, 1, X);
       return II;
     }

llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll

;
   %res = tail call <15 x double> @llvm.matrix.multiply.v15f64.v3f64.v5f64(<3 x double> %a, <5 x double> %b, i32 3, i32 1, i32 5)
   %res.2 = fneg <15 x double> %res
   ret <15 x double> %res.2
 }
 
 ; both negations can be deleted
 define <2 x double> @test_with_two_operands_negated1(<6 x double> %a, <3 x double> %b){
 ; CHECK-LABEL: @test_with_two_operands_negated1(
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <6 x double> [[A:%.*]]
-; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <3 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = tail call <2 x double> @llvm.matrix.multiply.v2f64.v6f64.v3f64(<6 x double> [[A_NEG]], <3 x double> [[B_NEG]], i32 2, i32 3, i32 1)
+; CHECK-NEXT:    [[RES:%.*]] = tail call <2 x double> @llvm.matrix.multiply.v2f64.v6f64.v3f64(<6 x double> [[A:%.*]], <3 x double> [[B:%.*]], i32 2, i32 3, i32 1)
 ; CHECK-NEXT:    ret <2 x double> [[RES]]
 ;
   %a.neg = fneg <6 x double> %a
   %b.neg = fneg <3 x double> %b
   %res = tail call <2 x double> @llvm.matrix.multiply.v2f64.v6f64.v3f64(<6 x double> %a.neg, <3 x double> %b.neg, i32 2, i32 3, i32 1)
   ret <2 x double> %res
 }
 
 ; both negations will be removed
 define <9 x double> @test_with_two_operands_negated2(<27 x double> %a, <3 x double> %b){
 ; CHECK-LABEL: @test_with_two_operands_negated2(
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <27 x double> [[A:%.*]]
-; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <3 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> [[A_NEG]], <3 x double> [[B_NEG]], i32 9, i32 3, i32 1)
+; CHECK-NEXT:    [[RES:%.*]] = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> [[A:%.*]], <3 x double> [[B:%.*]], i32 9, i32 3, i32 1)
 ; CHECK-NEXT:    ret <9 x double> [[RES]]
 ;
   %a.neg = fneg <27 x double> %a
   %b.neg = fneg <3 x double> %b
   %res = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> %a.neg, <3 x double> %b.neg, i32 9, i32 3, i32 1)
   ret <9 x double> %res
 }
 
 ; both negations will be removed
 define <9 x double> @test_with_two_operands_negated_with_fastflags(<27 x double> %a, <3 x double> %b){
 ; CHECK-LABEL: @test_with_two_operands_negated_with_fastflags(
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <27 x double> [[A:%.*]]
-; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <3 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = tail call fast <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> [[A_NEG]], <3 x double> [[B_NEG]], i32 9, i32 3, i32 1)
+; CHECK-NEXT:    [[RES:%.*]] = tail call fast <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> [[A:%.*]], <3 x double> [[B:%.*]], i32 9, i32 3, i32 1)
 ; CHECK-NEXT:    ret <9 x double> [[RES]]
 ;
   %a.neg = fneg <27 x double> %a
   %b.neg = fneg <3 x double> %b
   %res = tail call fast <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> %a.neg, <3 x double> %b.neg, i32 9, i32 3, i32 1)
   ret <9 x double> %res
 }
 
 ; both negations should be removed
 define <9 x double> @test_with_two_operands_negated2_commute(<3 x double> %a, <27 x double> %b){
 ; CHECK-LABEL: @test_with_two_operands_negated2_commute(
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <3 x double> [[A:%.*]]
-; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <27 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = call <9 x double> @llvm.matrix.multiply.v9f64.v3f64.v27f64(<3 x double> [[A_NEG]], <27 x double> [[B_NEG]], i32 1, i32 3, i32 9)
+; CHECK-NEXT:    [[RES:%.*]] = call <9 x double> @llvm.matrix.multiply.v9f64.v3f64.v27f64(<3 x double> [[A:%.*]], <27 x double> [[B:%.*]], i32 1, i32 3, i32 9)
 ; CHECK-NEXT:    ret <9 x double> [[RES]]
 ;
   %a.neg = fneg <3 x double> %a
   %b.neg = fneg <27 x double> %b
   %res = call <9 x double> @llvm.matrix.multiply.v9f64.v3f64.v27f64(<3 x double> %a.neg, <27 x double> %b.neg, i32 1, i32 3, i32 9)
   ret <9 x double> %res
 }
 
 define <4 x double> @matrix_multiply_two_operands_negated_with_same_size(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @matrix_multiply_two_operands_negated_with_same_size(
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <2 x double> [[A:%.*]]
-; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <2 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = call <4 x double> @llvm.matrix.multiply.v4f64.v2f64.v2f64(<2 x double> [[A_NEG]], <2 x double> [[B_NEG]], i32 2, i32 1, i32 2)
+; CHECK-NEXT:    [[RES:%.*]] = call <4 x double> @llvm.matrix.multiply.v4f64.v2f64.v2f64(<2 x double> [[A:%.*]], <2 x double> [[B:%.*]], i32 2, i32 1, i32 2)
 ; CHECK-NEXT:    ret <4 x double> [[RES]]
 ;
   %a.neg = fneg <2 x double> %a
   %b.neg = fneg <2 x double> %b
   %res = call <4 x double> @llvm.matrix.multiply.v4f64.v2f64.v2f64(<2 x double> %a.neg, <2 x double> %b.neg, i32 2, i32 1, i32 2)
   ret <4 x double> %res
 }
 
 define <2 x double> @matrix_multiply_two_operands_with_multiple_uses(<6 x double> %a, <3 x double> %b) {
 ; CHECK-LABEL: @matrix_multiply_two_operands_with_multiple_uses(
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <6 x double> [[A:%.*]]
-; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <3 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = tail call <2 x double> @llvm.matrix.multiply.v2f64.v6f64.v3f64(<6 x double> [[A_NEG]], <3 x double> [[B_NEG]], i32 2, i32 3, i32 1)
-; CHECK-NEXT:    [[RES_2:%.*]] = shufflevector <6 x double> [[A_NEG]], <6 x double> undef, <2 x i32> <i32 0, i32 1>
-; CHECK-NEXT:    [[RES_3:%.*]] = fadd <2 x double> [[RES_2]], [[RES]]
+; CHECK-NEXT:    [[RES:%.*]] = tail call <2 x double> @llvm.matrix.multiply.v2f64.v6f64.v3f64(<6 x double> [[A:%.*]], <3 x double> [[B:%.*]], i32 2, i32 3, i32 1)
+; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <6 x double> [[A]], <6 x double> poison, <2 x i32> <i32 0, i32 1>
+; CHECK-NEXT:    [[RES_3:%.*]] = fsub <2 x double> [[RES]], [[TMP1]]
 ; CHECK-NEXT:    ret <2 x double> [[RES_3]]
 ;
   %a.neg = fneg <6 x double> %a
   %b.neg = fneg <3 x double> %b
   %res = tail call <2 x double> @llvm.matrix.multiply.v2f64.v6f64.v3f64(<6 x double> %a.neg, <3 x double> %b.neg, i32 2, i32 3, i32 1)
   %res.2 = shufflevector <6 x double> %a.neg, <6 x double> undef,
   <2 x i32>  <i32 0, i32 1>
   %res.3 = fadd <2 x double> %res.2, %res
   ret <2 x double> %res.3
 }
 
 define <9 x double> @matrix_multiply_two_operands_with_multiple_uses2(<27 x double> %a, <3 x double> %b, ptr %a_loc, ptr %b_loc){
 ; CHECK-LABEL: @matrix_multiply_two_operands_with_multiple_uses2(
 ; CHECK-NEXT:    [[A_NEG:%.*]] = fneg <27 x double> [[A:%.*]]
 ; CHECK-NEXT:    [[B_NEG:%.*]] = fneg <3 x double> [[B:%.*]]
-; CHECK-NEXT:    [[RES:%.*]] = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> [[A_NEG]], <3 x double> [[B_NEG]], i32 9, i32 3, i32 1)
+; CHECK-NEXT:    [[RES:%.*]] = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> [[A]], <3 x double> [[B]], i32 9, i32 3, i32 1)
 ; CHECK-NEXT:    store <27 x double> [[A_NEG]], ptr [[A_LOC:%.*]], align 256
 ; CHECK-NEXT:    store <3 x double> [[B_NEG]], ptr [[B_LOC:%.*]], align 32
 ; CHECK-NEXT:    ret <9 x double> [[RES]]
 ;
   %a.neg = fneg <27 x double> %a
   %b.neg = fneg <3 x double> %b
   %res = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> %a.neg, <3 x double> %b.neg, i32 9, i32 3, i32 1)
   store <27 x double> %a.neg, ptr %a_loc