[RISCV] Add vector hasAndNot to enable optimizations #132438

pfusik · 2025-03-21T18:01:42Z

No description provided.

pfusik

I came across a FIXME comment. Implementing the vectors in a separate function because my understanding is that hasAndNotCompare says it's cheap to check if the result of ANDN is zero - true for scalars, but not RVV AFAIK. X86 does it this way.

pfusik · 2025-03-21T18:02:53Z

llvm/test/CodeGen/RISCV/rvv/vandn-sdnode.ll

-; CHECK-ZVKB-NEXT:    vmerge.vvm v8, v8, v9, v0
+; CHECK-ZVKB-NEXT:    vsra.vi v8, v8, 7
+; CHECK-ZVKB-NEXT:    vnot.v v8, v8
+; CHECK-ZVKB-NEXT:    vand.vv v8, v8, v9


I don't know why this isn't fold into vandn.vv.

The not immediate ended up being 255 instead of -1 like the pattern expected. The upper bits of splat_vector are ignored so either is correct.

This patch fixes it, but I'm still investigating other way to fix it that don't double the isel patterns.

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td index fcbb2dbc76a3..775c161b56e8 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td +++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td @@ -603,13 +603,15 @@ multiclass VPatUnarySDNode_V<SDPatternOperator op, string instruction_name, // This should match the logic in RISCVDAGToDAGISel::selectVSplat def riscv_splat_vector : PatFrag<(ops node:$rs1), (riscv_vmv_v_x_vl undef, node:$rs1, srcvalue)>; -def riscv_vnot : PatFrag<(ops node:$rs1), (xor node:$rs1, - (riscv_splat_vector -1))>; +class riscv_vnot<int sew> + : PatFrags<(ops node:$rs1), + [(xor node:$rs1, (riscv_splat_vector -1)), + (xor node:$rs1, (riscv_splat_vector !srl(-1, !sub(64, sew))))]>; foreach vti = AllIntegerVectors in { let Predicates = !listconcat([HasStdExtZvkb], GetVTypePredicates<vti>.Predicates) in { - def : Pat<(vti.Vector (and (riscv_vnot vti.RegClass:$rs1), + def : Pat<(vti.Vector (and (riscv_vnot<vti.SEW> vti.RegClass:$rs1), vti.RegClass:$rs2)), (!cast<Instruction>("PseudoVANDN_VV_"#vti.LMul.MX) (vti.Vector (IMPLICIT_DEF)),

Here's another variation using ImmLeaf

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td index fcbb2dbc76a3..070a7f71edb5 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td +++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td @@ -603,13 +603,17 @@ multiclass VPatUnarySDNode_V<SDPatternOperator op, string instruction_name, // This should match the logic in RISCVDAGToDAGISel::selectVSplat def riscv_splat_vector : PatFrag<(ops node:$rs1), (riscv_vmv_v_x_vl undef, node:$rs1, srcvalue)>; -def riscv_vnot : PatFrag<(ops node:$rs1), (xor node:$rs1, - (riscv_splat_vector -1))>; + +def allonessew8 : ImmLeaf<XLenVT, "return SignExtend64<8>(Imm) == -1LL;">; +def allonessew16 : ImmLeaf<XLenVT, "return SignExtend64<16>(Imm) == -1LL;">; +def allonessew32 : ImmLeaf<XLenVT, "return SignExtend64<32>(Imm) == -1LL;">; +def allonessew64 : ImmLeaf<XLenVT, "return Imm == -1LL;">; foreach vti = AllIntegerVectors in { let Predicates = !listconcat([HasStdExtZvkb], GetVTypePredicates<vti>.Predicates) in { - def : Pat<(vti.Vector (and (riscv_vnot vti.RegClass:$rs1), + def : Pat<(vti.Vector (and (xor vti.RegClass:$rs1, + (riscv_splat_vector !cast<ImmLeaf>("allonessew"#vti.SEW))), vti.RegClass:$rs2)), (!cast<Instruction>("PseudoVANDN_VV_"#vti.LMul.MX) (vti.Vector (IMPLICIT_DEF)),

