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

Mon Jul 24 01:47:21 PDT 2023

wangpc 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:
> 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?
Can you please wait for my patch? I think it will make this patch much simpler. :-)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155654