[PATCH] D12094: Avoid the propagation of debug locations in SelectionDAG via CSE

[Eric Christopher via llvm-commits]

echristo requested changes to this revision.
echristo added a comment.
This revision now requires changes to proceed.

I've made an inline comment on this, if you could reply that would be good. Please don't commit this yet.

Thanks!

-eric


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:997-1001
@@ -995,13 +996,7 @@
   if (N) {
-    switch (N->getOpcode()) {
-    default: break; // Process only regular (non-target) constant nodes.
-    case ISD::Constant:
-    case ISD::ConstantFP:
-      // Erase debug location from the node if the node is used at several
-      // different places to do not propagate one location to all uses as it
-      // leads to incorrect debug info.
-      if (N->getDebugLoc() != DL)
-        N->setDebugLoc(DebugLoc());
-      break;
-    }
+    // Erase the debug location from the node if the node is used in more
+    // than one place. Otherwise the location may be propagated to 
+    // all uses or, in some cases, to the wrong use.
+    if (N->getDebugLoc() != DL)
+      N->setDebugLoc(DebugLoc());
   }
----------------
This is an interesting perspective. I'm not sure I agree though. Things like CSE'ing of nodes can cause attribution of expressions to a line that does an equivalent computation, but this change is going to associate the expression to a line that doesn't do the expression at all in the general case. How is this better than the current status quo?


http://reviews.llvm.org/D12094