Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] DAG combine (mul (add x, 1), y) -> vmadd #71495

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

ChunyuLiao
Copy link
Member

@ChunyuLiao ChunyuLiao commented Nov 7, 2023

vmadd: (mul (add x, 1), y) -> (add (mul x, y), y)
       (mul x, add (y, 1)) -> (add x, (mul x, y))
vnmsub: (mul (sub 1, x), y) -> (sub y, (mul x, y))
        (mul x, (sub 1, y)) -> (sub x, (mul x, y))

Comparison with gcc:
vmadd: https://gcc.godbolt.org/z/xjePx87Y7
vnsub: https://gcc.godbolt.org/z/b17zG7nT1

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 7, 2023

@llvm/pr-subscribers-backend-risc-v

Author: Liao Chunyu (ChunyuLiao)

Changes

Comparison with gcc: https://gcc.godbolt.org/z/xjePx87Y7


Full diff: https://github.com/llvm/llvm-project/pull/71495.diff

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td (+18)
  • (modified) llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll (+15)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
index d92d3975d12f533..bd4a08cd3859398 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
@@ -2292,6 +2292,24 @@ defm : VPatMultiplyAddVL_VV_VX<riscv_sub_vl, "PseudoVNMSUB">;
 defm : VPatMultiplyAccVL_VV_VX<riscv_add_vl_oneuse, "PseudoVMACC">;
 defm : VPatMultiplyAccVL_VV_VX<riscv_sub_vl_oneuse, "PseudoVNMSAC">;
 
+// mul_vl(v, (add_vl v1, splat 1)) is a special case of vmadd.
+foreach vti = AllIntegerVectors in {
+  let Predicates = GetVTypePredicates<vti>.Predicates in {
+    // NOTE: We choose VMADD because it has the most commuting freedom. So it
+    // works best with how TwoAddressInstructionPass tries commuting.
+    def : Pat<(vti.Vector
+              (riscv_mul_vl vti.RegClass:$rs1,
+                 (riscv_add_vl_oneuse vti.RegClass:$rd,
+                                      (vti.Vector (riscv_vmv_v_x_vl
+                                      (vti.Vector undef), 1, VLOpFrag)),
+                                      srcvalue, (vti.Mask true_mask), VLOpFrag),
+                            srcvalue, (vti.Mask true_mask), VLOpFrag)),
+              (!cast<Instruction>("PseudoVMADD_VV_"#vti.LMul.MX)
+                   vti.RegClass:$rd, vti.RegClass:$rs1, vti.RegClass:$rs1,
+                   GPR:$vl, vti.Log2SEW, TAIL_AGNOSTIC)>;
+  }
+}
+
 // 11.14. Vector Widening Integer Multiply-Add Instructions
 defm : VPatWidenMultiplyAddVL_VV_VX<riscv_vwmacc_vl, "PseudoVWMACC">;
 defm : VPatWidenMultiplyAddVL_VV_VX<riscv_vwmaccu_vl, "PseudoVWMACCU">;
diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll
index c95d144a970895c..a7f7f8c4a8136c9 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll
@@ -8237,3 +8237,18 @@ define void @mulhs_vx_v2i64(ptr %x) {
   store <2 x i64> %b, ptr %x
   ret void
 }
+
+define void @madd_vv_v2i64(ptr %x, <2 x i64> %y) {
+; CHECK-LABEL: madd_vv_v2i64:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetivli zero, 2, e64, m1, ta, ma
+; CHECK-NEXT:    vle64.v v9, (a0)
+; CHECK-NEXT:    vmadd.vv v9, v8, v8
+; CHECK-NEXT:    vse64.v v9, (a0)
+; CHECK-NEXT:    ret
+  %a = load <2 x i64>, ptr %x
+  %b = add <2 x i64> %a, <i64 1, i64 1>
+  %c = mul <2 x i64> %b, %y
+  store <2 x i64> %c, ptr %x
+  ret void
+}

@wangpc-pp
Copy link
Contributor

Can we do this in DAGCombine? We already have a large MatcherTable now.
And scalable tests are needed too.

@ChunyuLiao
Copy link
Member Author

Can we do this in DAGCombine? We already have a large MatcherTable now.
And scalable tests are needed too.

Thanks, I can implement this feature in DAGCombine. I'd like to get more advice on which of these two ways is better.
But it seems a backend tends to use td? I can't find the source now.

@topperc
Copy link
Collaborator

topperc commented Nov 8, 2023

Looks like AArch64 does this same transform in performMulCombine. They also handle a subtract case.

@ChunyuLiao ChunyuLiao changed the title [RISCV] Match mul_vl(v, (add_vl v1, splat 1)) to vmadd_vl [RISCV] DAG combine (mul (add x, 1), y) -> vmadd Nov 20, 2023
@ChunyuLiao
Copy link
Member Author

Move to performMULCombine. And add more test cases. Thanks

@@ -12423,6 +12423,45 @@ static SDValue performXORCombine(SDNode *N, SelectionDAG &DAG,
return combineSelectAndUseCommutative(N, DAG, /*AllOnes*/ false, Subtarget);
}

static SDValue performMULCombine(SDNode *N, SelectionDAG &DAG,
const RISCVSubtarget &Subtarget) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Subtarget is used, you can omit it.

static SDValue performMULCombine(SDNode *N, SelectionDAG &DAG,
const RISCVSubtarget &Subtarget) {
SDLoc DL(N);
EVT VT = N->getValueType(0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check that the type is a vector so we don't do this to scalar operations. AArch64 doesn't have this because they have a scalar integer multiply accumulate instruction which RISC-V does not have.

Copy link
Contributor

@wangpc-pp wangpc-pp Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MACs for scalar integer exist in XTheadMac and XCVmac but I don't know if it matters here.

vmadd: (mul (add x, 1), y) -> (add (mul x, y), y)
       (mul x, add (y, 1)) -> (add x, (mul x, y))
vnmsub: (mul (sub 1, x), y) -> (sub y, (mul x, y))
        (mul x, (sub 1, y)) -> (sub x, (mul x, y))

Comparison with gcc:
vmadd: https://gcc.godbolt.org/z/xjePx87Y7
vnsub: https://gcc.godbolt.org/z/b17zG7nT1
@ChunyuLiao
Copy link
Member Author

address comments. thanks

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ChunyuLiao ChunyuLiao merged commit 9166cd2 into llvm:main Nov 21, 2023
3 checks passed
@ChunyuLiao ChunyuLiao deleted the vmadd branch November 21, 2023 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants