[PATCH] D42323: [DAGCombiner] filter out denorm inputs when calculating sqrt estimate (PR34994)

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:17465
+        Attribute Denorms = F.getFnAttribute("denormal-fp-math");
+        if (Denorms.getValueAsString().equals("ieee")) {
+          // fabs(X) < SmallestNormal ? 0.0 : Est
----------------
arsenm wrote:
> Why doesn't this assume IEEE behavior if the attribute wasn't specified?
I had to go back through the comments in:
https://bugs.llvm.org/show_bug.cgi?id=34994

The concern was that adding runtime code to the estimate to deal with denorms would reduce performance on platforms (like x86 Linux or PlayStation) that would flush denorms anyway when building with -ffast-math. 

So we offered the explicit use of clang's "-fdenormal-fp-math=ieee" as a way to specify the seemingly uncommon case. If we say that LLVM assumes an IEEE target by default, then we could change that. But that means we need to explicitly set the function attribute for all targets in clang?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D42323