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

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
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
----------------
hfinkel wrote:
> Why did you add { } here?
No good reason - leftover bits from an earlier draft where I had added a temp variable. Will fix.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4290
@@ +4289,3 @@
+    if (auto *FlagsNode = dyn_cast<BinaryWithFlagsSDNode>(Node))
+      Flags = &FlagsNode->Flags;
+
----------------
hfinkel wrote:
> This is repeated a lot. Can we add a utility function to SDNode that does this?
> 
Sure - that would have saved a lot of cut and paste. :)
I think this would be a virtual function in SDNode that returns a nullptr and derived classes would return a pointer to their SDNodeFlags if they have them. However, I see that SDNode has no virtual functions, so adding one means I should also add a virtual destructor? But is the lack of virtual functions in this class to avoid a vtable for size/perf? If that's a conscious design decision, is it better to fake a virtual function with something like:
   const SDNodeFlags *SDNode::getFlags() const {
     if (auto *FlagsNode = dyn_cast<BinaryWithFlagsSDNode>(this))
       return &FlagsNode->Flags;
     return nullptr;
   }



http://reviews.llvm.org/D12095