[PATCH] D12095: propagate fast-math-flags on DAG nodes

[hfinkel@anl.gov via llvm-commits]

hfinkel added a comment.

Thanks for continuing to work on this! I think this sets out a fairly-concrete plan to move forward (we need to do this, and then also add flags on FNEG, the comparisons and intrinsics). We need the flags on intrinsics at the IR level anyway.

> Note: I didn't try to locate all the spots where the existing nsw / nuw / exact flags should be propagated, but it doesn't look like those are handled correctly either. Maybe it doesn't matter as much? Nobody has reported a problem from those, and they've been in the DAG for over a year ( http://reviews.llvm.org/rL210467 ).


This is not a safe assumption; performance can vary a lot because of the rate of development and other factors, and the number of small performance regressions that are investigated in detail is very small.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3304
@@ -3300,3 +3303,3 @@
     break;
-  case ISD::FNEG:
+  case ISD::FNEG:{
     // Expand Y = FNEG(X) ->  Y = SUB -0.0, X
----------------
Why did you add { } here?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4290
@@ +4289,3 @@
+    if (auto *FlagsNode = dyn_cast<BinaryWithFlagsSDNode>(Node))
+      Flags = &FlagsNode->Flags;
+
----------------
This is repeated a lot. Can we add a utility function to SDNode that does this?



http://reviews.llvm.org/D12095