[PATCH] D24164: Remove debug info when hoisting instruction from then/else branch.

[Eric Christopher via llvm-commits]

echristo added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1248
@@ -1247,1 +1247,3 @@
 
+    if (!isa<CallInst>(I1) &&  I1->getDebugLoc() != I2->getDebugLoc())
+      I1->setDebugLoc(nullptr);
----------------
I think you have a use after free here, IS->eraseFromParent() above.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1248
@@ -1247,1 +1247,3 @@
 
+    if (!isa<CallInst>(I1) &&  I1->getDebugLoc() != I2->getDebugLoc())
+      I1->setDebugLoc(nullptr);
----------------
echristo wrote:
> I think you have a use after free here, IS->eraseFromParent() above.
This needs a comment about the point of what we're doing.

That said while this may help the AFDO type of use case I think it might the debug experience worse for sanitizers/debuggers. For the easy case, take one of the instructions that's being hoisted and make it a load of NULL. If we assert/trap/etc on this then the back trace will give us a fairly unhelpful line in the basic block that doesn't correlate to any of the source that the user wrote.


https://reviews.llvm.org/D24164