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] Use Log2SEW=0 for VMNAND/VMSET created for riscv_vmsge(u) intrinsics. #119767

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Dec 12, 2024

These instructions should always be created with Log2SEW=0 and an LMUL based on SEW=8. This is used by the vsetvli pass to know these instructions only care about the ratio and not the specific value.

Not sure if I can construct a specific test thats hows a vsetvli difference because they were using the SEW and LMUL of the compare instruction that must comes before. Maybe if another instruction got scheduled between you could see a vtype toggle.

Looks like I fixed riscv_vmsge(u)_mask intrinsics years ago, but forgot the unmasked.

I'm working on an enhancement to our MachineVerifier checks that will require VMNAND and VMSET to have Log2SEW=0.

…rinsics.

These instructions should always be created with Log2SEW=0 and an LMUL
based on SEW=8. This is used by the vsetvli pass to know these
instructions only care about the ratio and not the specific value.

Not sure if I can construct a specific test thats hows a vsetvli
difference because they were using the SEW and LMUL of the compare
instruction that must comes before. Maybe if another instruction
got scheduled between you could see a vtype toggle.

Looks like I fixed riscv_vmsge(u)_mask intrinsics years ago, but
forgot the unmasked.
@llvmbot
Copy link
Member

llvmbot commented Dec 12, 2024

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

Author: Craig Topper (topperc)

Changes

These instructions should always be created with Log2SEW=0 and an LMUL based on SEW=8. This is used by the vsetvli pass to know these instructions only care about the ratio and not the specific value.

Not sure if I can construct a specific test thats hows a vsetvli difference because they were using the SEW and LMUL of the compare instruction that must comes before. Maybe if another instruction got scheduled between you could see a vtype toggle.

Looks like I fixed riscv_vmsge(u)_mask intrinsics years ago, but forgot the unmasked.

I'm working on an enhancement to our MachineVerifier checks that will require VMNAND and VMSET to have Log2SEW=0.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+37-19)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vmsgeu.ll (+1-1)
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index c3922e38729dc3..884dfc1b30fe61 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -1664,32 +1664,50 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
       switch (RISCVTargetLowering::getLMUL(Src1VT)) {
       default:
         llvm_unreachable("Unexpected LMUL!");
-#define CASE_VMSLT_VMNAND_VMSET_OPCODES(lmulenum, suffix, suffix_b)            \
+#define CASE_VMSLT_OPCODES(lmulenum, suffix)                                   \
   case RISCVII::VLMUL::lmulenum:                                               \
     VMSLTOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix                 \
                              : RISCV::PseudoVMSLT_VX_##suffix;                 \
     VMSGTOpcode = IsUnsigned ? RISCV::PseudoVMSGTU_VX_##suffix                 \
                              : RISCV::PseudoVMSGT_VX_##suffix;                 \
+    break;
+        CASE_VMSLT_OPCODES(LMUL_F8, MF8)
+        CASE_VMSLT_OPCODES(LMUL_F4, MF4)
+        CASE_VMSLT_OPCODES(LMUL_F2, MF2)
+        CASE_VMSLT_OPCODES(LMUL_1, M1)
+        CASE_VMSLT_OPCODES(LMUL_2, M2)
+        CASE_VMSLT_OPCODES(LMUL_4, M4)
+        CASE_VMSLT_OPCODES(LMUL_8, M8)
+#undef CASE_VMSLT_OPCODES
+      }
+      // Mask operations use the LMUL from the mask type.
+      switch (RISCVTargetLowering::getLMUL(VT)) {
+      default:
+        llvm_unreachable("Unexpected LMUL!");
+#define CASE_VMNAND_VMSET_OPCODES(lmulenum, suffix, suffix_b)                  \
+  case RISCVII::VLMUL::lmulenum:                                               \
     VMNANDOpcode = RISCV::PseudoVMNAND_MM_##suffix;                            \
     VMSetOpcode = RISCV::PseudoVMSET_M_##suffix_b;                             \
     break;
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_F8, MF8, B1)
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_F4, MF4, B2)
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_F2, MF2, B4)
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_1, M1, B8)
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_2, M2, B16)
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_4, M4, B32)
-        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_8, M8, B64)
-#undef CASE_VMSLT_VMNAND_VMSET_OPCODES
+        CASE_VMNAND_VMSET_OPCODES(LMUL_F8, MF8, B1)
+        CASE_VMNAND_VMSET_OPCODES(LMUL_F4, MF4, B2)
+        CASE_VMNAND_VMSET_OPCODES(LMUL_F2, MF2, B4)
+        CASE_VMNAND_VMSET_OPCODES(LMUL_1, M1, B8)
+        CASE_VMNAND_VMSET_OPCODES(LMUL_2, M2, B16)
+        CASE_VMNAND_VMSET_OPCODES(LMUL_4, M4, B32)
+        CASE_VMNAND_VMSET_OPCODES(LMUL_8, M8, B64)
+#undef CASE_VMNAND_VMSET_OPCODES
       }
       SDValue SEW = CurDAG->getTargetConstant(
           Log2_32(Src1VT.getScalarSizeInBits()), DL, XLenVT);
