[PATCH] D137220: [LV][IVDescriptors] Fix recurrence identity element for FMin and FMax reductions.

[Karthik Senthil via Phabricator via llvm-commits]

karthiksenthil added a comment.

Thanks for the reviews! I've addressed the comments below -

In D137220#3901561 <https://reviews.llvm.org/D137220#3901561>, @dmgreen wrote:

> Do we have any tests for Max as well as Min? It looks like the llvm/test/Transforms/LoopVectorize/AArch64/scalable-reduction-inloop-cond.ll test may need updating.

I've added new tests in `llvm/unittests/Analysis/IVDescriptorsTest.cpp` to cover both FMin and FMax reductions. Updated the LIT test as well.

In D137220#3902612 <https://reviews.llvm.org/D137220#3902612>, @spatel wrote:

> Should these be guarded with an FMF check?
>
> The enum definition says:
>
>   FMin,       ///< FP min implemented in terms of select(cmp()).
>   FMax,       ///< FP max implemented in terms of select(cmp()).
>
> So if we have a NaN input:
> fmin(NaN, Inf) --> select (fcmp olt NaN, Inf), NaN, Inf --> Inf (so the NaN input did not survive)

Do you mean that identity value computation for FMin/FMax should be guarded with a `FMF.noNaNs()` check? What would be the identity value when the flag is absent?

> Is it possible to write unit-tests to check these directly? I see a llvm/unittests/Analysis/IVDescriptorsTest.cpp test file...

I've updated the test file with new tests, I can extend this for testing missing `nnan` flag scenario as well.


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

https://reviews.llvm.org/D137220