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] Fix and refactor Zvk sched classes #86519

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

michaelmaitland
Copy link
Contributor

@michaelmaitland michaelmaitland commented Mar 25, 2024

* VPseudoVALU_V_NoMask_Zvk, VPseudoVALU_S_NoMask_Zvk, VPseudoVALU_VV_NoMask_Zvk,
  and VPseudoVALU_VI_NoMask_Zvk do not read a merge op
* VPseudoUnaryV_V is a unary read instead of a binary read
* Convert all other cases `Sched<[...]>` to the equivalent SchedUnary,
  SchedBinary, or SchedTernary.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2024

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

Author: Michael Maitland (michaelmaitland)

Changes
  • Add scheduling information for VPseudoVWALU_VI.
  • VPseudoVALU_V_NoMask_Zvk defines VPseudoUnaryV_V_NoMask_Zvk and read two vector operands. It should only read one vector operand. The read of the merge operand is already noted by ReadVMask (or forceReadMergeOp=true).
  • VPseudoVALU_S_NoMask_Zvk defines VPseudoUnaryV_S_NoMask_Zvk and read two vector operands. It should only read one vector operand. The read of the merge operand is already noted by ReadVMask (or forceReadMergeOp=true).
  • VPseudoUnaryV_V defines VPseudoUnaryV_V and read two vector operands. It should only read one vector operand. The read of the merge operand is already noted by ReadVMask (or forceReadMergeOp=true).
  • Convert all other cases Sched&lt;[...]&gt; to the equivalent SchedUnary or SchedBinary.

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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td (+3-1)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td (+18-35)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index 8be4c7741ca12b..4be5ee22f61a84 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -2998,7 +2998,9 @@ multiclass VPseudoVWALU_VV_VX {
 
 multiclass VPseudoVWALU_VV_VX_VI<Operand ImmType> : VPseudoVWALU_VV_VX {
   foreach m = MxListW in {
-    defm "" : VPseudoBinaryW_VI<ImmType, m>;
+    defm "" : VPseudoBinaryW_VI<ImmType, m>,
+              SchedUnary<"WriteVIWALUV", "ReadVIWALUV", m.MX,
+                         forceMergeOpRead=true>;
   }
 }
 
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
index b4bd074b710179..efd452ec2654e4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
@@ -237,22 +237,18 @@ multiclass VPseudoUnaryV_S_NoMask_Zvk<LMULInfo m, string Constraint = ""> {
 multiclass VPseudoVALU_V_NoMask_Zvk<string Constraint = ""> {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm "" : VPseudoUnaryV_V_NoMask_Zvk<m, Constraint>,
-              Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+              SchedUnary<"WriteVIALUV", "ReadVIALUV", mx,
+                         forceMergeOpRead=true>;
   }
 }
 
 multiclass VPseudoVALU_S_NoMask_Zvk<string Constraint = ""> {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm "" : VPseudoUnaryV_S_NoMask_Zvk<m, Constraint>,
-              Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+              SchedUnary<"WriteVIALUV", "ReadVIALUV", mx,
+                         forceMergeOpRead=true>;
   }
 }
 
