[PATCH] D98002: [RISCV] Add scheduling resources for V

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a comment.

I've given it a quick once-over. Apologies if I've missed the mark in some of my comments; there might be reasons for some of them that I'm unaware of.

In general, I'm not seeing resources for mask reads? Should those be modeled?



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:347
+  def X  : VALUVX<funct6, OPIVX, opcodestr # "." # vw # "x">,
+           Sched<[WriteVIALUV, ReadVIALUV, ReadVIALUV]>;
+  def I  : VALUVI<funct6, opcodestr # "." # vw # "i", optype>,
----------------
Should this use `ReadVIALUX`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:415
+  def XM : VALUmVX<funct6, OPIVX, opcodestr # ".vxm">,
+           Sched<[WriteVIMergeX, ReadVIMergeV]>;
+  def IM : VALUmVI<funct6, opcodestr # ".vim">,
----------------
Is this missing a read of an `X` resource?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:478
+  def F : VALUVF<funct6, OPFVF, opcodestr # "." # vw # "f">,
+          Sched<[WriteVFDivV, ReadVFDivV, ReadVFDivF]>;
+}
----------------
Should this be `WriteVFDivF`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:626
+  def V  : VALUVV<funct6, OPIVV, opcodestr # "." # vw # "v">,
+           Sched<[WriteVNShiftV, ReadVShiftV, ReadVNShiftV]>;
+  def X  : VALUVX<funct6, OPIVX, opcodestr # "." # vw # "x">,
----------------
`ReadVNShiftV` instead of `ReadVShiftV`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:628
+  def X  : VALUVX<funct6, OPIVX, opcodestr # "." # vw # "x">,
+           Sched<[WriteVNShiftX, ReadVShiftV, ReadVNShiftX]>;
+  def I  : VALUVI<funct6, opcodestr # "." # vw # "i", optype>,
----------------
`ReadVNShiftV` instead of `ReadVShiftV`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:644
+  def X  : VALUVX<funct6, OPIVX, opcodestr # "." # vw # "x">,
+           Sched<[WriteVICmpV, ReadVICmpV, ReadVICmpV]>;
+  def I  : VALUVI<funct6, opcodestr # "." # vw # "i", optype>,
----------------
Missing `ReadVICmpX`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1045
+                       (ins VR:$vs1), "vmv.v.v", "$vd, $vs1">,
+              Sched<[WriteVIMovV, ReadVIMovV, ReadVIMovV]>;
 // op vd, rs1
----------------
Feels like there's too many read resources: VMV_V_V only has one operand?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1049
+                       (ins GPR:$rs1), "vmv.v.x", "$vd, $rs1">,
+              Sched<[WriteVIMovX, ReadVIMovV, ReadVIMovX]>;
 // op vd, imm
----------------
Again, too many read resources? VMV_V_X only has one operand.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1053
+                       (ins simm5:$imm), "vmv.v.i", "$vd, $imm">,
+              Sched<[WriteVIMovI, ReadVIMovV]>;
 } // hasSideEffects = 0, mayLoad = 0, mayStore = 0
----------------
Too many read resources?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1174
+                           "vfmerge.vfm", "$vd, $vs2, $rs1, v0">,
+                  Sched<[WriteVFMergeV, ReadVFMergeV, ReadVFMergeV]>;
 
----------------
Unsure here, but since this reads a scalar should it read something like `ReadVFMergeF`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1181
+                       (ins FPR32:$rs1), "vfmv.v.f", "$vd, $rs1">,
+               Sched<[WriteVFMovV, ReadVFMovV, ReadVFMovV]>;
+
----------------
Too many read operands? Should it read only something like `ReadVFMovF`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1349
+                       (ins VR:$vd, FPR32:$rs1), "vfmv.s.f", "$vd, $rs1">,
+               Sched<[WriteVFMovFV, ReadVFMovFV]>;
 
----------------
Missing read resource?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1390
+                           (ins VR:$vs2), "vmv" # nf # "r.v", "$vd, $vs2">,
+                    Sched<[WriteVMov1V, ReadVMov1V]> {
+  let Uses = [];
----------------
Do all nfs 1,2,4,8 have the same `1V` resource?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98002/new/

https://reviews.llvm.org/D98002