[PATCH] D111077: [LV] Support converting FP add to integer reductions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D111077#3047408 <https://reviews.llvm.org/D111077#3047408>, @dmgreen wrote:

> In D111077#3040938 <https://reviews.llvm.org/D111077#3040938>, @fhahn wrote:
>
>> In D111077#3040417 <https://reviews.llvm.org/D111077#3040417>, @dmgreen wrote:
>>
>>> Interesting idea. Are these two bits of code always the same?
>>> https://godbolt.org/z/EfPKPTMdf
>>
>> I think both cases above should be the same. But I think we can construct slight variations where is they would not be. E.g. consider a loop where the induction variable starts at 0 and is incremented and overflow is allowed. If `n` would be negative, the result of removing the loop and converting `n` to a float would yield a negative number , but the loop version would always return a positive number. I might be missing some subtleties when it comes to sign handling, perhaps @scanon as further thoughts.
>
> Yep, I was ignoring the negative numbers :) I meant more about the general idea of converting the loop to straight line code.
>
>>> Should we be doing this more generally, outside the vectorizing reductions?
>>
>> I think it might be worthwhile to convert such reductions outside the vectorizer in some cases. My motivation for starting in LV is that it should be clearly profitable if it allows vectorization. For general loops without vectorization, it might not be profitable I think, e.g. for loops that only execute once, due to the conversion overhead.
>
> And that's not including vectorization. It's kind of like a "high cost expansion" from SCEV (but to be fair as far as I understand we wouldn't always rewrite high cost exit values, even if it would mean deleting the loop (?)).  Which is what made me wonder if we should be doing it generally, not just in the vectorizer. (Not that I have anything against this patch - it looks pretty sensible and doesn't complicate the reduction code any more than it already is. It seems to fit quite well).

I think the case you shared is should be beneficial. I'd need to check where it would fit best and I think we'd need a few extra cost checks, but it shouldn't be too tricky.

I think there will still be cases in which it may not be profitable to perform the transformation on its own, so it might be good to also support this in LV, especially as we should be able to support it quite naturally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111077