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

[Wolfgang Pieb via llvm-commits]

wolfgangp added inline comments.

================
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());
   }
----------------
echristo wrote:
> 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?
I have to agree that it's not a good solution, and the only reason I prefer it over the status quo is that I haven't been able to come up with a case where an expression is actually attributed to a line that doesn't compute it (likely the preceding line), whereas there's at least one case that suffers from the status quo (PR21006). Anecdotal evidence, I know. 


http://reviews.llvm.org/D12094