[PATCH] D126794: [RISCV] Lower VLEFF/VLSEGFF SDNodes to MachineInstrs with VL outputs.

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 8 08:07:25 PDT 2022


reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

Please refresh with prior comments addressed so I can LGTM.



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:695
 
+class VPseudoUSLoadFFNoMask<VReg RetClass, int EEW, bit DummyMask = 1> :
+      Pseudo<(outs RetClass:$rd, GPR:$vl),
----------------
fakepaper56 wrote:
> fakepaper56 wrote:
> > craig.topper wrote:
> > > fakepaper56 wrote:
> > > > reames wrote:
> > > > > Unless I'm missing something, the need for separate FF pseudos disappears once you remove the VLOperand flag doesn't it?
> > > > The VLOperand you mentioned is HasVLOutput in my code? I think we should still use separate FF pseudos, since the output of VLEFF/VLSEGFF needs two explicit defines.
> > > Did we rule out making it an OptionalDef and setting the def to NoRegister?
> > Sorry I forgot OptionalDef, I will try to use it.
> I think OptionalDef is only for physical register, the case needs virtual register fit into the output. There is another solution to merge VPseudoUSLoad and VPseudoUSLoadFF is to make RISCVVLE and RISCVVLSEG have two explicit defines, but I think the way is too aggressive.
Let's drop this point for now.  I'm fine with this landing with this particular point unaddressed, we can revisit in tree.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126794



More information about the llvm-commits mailing list