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

[Wang Pengcheng via Phabricator via llvm-commits]

wangpc added a comment.

Nice! I just have some suggestions oncode style.



================
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),
----------------
This class `VLXSEGSchedMask` should be used.



================
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,
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155654