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

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
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));
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100856