[RISCV] Move ActiveElementsAffectResult to TSFlags. NFC #101123

lukel97 · 2024-07-30T04:20:17Z

As noted in https://github.com/llvm/llvm-project/pull/100367/files#r1695442138, RISCVMaskedPseudoInfo currently stores two things, whether or not a masked pseudo has an unmasked variant, and whether or not it's element-wise.

These are separate things, so this patch splits the latter out into the underlying instruction's TSFlags to help make the semantics of #100367 more clear.

To the best of my knowledge the only non-element-wise instructions in V are:

vredsum.vs and other reductions
vcompress.vm
vms*f.m
vcpop.m and vfirst.m
viota.m

In vector crypto the instructions that operate on element groups are conservatively marked (this might be fine to relax later given since non-EGS multiple vls are reserved), as well as the SiFive extensions and XTHeadVdot.

As noted in https://github.com/llvm/llvm-project/pull/100367/files#r1695442138, the RISCVMaskedPseudoInfo currently stores two things, whether or not a masked pseudo has an unmasked variant, and whether or not it's element-wise. These are separate things, so this patch splits the latter out into the pseudo's TSFlags to help make the semantics of llvm#100367 more clear. To the best of my knowledge the only non-element-wise instructions in V are: - vredsum.vs and other reductions - vcompress.vm - vms*f.m - vcpop.m and vfirst.m - viota.m In vector crypto the pseudos that operate on element groups are conservatively marked (this might be fine to relax later given since non-EGS multiple vls are reserved), as well as the SiFive extensions.

llvmbot · 2024-07-30T04:20:45Z

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

Author: Luke Lau (lukel97)

Changes

As noted in https://github.com/llvm/llvm-project/pull/100367/files#r1695442138, the RISCVMaskedPseudoInfo currently stores two things, whether or not a masked pseudo has an unmasked variant, and whether or not it's element-wise.

These are separate things, so this patch splits the latter out into the pseudo's TSFlags to help make the semantics of #100367 more clear.

To the best of my knowledge the only non-element-wise instructions in V are:

vredsum.vs and other reductions
vcompress.vm
vms*f.m
vcpop.m and vfirst.m
viota.m

In vector crypto the pseudos that operate on element groups are conservatively marked (this might be fine to relax later given since non-EGS multiple vls are reserved), as well as the SiFive extensions.

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

7 Files Affected:

(modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+9)
(modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+1-1)
(modified) llvm/lib/Target/RISCV/RISCVInstrFormats.td (+3)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfo.h (-1)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td (+12-13)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td (+4)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td (+4)

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 626206962e752..28ee2fc8aa0b1 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -123,6 +123,9 @@ enum {
   // 3 -> widening case
   TargetOverlapConstraintTypeShift = UsesVXRMShift + 1,
   TargetOverlapConstraintTypeMask = 3ULL << TargetOverlapConstraintTypeShift,
+
+  ActiveElementsAffectResultShift = TargetOverlapConstraintTypeShift + 2,
+  ActiveElementsAffectResultMask = 1 << ActiveElementsAffectResultShift,
 };
 
 // Helper functions to read TSFlags.
