[PATCH] D101727: Fix PR47960 - Incorrect transformation of fabs with nnan flag

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

1. Please upload patch with full context:

https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

2. I prefer that we add more tests rather than alter the FMF on the existing tests. We want to be able to verify that all of these flag permutations are behaving as expected. We can commit the baseline tests as a preliminary step, so we just see the diffs in this review. If you do not have commit access, let me know so I can commit for you.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2867-2868
   // fast-math-flags (nsz) or fsub with +0.0 (not fneg) for this to work. We
   // also require nnan because we do not want to unintentionally change the
   // sign of a NaN value.
   // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X)
----------------
If we are dropping the nnan requirement, this comment should be removed. Can we make that change independently of the change that propagates flags from the select without breaking anything? If so, let's do that first as its own patch before this one.


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

https://reviews.llvm.org/D101727