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

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Hi @dmgreen and @spatel, I've been trying to follow the discussion but I'm not entirely sure I follow what you're proposing @kmclaughlin should do here? Are you suggesting that Kerry could change this patch to:

1. Change InstCombine in two places so that a) prior to vectorisation the PHI inherits the FMF from the instruction it feeds into, then b) apply a second fold in InstCombine as well that changes all 0.0 values to -0.0, including for any possible vector PHIs? Obviously we have to remember InstCombine is run after the vectoriser too so will also look at vector PHIs.
2. Always set the default identity to be <-0.0, -0.0> regardless of the nsz flag?

I guess this might take some time to implement and test - I imagine there are issues/corner cases that might come up, such as what if the chain of FP operations includes a `fadd`, etc. with FMF different from the rest? Does that mean we cannot propagate FMF to the PHI?

The other possibility is to revert the patch back to the original so we at least get some sensible behaviour for now, whilst working on a second patch to investigate these new proposals?


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

https://reviews.llvm.org/D98963