[PATCH] D137530: [RISCV] Splat scalar to be of length VL instead of 1 for reductions

[Wang Pengcheng via Phabricator via llvm-commits]

pcwang-thead marked an inline comment as done.
pcwang-thead added a comment.

In D137530#3978568 <https://reviews.llvm.org/D137530#3978568>, @reames wrote:

> I'm looking at your test changes - not the code - and it really seems like the sole benefit of this here is vsetvli insertion.

Yes, you're right. My intention is to reduce `vsetvli`s.

> I'd default to expecting this to belong inside RISCVInsertVSETVLI.cpp not during ISEL.

Thanks for your suggestions, I will do some works on RISCVInsertVSETVLI.cpp to see if it is beneficial.

> From what I can tell glancing at the test deltas, you're effectively proposing the following rules:
>
> - A scalar insert w/TA demands the "non-zeroness" of the AVL.
> - A splat or broadcast load w/TA demands the an equal or greater AVL (with the same LMUL).
>
> From what I can tell, we have no reason to prefer the v.x/v.i/v.f form over the s.x/s.f forms.  The changes in your diff appear to be an artifact of how you implemented the above rules, not a benefit on their own.  Unless you disagree with this, I would ask that you rework the patch and patch description to make that obvious.
>
> Worth noting is that even if you decide to keep this in ISEL, there's nothing about the rules above which are specific to reductions.

I have no strong opinion on this. Let's see what we can do in RISCVInsertVSETVLI.cpp first. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137530