[PATCH] D100856: [RISCV] Support STEP_VECTOR with a step greater than one

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks @frasercrmck !



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:3774
+      SDValue StepVal =
+          DAG.getNode(RISCVISD::VMV_V_X_VL, DL, VT,
+                      DAG.getConstant(Log2_64(StepValImm), DL, XLenVT));
----------------
frasercrmck wrote:
> rogfer01 wrote:
> > I used `DAG.getSplatVector` here (I guess makes little difference) but I didn't consider the power-of-two case.
> > 
> > I'm curious that we cannot combine a case like this
> > 
> > ```
> >       t13: nxv1i64 = RISCVISD::VID_VL t12, Register:i64 $x0
> >       t14: nxv1i64 = splat_vector Constant:i64<2>
> >     t15: nxv1i64 = mul t13, t14
> > ```
> > 
> > into 
> > 
> > ```
> >       t13: nxv1i64 = RISCVISD::VID_VL t12, Register:i64 $x0
> >       t16: nxv1i64 = splat_vector Constant:i64<1>
> >     t15: nxv1i64 = shl t13, t16
> > ```
> > 
> I don't think we can use `getSplatVector` because at this stage for i64 vectors on RV32, we can't introduce an illegal i64 type and so it would be `nxvXi64 = splat_vector i32` which is ill-formed.
> 
> Regarding the second part, I think with `splat_vector` that would be possible at this stage -- it relies on `getNode(ISD::MUL)` folding constant arithmetic as we're post-combine -- but as I mentioned we can't use that node.
> 
> Perhaps if we conditionally used `splat_vector` and `splat_vector_parts` it would work, but `splat_vector_parts` isn't yet hooked up to the optimization machinery. I think it's better to do this awkward MUL/SHL lowering ourselves so that rv32 and rv64 behave the same way, but I can appreciate the //DRY// side of the argument.
RIght, thanks for the explanation I had forgotten about rv32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100856