[PATCH] D155654: [RISCV] Add SchedRead for Merge operands on MASK Pseudos

[Michael Maitland via Phabricator via llvm-commits]

michaelmaitland added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:258
 ]>;
+class VLXSEGSchedMask<int nf, int eew, string isOrdered, string emul = "WorstCase"> : Sched<[
+  !cast<SchedReadWrite>("WriteVL" #isOrdered #"XSEG" #nf #"e" #eew #"_" #emul),
----------------
wangpc wrote:
> This class `VLXSEGSchedMask` should be used.
> 
Thanks for catching this


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:2172
+          defvar ReadWritesMask = InsertAtIdx1<ReadWrites, ReadVMergeOp_MX_E>.l;
+          defm _VV : VPseudoBinaryEmul<m.vrclass, m.vrclass, emul.vrclass, m,
+                                       emul, ReadWrites, ReadWritesMask,
----------------
wangpc wrote:
> wangpc wrote:
> > We should add some multiclasses to define `Sched`s (just like existed multiclasses for load/store, e.g., `VLESchedMask`, ) instead of passing `SchedRead/SchedWrite`s to these multiclasses which are used to define pseudos. They make the code messy, I think.
> > We can seperate these multiclasses into other patches to make this patch easy to review. I am pleased to help you with this.
> `multiclass`->`class`
> 
> Please see D155932, feedback is welcome.
I agree with your proposal in D155932. I think this comment can be resolved and this patch will make the required changes after rebasing once D155932 lands?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155654