[RISCV] Avoid VMNOT by swapping VMERGE operands for mask extensions #126751

pfusik · 2025-02-11T15:51:51Z

Fold:

(select (not m),  1, 0) -> (select m, 0,  1)
(select (not m), -1, 0) -> (select m, 0, -1)

pfusik · 2025-02-11T15:53:47Z

In llvm-codegen-benchmark I noticed hundreds of patterns such as https://github.com/dtcxzyw/llvm-codegen-benchmark/blob/12a783bf6af60c93d100354b71dc6c0c4dea4eb7/result/rvv/0002ccdf5e758f9a.S
This crude proof-of-concept turns it into:

    vsetivli        zero, 8, e32, m2, ta, ma
    vmv.v.i v10, 1
    vmerge.vim      v10, v10, 0, v0
    vand.vi v8, v8, 3
    vor.vv  v8, v8, v10
    vmseq.vi        v0, v8, 0
    ret

Questions:

Do we want this optimization?
Do we want it at this stage or maybe there's a better place?

Further work:
I think in many scenarios we could emit a mask-undisturbed operation (here: vor.vi) instead of vmv+vmerge+vector op.

preames · 2025-02-11T18:24:35Z

In concept, sound reasonable. I had played with a more generic vmerge operand swap a while back, and got lost in unprofitable cases. Something restricted specifically to the extend doesn't seem unreasonable. Would need a real patch and tests to meaningful review beyond that.

llvmbot · 2025-02-17T11:41:28Z

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

Author: Piotr Fusik (pfusik)

Changes

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

2 Files Affected:

(modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+17)
(added) llvm/test/CodeGen/RISCV/rvv/mask-exts-not.ll (+52)

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index c40ab0d09bdf6..11f532c25d311 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -8965,6 +8965,9 @@ SDValue RISCVTargetLowering::lowerVectorMaskExt(SDValue Op, SelectionDAG &DAG,
   if (VecVT.isScalableVector()) {
     SDValue SplatZero = DAG.getConstant(0, DL, VecVT);
     SDValue SplatTrueVal = DAG.getSignedConstant(ExtTrueVal, DL, VecVT);
+    if (Src.getOpcode() == ISD::XOR &&
+        ISD::isConstantSplatVectorAllOnes(Src.getOperand(1).getNode(), false))
+      return DAG.getNode(ISD::VSELECT, DL, VecVT, Src.getOperand(0), SplatZero, SplatTrueVal);
     return DAG.getNode(ISD::VSELECT, DL, VecVT, Src, SplatTrueVal, SplatZero);
   }
 
@@ -8980,6 +8983,20 @@ SDValue RISCVTargetLowering::lowerVectorMaskExt(SDValue Op, SelectionDAG &DAG,
   SDValue SplatZero = DAG.getConstant(0, DL, XLenVT);
   SDValue SplatTrueVal = DAG.getSignedConstant(ExtTrueVal, DL, XLenVT);
 
+  if (Src.getOpcode() == ISD::EXTRACT_SUBVECTOR) {
+    SDValue Xor = Src.getOperand(0);
+    if (Xor.getOpcode() == RISCVISD::VMXOR_VL) {
+      SDValue ScalableOnes = Xor.getOperand(1);
+      if (ScalableOnes.getOpcode() == ISD::INSERT_SUBVECTOR &&
+          ScalableOnes.getOperand(0).isUndef() &&
+          ISD::isConstantSplatVectorAllOnes(
+              ScalableOnes.getOperand(1).getNode(), false)) {
+        CC = Xor.getOperand(0);
+        std::swap(SplatZero, SplatTrueVal);
+      }
+    }
+  }
+
   SplatZero = DAG.getNode(RISCVISD::VMV_V_X_VL, DL, ContainerVT,
                           DAG.getUNDEF(ContainerVT), SplatZero, VL);
   SplatTrueVal = DAG.getNode(RISCVISD::VMV_V_X_VL, DL, ContainerVT,
diff --git a/llvm/test/CodeGen/RISCV/rvv/mask-exts-not.ll b/llvm/test/CodeGen/RISCV/rvv/mask-exts-not.ll
new file mode 100644
index 0000000000000..c72d717c033ea
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/rvv/mask-exts-not.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+v -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+v -verify-machineinstrs < %s | FileCheck %s
+
+define <vscale x 8 x i8> @mask_sext_not_nxv8i8(<vscale x 8 x i1> %m) {
+; CHECK-LABEL: mask_sext_not_nxv8i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetvli a0, zero, e8, m1, ta, ma
+; CHECK-NEXT:    vmv.v.i v8, -1
+; CHECK-NEXT:    vmerge.vim v8, v8, 0, v0
+; CHECK-NEXT:    ret
+  %n = xor <vscale x 8 x i1> %m, splat (i1 true)
+  %ext = sext <vscale x 8 x i1> %n to <vscale x 8 x i8>
+  ret <vscale x 8 x i8> %ext
+}
+
+define <vscale x 8 x i8> @mask_zext_not_nxv8i8(<vscale x 8 x i1> %m) {
+; CHECK-LABEL: mask_zext_not_nxv8i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetvli a0, zero, e8, m1, ta, ma
+; CHECK-NEXT:    vmv.v.i v8, 1
+; CHECK-NEXT:    vmerge.vim v8, v8, 0, v0
+; CHECK-NEXT:    ret
+  %n = xor <vscale x 8 x i1> %m, splat (i1 true)
+  %ext = zext <vscale x 8 x i1> %n to <vscale x 8 x i8>
+  ret <vscale x 8 x i8> %ext
+}
+
+define <8 x i8> @mask_sext_not_v8i8(<8 x i1> %m) {
+; CHECK-LABEL: mask_sext_not_v8i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
+; CHECK-NEXT:    vmv.v.i v8, -1
+; CHECK-NEXT:    vmerge.vim v8, v8, 0, v0
+; CHECK-NEXT:    ret
+  %n = xor <8 x i1> %m, splat (i1 true)
+  %ext = sext <8 x i1> %n to <8 x i8>
+  ret <8 x i8> %ext
+}
+
+define <8 x i8> @mask_zext_not_v8i8(<8 x i1> %m) {
+; CHECK-LABEL: mask_zext_not_v8i8:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
+; CHECK-NEXT:    vmv.v.i v8, 1
+; CHECK-NEXT:    vmerge.vim v8, v8, 0, v0
+; CHECK-NEXT:    ret
+  %n = xor <8 x i1> %m, splat (i1 true)
+  %ext = zext <8 x i1> %n to <8 x i8>
+  ret <8 x i8> %ext
+}
+

github-actions · 2025-02-17T11:43:55Z

✅ With the latest revision this PR passed the C/C++ code formatter.

preames

LGTM

Please make sure to update the review description to be non-blank before landing.

Do you have commit access? Or do you need me to land for you?

lukel97

LGTM

lukel97 · 2025-02-20T03:55:36Z

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

    SDValue SplatZero = DAG.getConstant(0, DL, VecVT);
    SDValue SplatTrueVal = DAG.getSignedConstant(ExtTrueVal, DL, VecVT);
+    if (Src.getOpcode() == ISD::XOR &&
+        ISD::isConstantSplatVectorAllOnes(Src.getOperand(1).getNode(), false))


The BuildVectorOnly arg defaults to false so you can omit if you'd like

Default arg omitted, thanks!

lukel97 · 2025-02-20T03:55:53Z

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

+      if (ScalableOnes.getOpcode() == ISD::INSERT_SUBVECTOR &&
+          ScalableOnes.getOperand(0).isUndef() &&
+          ISD::isConstantSplatVectorAllOnes(
+              ScalableOnes.getOperand(1).getNode(), false)) {


pfusik · 2025-02-20T07:23:41Z

LGTM

Please make sure to update the review description to be non-blank before landing.

Done.
Also added negative tests for XOR that is not a NOT. Force-pushed because of editing the first commit with the tests.

Do you have commit access? Or do you need me to land for you?

I do have commit access. Since the GitHub interface is restricted to squash merges, I'll cherry-pick the commit adding the tests to the main branch. I've done that before.

Fold: (select (not m), 1, 0) -> (select m, 0, 1) (select (not m), -1, 0) -> (select m, 0, -1)

pfusik · 2025-02-20T10:41:09Z

Tests merged into main branch as 9787240. PR rebased.

pfusik force-pushed the vmnot branch from bee9440 to ca3ba6f Compare February 17, 2025 11:40

pfusik marked this pull request as ready for review February 17, 2025 11:40

llvmbot added the backend:RISC-V label Feb 17, 2025

pfusik requested review from preames and topperc February 17, 2025 20:38

preames approved these changes Feb 19, 2025

View reviewed changes

lukel97 approved these changes Feb 20, 2025

View reviewed changes

pfusik changed the title ~~[RISCV] Avoid VMNOT by swapping VMERGE operands~~ [RISCV] Avoid VMNOT by swapping VMERGE operands for mask extensions Feb 20, 2025

pfusik force-pushed the vmnot branch from e3cbf3d to e8f700e Compare February 20, 2025 07:19

[RISCV] Avoid VMNOT by swapping VMERGE operands for mask extensions

55ad244

Fold: (select (not m), 1, 0) -> (select m, 0, 1) (select (not m), -1, 0) -> (select m, 0, -1)

pfusik force-pushed the vmnot branch from e8f700e to 55ad244 Compare February 20, 2025 10:40

pfusik merged commit 0a8341f into llvm:main Feb 20, 2025
8 checks passed

[RISCV] Avoid VMNOT by swapping VMERGE operands for mask extensions #126751

[RISCV] Avoid VMNOT by swapping VMERGE operands for mask extensions #126751

Uh oh!

Conversation

pfusik commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfusik commented Feb 11, 2025

Uh oh!

preames commented Feb 11, 2025

Uh oh!

llvmbot commented Feb 17, 2025

Uh oh!

github-actions bot commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

preames left a comment

Choose a reason for hiding this comment

Uh oh!

lukel97 left a comment

Choose a reason for hiding this comment

Uh oh!

lukel97 Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

pfusik Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

lukel97 Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

pfusik Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

pfusik commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfusik commented Feb 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pfusik commented Feb 11, 2025 •

edited

Loading

github-actions bot commented Feb 17, 2025 •

edited

Loading

pfusik commented Feb 20, 2025 •

edited

Loading