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

[Wang Pengcheng via Phabricator via llvm-commits]

pcwang-thead 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);
----------------
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. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151653