+      SDValue MaskSEW = CurDAG->getTargetConstant(0, DL, XLenVT);
       SDValue VL;
       selectVLOp(Node->getOperand(3), VL);
 
       // If vmsge(u) with minimum value, expand it to vmset.
       if (IsCmpMinimum) {
-        ReplaceNode(Node, CurDAG->getMachineNode(VMSetOpcode, DL, VT, VL, SEW));
+        ReplaceNode(Node,
+                    CurDAG->getMachineNode(VMSetOpcode, DL, VT, VL, MaskSEW));
         return;
       }
 
@@ -1708,7 +1726,7 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
           CurDAG->getMachineNode(VMSLTOpcode, DL, VT, {Src1, Src2, VL, SEW}),
           0);
       ReplaceNode(Node, CurDAG->getMachineNode(VMNANDOpcode, DL, VT,
-                                               {Cmp, Cmp, VL, SEW}));
+                                               {Cmp, Cmp, VL, MaskSEW}));
       return;
     }
     case Intrinsic::riscv_vmsgeu_mask:
@@ -1742,7 +1760,7 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
       switch (RISCVTargetLowering::getLMUL(Src1VT)) {
       default:
         llvm_unreachable("Unexpected LMUL!");
-#define CASE_VMSLT_OPCODES(lmulenum, suffix, suffix_b)                         \
+#define CASE_VMSLT_OPCODES(lmulenum, suffix)                                   \
   case RISCVII::VLMUL::lmulenum:                                               \
     VMSLTOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix                 \
                              : RISCV::PseudoVMSLT_VX_##suffix;                 \
@@ -1751,13 +1769,13 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
     VMSGTMaskOpcode = IsUnsigned ? RISCV::PseudoVMSGTU_VX_##suffix##_MASK      \
                                  : RISCV::PseudoVMSGT_VX_##suffix##_MASK;      \
     break;
-        CASE_VMSLT_OPCODES(LMUL_F8, MF8, B1)
-        CASE_VMSLT_OPCODES(LMUL_F4, MF4, B2)
-        CASE_VMSLT_OPCODES(LMUL_F2, MF2, B4)
-        CASE_VMSLT_OPCODES(LMUL_1, M1, B8)
-        CASE_VMSLT_OPCODES(LMUL_2, M2, B16)
-        CASE_VMSLT_OPCODES(LMUL_4, M4, B32)
-        CASE_VMSLT_OPCODES(LMUL_8, M8, B64)
+        CASE_VMSLT_OPCODES(LMUL_F8, MF8)
+        CASE_VMSLT_OPCODES(LMUL_F4, MF4)
+        CASE_VMSLT_OPCODES(LMUL_F2, MF2)
+        CASE_VMSLT_OPCODES(LMUL_1, M1)
+        CASE_VMSLT_OPCODES(LMUL_2, M2)
+        CASE_VMSLT_OPCODES(LMUL_4, M4)
+        CASE_VMSLT_OPCODES(LMUL_8, M8)
 #undef CASE_VMSLT_OPCODES
       }
       // Mask operations use the LMUL from the mask type.
diff --git a/llvm/test/CodeGen/RISCV/rvv/vmsgeu.ll b/llvm/test/CodeGen/RISCV/rvv/vmsgeu.ll
index e42be4faafefcf..d3f57d58c7ab79 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vmsgeu.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/vmsgeu.ll
@@ -2183,7 +2183,7 @@ entry:
 define <vscale x 4 x i1> @intrinsic_vmsgeu_vi_nxv4i16_i16(<vscale x 4 x i16> %0, iXLen %1) nounwind {
 ; CHECK-LABEL: intrinsic_vmsgeu_vi_nxv4i16_i16:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    vsetvli zero, a0, e16, m1, ta, ma
+; CHECK-NEXT:    vsetvli zero, a0, e8, mf2, ta, ma
 ; CHECK-NEXT:    vmset.m v0
 ; CHECK-NEXT:    ret
 entry:

topperc added a commit to topperc/llvm-project that referenced this pull request Dec 12, 2024
Mask only instructions like vmand and vmsbf should always have 0 for
their Log2SEW operand.  Non-mask instructions should only have
3, 4, 5, or 6 for their Log2SEW operand.

Split the operand type so we can verify these cases separately.

I had to fix the SEW for whole register move to vmv.v.v copy
optimization and update an mir test. The vmv.v.v change isn't
functional since we have already done vsetvli insertion before and
nothing else uses the field after copy expansion. I can split these
changes off if desired.

Stacked on llvm#119767.
@topperc
Copy link
Collaborator Author

topperc commented Dec 13, 2024

I accidentally included the dependent patch when resolved the merge conflict against main.

Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit 3b3394b into llvm:main Dec 13, 2024
8 checks passed
@topperc topperc deleted the pr/log2sew-bug branch December 13, 2024 19:00
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.

3 participants