[PATCH] D106403: [RISCV] Avoid using x0,x0 vsetvli for vmv.x.s and vfmv.f.s unless we know the sew/lmul ratio is constant.

Wed Jul 21 09:32:53 PDT 2021

craig.topper added a comment.

In D106403#2892606 <https://reviews.llvm.org/D106403#2892606>, @frasercrmck wrote:

> In D106403#2892491 <https://reviews.llvm.org/D106403#2892491>, @craig.topper wrote:
>
>> Use 0 instead of 1.
>
> The patch summary will need update to mention 0 instead of 1.
>
> Interesting that we've been having `zero,zero` at the beginning of functions all this time and I'd never thought that this is very likely to break.
>
> I'm wondering about the pros/cons of adding a VL to the C functions. I think I'm missing something: would it really change much? If the SEW/LMUL ratio remains the same we'll omit the `vsetvli`, else we'll add one as we're doing in this patch. Is it likely to improve much about the generated code or is it just the "right" thing to do because it's not arbitrarily decided to be `0`?

Having the user provide a value may allow us to use the same vsetvli that's needed by code after the vmv.x.s if the user can tell us that the vmv.x.s and later code all uses the same VL. Our current vsetvli insertion algorithm can't detect that. I suppose we could defer the insertion for vmv.x.s until we see what the next instruction and try to match it ourselves, but that add more complexity.

Out of order cores may want to speculate on the value of VL rather than waiting on the scalar computation for it. This would require the VL to be somewhat predictable. I'm not sure making up a number would play well with that.

It would allow us to remove some code from the vsetvli insertion pass if these instructions weren't special.

OTOH, it is perhaps confusing to provide vl for the intrinsic when the instruction doesn't need it. We'll need to rework isel patterns and may not be able to use ISD::EXTRACT_VECTOR_ELT directly for floating point.

I think we need to weigh the options here. I'm leaning towards adding the VL to the intrinsics and instructions, but I might be persuaded to just keep this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106403