[PATCH] D151653: [WIP][RISCV] Combine vmv.s.x of constants into vmv.v.i

[Luke Lau via Phabricator via llvm-commits]

luke added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12348
+    // fits into the OPIVI format.
+    if (N->getOperand(0).isUndef() && isa<ConstantSDNode>(N->getOperand(1))) {
+      uint64_t C = N->getConstantOperandVal(1);
----------------
pcwang-thead wrote:
> luke wrote:
> > pcwang-thead wrote:
> > > We may need to limit it to `LMUL<=1` in order not to increase register pressure.
> > > 
> > > Similar patches:  D139656 and its related patches.
> > Thanks for pointing this out, just want to check my understanding here: Do we currently allocate M2/4/8 register classes for vmv.s.x of LMUL>1, or do we ignore register groups for it yet? I see there's https://reviews.llvm.org/D139699 but it doesn't look to be upstream yet.
> > 
> > If we are allocating M2/4/8, then I think this transform from vmv.x.s to vmv.v.i will keep LMUL the same.
> > 
> > Otherwise if not, then I can see why we would need to add the check to avoid LMUL=1 ballooning to LMUL=8
> Your understanding is right that we allocate M2/4/8 register classes for vmv.s.x of LMUL>1. D139699 is just a PoC and I don't know if it's the right way to go. :-)
> I think I misunderstood your patch before, there is no need to consider the register pressure because LMUL won't change.
> I have no problem here now. :-)
No worries, thanks for the clarification! If the PoC does go in then let me know and I can revisit this to constrain LMUL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151653