[PATCH] D11866: transform fmin/fmax calls when possible (PR24314)

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1221
@@ +1220,3 @@
+    if (Attr.getValueAsString() != "true")
+      return nullptr;
+    // No-signed-zeros is implied by the definitions of fmax/fmin themselves.
----------------
hfinkel wrote:
> Actually, why do we need no NaNs? We don't support FP exceptions, so we only need to do the correct thing with NaN arguments (by returning the non-NaN). This should be easy to guarantee by picking the right ordered vs. unordered fcmp predicate.
> 
An unordered compare would let us know that at least one operand is NaN, but not which one. So we'd have to check each operand for NaN. We'd be rewriting an fmax() implementation in IR?

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1223
@@ +1222,3 @@
+    // No-signed-zeros is implied by the definitions of fmax/fmin themselves.
+    FMF.setNoSignedZeros();
+    FMF.setNoNaNs();
----------------
hfinkel wrote:
> What in the definition implies this?
The C standard is silent about signed zeros for these, but says this in a footnote:
"Ideally, fmax would be sensitive to the sign of zero, for example fmax(−0. 0, +0. 0) would return +0; however, implementation in software might be impractical."

Should we add that here in the comment?


http://reviews.llvm.org/D11866