@@ -264,63 +260,52 @@ multiclass VPseudoVALU_V_S_NoMask_Zvk<string Constraint = ""> {
 multiclass VPseudoVALU_VV_NoMask_Zvk<string Constraint = ""> {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm _VV : VPseudoBinaryNoMask_Zvk<m.vrclass, m.vrclass, m.vrclass, m,
                                        Constraint>,
-               Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+               SchedBinary<"WriteVIALUV", "ReadVIALUV", "ReadVIALUV", mx,
+                           forceMergeOpRead=true>;
   }
 }
 
 multiclass VPseudoVALU_VI_NoMask_Zvk<Operand ImmType = simm5, string Constraint = ""> {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm _VI : VPseudoBinaryNoMask_Zvk<m.vrclass, m.vrclass, ImmType, m,
                                        Constraint>,
-               Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+               SchedBinary<"WriteVIALUV", "ReadVIALUV", "ReadVIALUV", mx,
+                           forceMergeOpRead=true>;
   }
 }
 
 multiclass VPseudoVALU_VI_NoMaskTU_Zvk<Operand ImmType = uimm5, string Constraint = ""> {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm _VI : VPseudoBinaryNoMask<m.vrclass, m.vrclass, ImmType, m,
                                    Constraint>,
-               Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+               SchedBinary<"WriteVIALUV", "ReadVIALUV", "ReadVIALUV", mx,
+                           forceMergeOpRead=true>;
   }
 }
 
 multiclass VPseudoVALU_VV_NoMaskTU_Zvk<string Constraint = ""> {
   foreach m = MxListVF4 in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm _VV : VPseudoBinaryNoMask<m.vrclass, m.vrclass, m.vrclass, m,
                                    Constraint>,
-               Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+               SchedBinary<"WriteVIALUV", "ReadVIALUV", "ReadVIALUV", mx,
+                           forceMergeOpRead=true>;
   }
 }
 
 multiclass VPseudoVCLMUL_VV_VX {
   foreach m = MxList in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar WriteVIALUX_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-    defvar ReadVIALUX_MX = !cast<SchedRead>("ReadVIALUX_" # mx);
-
     defm "" : VPseudoBinaryV_VV<m>,
-              Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+              SchedBinary<"WriteVIALUV", "ReadVIALUV", "ReadVIALUV", mx,
+                          forceMergeOpRead=true>;
     defm "" : VPseudoBinaryV_VX<m>,
-              Sched<[WriteVIALUX_MX, ReadVIALUV_MX, ReadVIALUX_MX, ReadVMask]>;
+              SchedBinary<"WriteVIALUX", "ReadVIALUV", "ReadVIALUX", mx,
+                          forceMergeOpRead=true>;
   }
 }
 
@@ -336,11 +321,9 @@ multiclass VPseudoUnaryV_V<LMULInfo m> {
 multiclass VPseudoVALU_V {
   foreach m = MxList in {
     defvar mx = m.MX;
-    defvar WriteVIALUV_MX = !cast<SchedWrite>("WriteVIALUV_" # mx);
-    defvar ReadVIALUV_MX = !cast<SchedRead>("ReadVIALUV_" # mx);
-
     defm "" : VPseudoUnaryV_V<m>,
-              Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
+              SchedUnary<"WriteVIALUV", "ReadVIALUV", mx,
+                         forceMergeOpRead=true>;
   }
 }
 

Copy link

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

Copy link

✅ With the latest revision this PR passed the Python code formatter.

llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td Outdated Show resolved Hide resolved
llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td Outdated Show resolved Hide resolved
llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td Outdated Show resolved Hide resolved
llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td Outdated Show resolved Hide resolved
llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td Show resolved Hide resolved
llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td Show resolved Hide resolved
* VPseudoVALU_V_NoMask_Zvk, VPseudoVALU_S_NoMask_Zvk, VPseudoVALU_VV_NoMask_Zvk,
  VPseudoVALU_VI_NoMask_Zvk, VPseudoVALU_VI_NoMaskTU_Zvk, and
  VPseudoVALU_VV_NoMaskTU_Zvk read a merge op
* VPseudoUnaryV_V is a unary read instead of a binary read
* Convert all other cases `Sched<[...]>` to the equivalent SchedUnary,
  SchedBinary, or SchedTernary.
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM.
I think all the comments have been addressed? Maybe you need to reword the description.

@michaelmaitland michaelmaitland merged commit 15abe09 into llvm:main Apr 2, 2024
4 checks passed
defm "" : VPseudoBinaryV_VV<m>,
Sched<[WriteVIALUV_MX, ReadVIALUV_MX, ReadVIALUV_MX, ReadVMask]>;
SchedBinary<"WriteVIALUV", "ReadVIALUV", "ReadVIALUV", mx,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a multiply so shouldn't it use the multiply scheduler class? Really it should have its own class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will address this in a follow up patch, where I plan to make sure that sched classes match with behavior. This patch was about the right number of classes.

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