[PATCH] D155218: [InstCombine] Optimize addition/subtraction operations of splats of vscale multiplied by a constant

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D155218#4624956 <https://reviews.llvm.org/D155218#4624956>, @nikic wrote:

> In D155218#4587835 <https://reviews.llvm.org/D155218#4587835>, @igor.kirillov wrote:
>
>>> Please make sure to pre-commit test coverage (https://llvm.org/docs/TestingGuide.html#precommit-workflow-for-tests). This is also missing some tests, in particular multi-use and negative tests.
>>
>> @nikic Added a test with `use`. Could you clarify what you mean by negative tests in this context?
>> I am aware of pre-commits. I just wasn't sure we wanted this patch at all. That's why I will pre-commit them just before landing the patch.
>
> Negative test = test where no transform happens. Generally speaking, you want one negative test for every condition in the transform, such that each test fails exactly one condition.
>
> In D155218#4576394 <https://reviews.llvm.org/D155218#4576394>, @paulwalker-arm wrote:
>
>> I guess I'm making a generalisation here based on scalable vectors needing to calculate addresses and the like based on vscale and so am assuming specialised add/sub instructions will always exist.  My eyes are not tuned in to reading rvv asm, can @craig.topper  or @reames help clarify whether this combine also makes sense from a RISCV point of view.
>
> To make the question more precise: If you have `spat(vscale * C1) + splat(vscale * C2)`, does it make sense to transform it into `splat(vscale * (C1+C2))` **if neither `splat(vscale *  C1)` nor `splat(vscale * C2)` are going away**?

RISC-V does not have any special add/sub instructions for vscale values. We define vscale as vector length in bytes divided by 8 and the vector length should always be divisible by 8. We have a read only status register called `vlenb` that returns the vector length in bytes. In the base case `vscale` is always expanded to a read of `vlenb` followed by dividing by 8 (shifted right by 3). If vscale is multiplied by a constant, we will try to combine the right shift with the multiplier so that we don't shift right only to immediately shift left again.

So I think @nikic is correct that for RISC-V this only makes sense for the one use case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155218