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

Fri Jul 21 08:12:01 PDT 2023

michaelmaitland added inline comments.

================
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,
----------------
michaelmaitland wrote:
> 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?
It might be easier to wait to make your changes until this lands, otherwise one of us is going to be stuck rebasing unnecessarily. What do you think?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155654