@@ -171,6 +174,12 @@ static inline bool hasRoundModeOp(uint64_t TSFlags) {
 /// \returns true if this instruction uses vxrm
 static inline bool usesVXRM(uint64_t TSFlags) { return TSFlags & UsesVXRMMask; }
 
+/// \returns true if the result in each element depends on the active elements
+/// e.g. non-elementwise instructions like vredsum.vs/vcompress.vm/viota.m
+static inline bool activeElementsAffectResult(uint64_t TSFlags) {
+  return TSFlags & ActiveElementsAffectResultMask;
+}
+
 static inline unsigned getVLOpNum(const MCInstrDesc &Desc) {
   const uint64_t TSFlags = Desc.TSFlags;
   // This method is only called if we expect to have a VL operand, and all
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 01b2bc08d3ba0..b6f4c4db3722c 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -3845,7 +3845,7 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) {
   // Some operations produce different elementwise results depending on the
   // active elements, like viota.m or vredsum. This transformation is illegal
   // for these if we change the active elements (i.e. mask or VL).
-  if (Info->ActiveElementsAffectResult) {
+  if (RISCVII::activeElementsAffectResult(TrueTSFlags)) {
     if (Mask && !usesAllOnesMask(Mask, Glue))
       return false;
     if (TrueVL != VL)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrFormats.td b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
index a5c8524d05cbc..95f157064d73e 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrFormats.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
@@ -223,6 +223,9 @@ class RVInstCommon<dag outs, dag ins, string opcodestr, string argstr,
   // 3 -> widening case
   bits<2> TargetOverlapConstraintType = 0;
   let TSFlags{22-21} = TargetOverlapConstraintType;
+
+  bit ActiveElementsAffectResult = 0;
+  let TSFlags{23} = ActiveElementsAffectResult;
 }
 
 class RVInst<dag outs, dag ins, string opcodestr, string argstr,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 025e12d81e60d..e9f7cd003685b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -383,7 +383,6 @@ struct RISCVMaskedPseudoInfo {
   uint16_t MaskedPseudo;
   uint16_t UnmaskedPseudo;
   uint8_t MaskOpIdx;
-  uint8_t ActiveElementsAffectResult : 1;
 };
 #define GET_RISCVMaskedPseudosTable_DECL
 #include "RISCVGenSearchableTables.inc"
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index 3f1fd62b1e143..ca55c997b6ceb 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -561,17 +561,16 @@ def RISCVVIntrinsicsTable : GenericTable {
 //    unmasked variant.  For all but compares, both the masked and
 //    unmasked variant have a passthru and policy operand.  For compares,
 //    neither has a policy op, and only the masked version has a passthru.
-class RISCVMaskedPseudo<bits<4> MaskIdx, bit ActiveAffectsRes=false> {
+class RISCVMaskedPseudo<bits<4> MaskIdx> {
   Pseudo MaskedPseudo = !cast<Pseudo>(NAME);
   Pseudo UnmaskedPseudo = !cast<Pseudo>(!subst("_MASK", "", NAME));
   bits<4> MaskOpIdx = MaskIdx;
-  bit ActiveElementsAffectResult = ActiveAffectsRes;
 }
 
 def RISCVMaskedPseudosTable : GenericTable {
   let FilterClass = "RISCVMaskedPseudo";
   let CppTypeName = "RISCVMaskedPseudoInfo";
-  let Fields = ["MaskedPseudo", "UnmaskedPseudo", "MaskOpIdx", "ActiveElementsAffectResult"];
+  let Fields = ["MaskedPseudo", "UnmaskedPseudo", "MaskOpIdx"];
   let PrimaryKey = ["MaskedPseudo"];
   let PrimaryKeyName = "getMaskedPseudoInfo";
 }
@@ -2021,7 +2020,7 @@ multiclass VPseudoVSFS_M {
   defvar constraint = "@earlyclobber $rd";
   foreach mti = AllMasks in {
     defvar mx = mti.LMul.MX;
-    let VLMul = mti.LMul.value in {
+    let VLMul = mti.LMul.value, ActiveElementsAffectResult = true in {
       def "_M_" # mti.BX : VPseudoUnaryNoMaskNoPolicy<VR, VR, constraint>,
                            SchedUnary<"WriteVMSFSV", "ReadVMSFSV", mx,
                                       forceMergeOpRead=true>;
@@ -2060,12 +2059,12 @@ multiclass VPseudoVIOTA_M {
   defvar constraint = "@earlyclobber $rd";
   foreach m = MxList in {
     defvar mx = m.MX;
-    let VLMul = m.value in {
+    let VLMul = m.value, ActiveElementsAffectResult = 1 in {
       def "_" # mx : VPseudoUnaryNoMask<m.vrclass, VR, constraint>,
                        SchedUnary<"WriteVIotaV", "ReadVIotaV", mx,
                                   forceMergeOpRead=true>;
       def "_" # mx # "_MASK" : VPseudoUnaryMask<m.vrclass, VR, constraint>,
-                                 RISCVMaskedPseudo<MaskIdx=2, ActiveAffectsRes=true>,
+                                 RISCVMaskedPseudo<MaskIdx=2>,
                                  SchedUnary<"WriteVIotaV", "ReadVIotaV", mx,
                                             forceMergeOpRead=true>;
     }
@@ -2076,7 +2075,7 @@ multiclass VPseudoVCPR_V {
   foreach m = MxList in {
     defvar mx = m.MX;
     defvar sews = SchedSEWSet<mx>.val;
-    let VLMul = m.value in
+    let VLMul = m.value, ActiveElementsAffectResult = true in
       foreach e = sews in {
         defvar suffix = "_" # m.MX # "_E" # e;
         let SEW = e in
@@ -3158,11 +3157,11 @@ multiclass VPseudoTernaryWithTailPolicy<VReg RetClass,
                                           DAGOperand Op2Class,
                                           LMULInfo MInfo,
                                           int sew> {
-  let VLMul = MInfo.value, SEW=sew in {
+  let VLMul = MInfo.value, SEW=sew, ActiveElementsAffectResult = true in {
     defvar mx = MInfo.MX;
     def "_" # mx # "_E" # sew : VPseudoTernaryNoMaskWithPolicy<RetClass, Op1Class, Op2Class>;
     def "_" # mx # "_E" # sew # "_MASK" : VPseudoTernaryMaskPolicy<RetClass, Op1Class, Op2Class>,
-                                          RISCVMaskedPseudo<MaskIdx=3, ActiveAffectsRes=true>;
+                                          RISCVMaskedPseudo<MaskIdx=3>;
   }
 }
 
@@ -3171,7 +3170,7 @@ multiclass VPseudoTernaryWithTailPolicyRoundingMode<VReg RetClass,
                                                     DAGOperand Op2Class,
                                                     LMULInfo MInfo,
                                                     int sew> {
-  let VLMul = MInfo.value, SEW=sew in {
+  let VLMul = MInfo.value, SEW=sew, ActiveElementsAffectResult = true in {
     defvar mx = MInfo.MX;
     def "_" # mx # "_E" # sew
         : VPseudoTernaryNoMaskWithPolicyRoundingMode<RetClass, Op1Class,
@@ -3179,7 +3178,7 @@ multiclass VPseudoTernaryWithTailPolicyRoundingMode<VReg RetClass,
     def "_" # mx # "_E" # sew # "_MASK"
         : VPseudoTernaryMaskPolicyRoundingMode<RetClass, Op1Class,
                                                Op2Class>,
-          RISCVMaskedPseudo<MaskIdx=3, ActiveAffectsRes=true>;
+          RISCVMaskedPseudo<MaskIdx=3>;
   }
 }
 
@@ -6712,14 +6711,14 @@ defm PseudoVMSET : VPseudoNullaryPseudoM<"VMXNOR">;
 //===----------------------------------------------------------------------===//
 // 15.2. Vector mask population count vcpop
 //===----------------------------------------------------------------------===//
-let IsSignExtendingOpW = 1 in
+let IsSignExtendingOpW = 1, ActiveElementsAffectResult = 1 in
 defm PseudoVCPOP: VPseudoVPOP_M;
 
 //===----------------------------------------------------------------------===//
 // 15.3. vfirst find-first-set mask bit
 //===----------------------------------------------------------------------===//
 
-let IsSignExtendingOpW = 1 in
+let IsSignExtendingOpW = 1, ActiveElementsAffectResult = 1 in
 defm PseudoVFIRST: VPseudoV1ST_M;
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
index 71aa1f19e089a..9d81e03a55079 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
@@ -407,6 +407,8 @@ multiclass VPseudoSiFiveVFNRCLIP<string Constraint = "@earlyclobber $rd"> {
                                                        UsesVXRM=0>;
 }
 
+let ActiveElementsAffectRes = true in {
+
 let Predicates = [HasVendorXSfvcp] in {
   foreach m = MxList in {
     defm X : VPseudoVC_X<m, GPR>;
@@ -458,6 +460,8 @@ let Predicates = [HasVendorXSfvfnrclipxfqf] in {
   defm VFNRCLIP_X_F_QF : VPseudoSiFiveVFNRCLIP;
 }
 
+} // ActiveElementsAffectResult = true
+
 // SDNode
 def SDT_SF_VC_V_X : SDTypeProfile<1, 4, [SDTCisVec<0>,
                                          SDTCisVT<1, XLenVT>,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
index 75fcc1e7cb110..06814aa1a5077 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
@@ -278,6 +278,8 @@ multiclass VPseudoBinaryV_S_NoMask_Zvk<LMULInfo m> {
       def "_VS_" # m.MX # "_" # vs2_lmul.MX : VPseudoBinaryNoMask_Zvk<m.vrclass, vs2_lmul.vrclass>;
 }
 
+let ActiveElementsAffectResult = true in {
+
 multiclass VPseudoVGMUL {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
@@ -397,6 +399,8 @@ multiclass VPseudoVSM3ME {
   }
 }
 
+} // ActiveElementsAffectResult = true
+
 multiclass VPseudoVCLMUL_VV_VX {
   foreach m = MxList in {
     defvar mx = m.MX;

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

wangpc-pp

LGTM.

wangpc-pp · 2024-07-30T04:48:37Z

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

@@ -171,6 +174,12 @@ static inline bool hasRoundModeOp(uint64_t TSFlags) {
 /// \returns true if this instruction uses vxrm
 static inline bool usesVXRM(uint64_t TSFlags) { return TSFlags & UsesVXRMMask; }

+/// \returns true if the result in each element depends on the active elements


in each element may not be true, result of reductions have only one element.
Not a native speaker so I don't know if this is the right expression.

Is the scalar result of vcpop/vfirst considered an "element"?

I guess, I'm finding this tricky to word. Saying "the result depends on the active elements" seems wrong since the vector result always depends on the active elements.

Maybe it's just easier to say "returns true if the result isn't element-wise"

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

topperc · 2024-07-30T05:06:14Z

as well as the SiFive extensions.

Why the SiFive extensions but not XTheadVdot? Or did you audit XTheadVdot?

lukel97 · 2024-07-30T05:12:52Z

Why the SiFive extensions but not XTheadVdot? Or did you audit XTheadVdot?

They slipped past my grep for RISCVVPseudo, I've marked them now

preames · 2024-08-01T23:55:36Z

High level question here. Why store the new TSFlag on the pseudo instead of the underlying instruction? We can go from the former to the later easily for lookup purposes. This seems like a property of the instruction, not of a particular pseudo which wraps it.

lukel97 requested review from frasercrmck, preames, topperc, wangpc-pp and yetingk July 30, 2024 04:20

llvmbot added the backend:RISC-V label Jul 30, 2024

wangpc-pp reviewed Jul 30, 2024

View reviewed changes

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td Outdated Show resolved Hide resolved

wangpc-pp approved these changes Jul 30, 2024

View reviewed changes

wangpc-pp reviewed Jul 30, 2024

View reviewed changes

Fix typo, reword comment

e60b7b5

topperc reviewed Jul 30, 2024

View reviewed changes

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h Outdated Show resolved Hide resolved

Use 1ULL, reword comment

f20e6b4

Mark XTHeadVDot

5f43557

lukel97 mentioned this pull request Jul 31, 2024

[RISCV] Move vmv.v.v peephole from SelectionDAG to RISCVVectorPeephole #100367

Merged

Move from pseudos to base instruction

45d7266

lukel97 merged commit a5b6539 into llvm:main Aug 5, 2024
5 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[RISCV] Move ActiveElementsAffectResult to TSFlags. NFC #101123

[RISCV] Move ActiveElementsAffectResult to TSFlags. NFC #101123

Uh oh!

lukel97 commented Jul 30, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jul 30, 2024

Uh oh!

Uh oh!

wangpc-pp left a comment

Uh oh!

wangpc-pp Jul 30, 2024 •

edited

Loading

Uh oh!

topperc Jul 30, 2024

Uh oh!

lukel97 Jul 30, 2024

Uh oh!

Uh oh!

topperc commented Jul 30, 2024

Uh oh!

lukel97 commented Jul 30, 2024

Uh oh!

preames commented Aug 1, 2024

Uh oh!

Uh oh!

Uh oh!

[RISCV] Move ActiveElementsAffectResult to TSFlags. NFC #101123

[RISCV] Move ActiveElementsAffectResult to TSFlags. NFC #101123

Uh oh!

Conversation

lukel97 commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 30, 2024

Uh oh!

Uh oh!

wangpc-pp left a comment

Choose a reason for hiding this comment

Uh oh!

wangpc-pp Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

topperc Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

lukel97 Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

topperc commented Jul 30, 2024

Uh oh!

lukel97 commented Jul 30, 2024

Uh oh!

preames commented Aug 1, 2024

Uh oh!

Uh oh!

Uh oh!

lukel97 commented Jul 30, 2024 •

edited

Loading

wangpc-pp Jul 30, 2024 •

edited

Loading