@topperc Thanks. I added your latter change. It affects no other test. Or do you prefer it as a separate PR or to work on it yourself?
I do not understand why VNOT is matched with a simpler pattern, i.e. at which point the constants are stripped to SEW.

@topperc Thanks. I added your latter change. It affects no other test. Or do you prefer it as a separate PR or to work on it yourself?

It can be part of this PR. I don't have test case without this PR.

I think type legalization sign extends immediates that are multiples of a byte. But this immediate was created at a different type and didn't get sign extended.

llvmbot · 2025-03-24T15:06:42Z

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

Author: Piotr Fusik (pfusik)

Changes

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

4 Files Affected:

(modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+9-1)
(modified) llvm/lib/Target/RISCV/RISCVISelLowering.h (+1)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td (+6-3)
(modified) llvm/test/CodeGen/RISCV/rvv/vandn-sdnode.ll (+20)

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 132faf5b85c1a..97ec9123448a5 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -2051,7 +2051,6 @@ bool RISCVTargetLowering::isMaskAndCmp0FoldingBeneficial(
 bool RISCVTargetLowering::hasAndNotCompare(SDValue Y) const {
   EVT VT = Y.getValueType();
 
-  // FIXME: Support vectors once we have tests.
   if (VT.isVector())
     return false;
 
@@ -2059,6 +2058,15 @@ bool RISCVTargetLowering::hasAndNotCompare(SDValue Y) const {
          (!isa<ConstantSDNode>(Y) || cast<ConstantSDNode>(Y)->isOpaque());
 }
 
+bool RISCVTargetLowering::hasAndNot(SDValue Y) const {
+  EVT VT = Y.getValueType();
+
+  if (!VT.isVector())
+    return hasAndNotCompare(Y);
+
+  return Subtarget.hasStdExtZvkb();
+}
+
 bool RISCVTargetLowering::hasBitTest(SDValue X, SDValue Y) const {
   // Zbs provides BEXT[_I], which can be used with SEQZ/SNEZ as a bit test.
   if (Subtarget.hasStdExtZbs())
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.h b/llvm/lib/Target/RISCV/RISCVISelLowering.h
index ffbc14a29006c..fb3931a561757 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.h
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.h
@@ -535,6 +535,7 @@ class RISCVTargetLowering : public TargetLowering {
   bool isCheapToSpeculateCtlz(Type *Ty) const override;
   bool isMaskAndCmp0FoldingBeneficial(const Instruction &AndI) const override;
   bool hasAndNotCompare(SDValue Y) const override;
+  bool hasAndNot(SDValue Y) const override;
   bool hasBitTest(SDValue X, SDValue Y) const override;
   bool shouldProduceAndByConstByHoistingConstFromShiftsLHSOfAnd(
       SDValue X, ConstantSDNode *XC, ConstantSDNode *CC, SDValue Y,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
index fcbb2dbc76a37..904ce1b616c2d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
@@ -603,13 +603,16 @@ multiclass VPatUnarySDNode_V<SDPatternOperator op, string instruction_name,
 // This should match the logic in RISCVDAGToDAGISel::selectVSplat
 def riscv_splat_vector : PatFrag<(ops node:$rs1),
                                  (riscv_vmv_v_x_vl undef, node:$rs1, srcvalue)>;
-def riscv_vnot : PatFrag<(ops node:$rs1), (xor node:$rs1,
-                                               (riscv_splat_vector -1))>;
+def allonessew8  : ImmLeaf<XLenVT, "return SignExtend64<8>(Imm) == -1LL;">;
+def allonessew16 : ImmLeaf<XLenVT, "return SignExtend64<16>(Imm) == -1LL;">;
+def allonessew32 : ImmLeaf<XLenVT, "return SignExtend64<32>(Imm) == -1LL;">;
+def allonessew64 : ImmLeaf<XLenVT, "return Imm == -1LL;">;
 
 foreach vti = AllIntegerVectors in {
   let Predicates = !listconcat([HasStdExtZvkb],
                                GetVTypePredicates<vti>.Predicates) in {
-    def : Pat<(vti.Vector (and (riscv_vnot vti.RegClass:$rs1),
+    def : Pat<(vti.Vector (and (xor vti.RegClass:$rs1,
+                                    (riscv_splat_vector !cast<ImmLeaf>("allonessew"#vti.SEW))),
                                vti.RegClass:$rs2)),
               (!cast<Instruction>("PseudoVANDN_VV_"#vti.LMul.MX)
                  (vti.Vector (IMPLICIT_DEF)),
diff --git a/llvm/test/CodeGen/RISCV/rvv/vandn-sdnode.ll b/llvm/test/CodeGen/RISCV/rvv/vandn-sdnode.ll
index a4c64d6fa5ef5..aef46e1f5cf1b 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vandn-sdnode.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/vandn-sdnode.ll
@@ -2567,3 +2567,23 @@ for.body:
   %exitcond.not = icmp eq i64 %indvars.iv.next, 256
   br i1 %exitcond.not, label %for.cond.cleanup, label %for.body
 }
+
+define <vscale x 1 x i8> @not_signbit_mask_nxv1i8(<vscale x 1 x i8> %a, <vscale x 1 x i8> %b) {
+; CHECK-LABEL: not_signbit_mask_nxv1i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetvli a0, zero, e8, mf8, ta, ma
+; CHECK-NEXT:    vmsgt.vi v0, v8, -1
+; CHECK-NEXT:    vmv.v.i v8, 0
+; CHECK-NEXT:    vmerge.vvm v8, v8, v9, v0
+; CHECK-NEXT:    ret
+;
+; CHECK-ZVKB-LABEL: not_signbit_mask_nxv1i8:
+; CHECK-ZVKB:       # %bb.0:
+; CHECK-ZVKB-NEXT:    vsetvli a0, zero, e8, mf8, ta, ma
+; CHECK-ZVKB-NEXT:    vsra.vi v8, v8, 7
+; CHECK-ZVKB-NEXT:    vandn.vv v8, v9, v8
+; CHECK-ZVKB-NEXT:    ret
+  %cond = icmp sgt <vscale x 1 x i8> %a, splat (i8 -1)
+  %r = select <vscale x 1 x i1> %cond, <vscale x 1 x i8> %b, <vscale x 1 x i8> zeroinitializer
+  ret <vscale x 1 x i8> %r
+}

topperc

LGTM

pfusik · 2025-03-24T16:58:44Z

Test merged into main branch as 9b8bcd2. PR rebased.

preames

Non blocking for this patch, but make sure you check the codegen for the fixed vector case too. We sometimes hit different lowering paths for fixed and scalable and I only see a scalable test changing here.

pfusik · 2025-03-24T18:42:59Z

Non blocking for this patch, but make sure you check the codegen for the fixed vector case too. We sometimes hit different lowering paths for fixed and scalable and I only see a scalable test changing here.

Thanks for pointing that out! Indeed there's a problem with fixed vectors here:

+define <8 x i8> @not_signbit_mask_v8i8(<8 x i8> %a, <8 x i8> %b) {
+; CHECK-LABEL: not_signbit_mask_v8i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
+; CHECK-NEXT:    vmsgt.vi v0, v8, -1
+; CHECK-NEXT:    vmv.v.i v8, 0
+; CHECK-NEXT:    vmerge.vvm v8, v8, v9, v0
+; CHECK-NEXT:    ret
+;
+; CHECK-ZVKB-LABEL: not_signbit_mask_v8i8:
+; CHECK-ZVKB:       # %bb.0:
+; CHECK-ZVKB-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
+; CHECK-ZVKB-NEXT:    vsra.vi v8, v8, 7
+; CHECK-ZVKB-NEXT:    vnot.v v8, v8
+; CHECK-ZVKB-NEXT:    vand.vv v8, v8, v9
+; CHECK-ZVKB-NEXT:    ret
+  %cond = icmp sgt <8 x i8> %a, splat (i8 -1)
+  %r = select <8 x i1> %cond, <8 x i8> %b, <8 x i8> zeroinitializer
+  ret <8 x i8> %r
+}

I'll debug it tomorrow.

I don't see any fixed vector tests for vandn. Is vandn-sdnode.ll the right file to add these in?

topperc · 2025-03-24T19:01:34Z

Non blocking for this patch, but make sure you check the codegen for the fixed vector case too. We sometimes hit different lowering paths for fixed and scalable and I only see a scalable test changing here.

Thanks for pointing that out! Indeed there's a problem with fixed vectors here:
+define <8 x i8> @not_signbit_mask_v8i8(<8 x i8> %a, <8 x i8> %b) {
+; CHECK-LABEL: not_signbit_mask_v8i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
+; CHECK-NEXT:    vmsgt.vi v0, v8, -1
+; CHECK-NEXT:    vmv.v.i v8, 0
+; CHECK-NEXT:    vmerge.vvm v8, v8, v9, v0
+; CHECK-NEXT:    ret
+;
+; CHECK-ZVKB-LABEL: not_signbit_mask_v8i8:
+; CHECK-ZVKB:       # %bb.0:
+; CHECK-ZVKB-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
+; CHECK-ZVKB-NEXT:    vsra.vi v8, v8, 7
+; CHECK-ZVKB-NEXT:    vnot.v v8, v8
+; CHECK-ZVKB-NEXT:    vand.vv v8, v8, v9
+; CHECK-ZVKB-NEXT:    ret
+  %cond = icmp sgt <8 x i8> %a, splat (i8 -1)
+  %r = select <8 x i1> %cond, <8 x i8> %b, <8 x i8> zeroinitializer
+  ret <8 x i8> %r
+}
I'll debug it tomorrow.

I don't see any fixed vector tests for vandn. Is vandn-sdnode.ll the right file to add these in?

We usually put fixed vectors in tests starting with fixed-vectors-. I think and/or/xor are in ../llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll. Since vandn requires a different extension we can create fixed-vectors-vandn.ll

pfusik · 2025-03-25T16:32:46Z

Non blocking for this patch, but make sure you check the codegen for the fixed vector case too. We sometimes hit different lowering paths for fixed and scalable and I only see a scalable test changing here.

Thanks for pointing that out! Indeed there's a problem with fixed vectors here

Fixed by updating the VL pattern. Please review.

topperc

LGTM

pfusik · 2025-03-26T06:44:13Z

Fixed vectors tests merged into main branch as 529feec. PR rebased.

pfusik commented Mar 21, 2025

View reviewed changes

pfusik marked this pull request as ready for review March 24, 2025 15:05

llvmbot added the backend:RISC-V label Mar 24, 2025

topperc approved these changes Mar 24, 2025

View reviewed changes

pfusik force-pushed the has-vandn branch from dec3bf2 to 12376ba Compare March 24, 2025 16:57

preames reviewed Mar 24, 2025

View reviewed changes

topperc approved these changes Mar 25, 2025

View reviewed changes

pfusik and others added 3 commits March 26, 2025 07:42

[RISCV] Add vector hasAndNot to enable optimizations

b52efb4

[RISCV] Match VANDN ignoring XOR constant bits above SEW

a25e006

[RISCV] VL pattern for VANDN ignoring XOR constant bits above SEW

7eacdcf

pfusik force-pushed the has-vandn branch from 5be683a to 7eacdcf Compare March 26, 2025 06:43

pfusik merged commit 96925fa into llvm:main Mar 26, 2025
11 checks passed

[RISCV] Add vector hasAndNot to enable optimizations #132438

[RISCV] Add vector hasAndNot to enable optimizations #132438

Uh oh!

Conversation

pfusik commented Mar 21, 2025

Uh oh!

pfusik left a comment

Choose a reason for hiding this comment

Uh oh!

pfusik Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

topperc Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

topperc Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

pfusik Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

topperc Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

llvmbot commented Mar 24, 2025

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

pfusik commented Mar 24, 2025

Uh oh!

preames left a comment

Choose a reason for hiding this comment

Uh oh!

pfusik commented Mar 24, 2025

Uh oh!

topperc commented Mar 24, 2025

Uh oh!

pfusik commented Mar 25, 2025

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

pfusik commented Mar 26, 2025

Uh oh!

Uh oh!

Uh oh!