[PATCH] D98963: [LoopVectorize] Change the identity element for FAdd

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D98963#2650388 <https://reviews.llvm.org/D98963#2650388>, @dmgreen wrote:

> It would presumably need to be SROA that added flags to phi's it created? I'm not sure where it would get that info from though.

I'm not sure either. We might need to apply FMF to load, stores, and function args to fill the gap. Another option might be to back-propagate the FMF from the `fadd` to its phi operand:

  %s.0 = phi float [ 0.000000e+00, %entry ], [ %add, %for.body ]
  %add = fadd fast float %s.0, %0

Not sure if there's some corner-case I'm overlooking, but I'm imagining something like what instcombine does to fill-in / restore no-wrap flags (and so the FMF setting could also happen in instcombine).

>> Do we know why we're seeing the mixed zeros in the tests? Is it an artifact of the test (because the test includes a +0.0 to start with) that we don't see in practice?
>
> I think most reductions will start out as
>
>   float s = 0;
>   for(int i = 0; i < n; i++)
>     s += x[i];
>
> I feel it would be unusual for a user to deliberately use -0.0 as the start value! 0 is always going to be the most common choice.

Yes, I agree - hardly anyone considers the subtlety of -0.0 in FP math, so it's almost never specified in source.


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

https://reviews.llvm.org/D98963