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

Dehao Chen via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 2 10:21:06 PDT 2016


danielcdh added a comment.

In https://reviews.llvm.org/D24164#532702, @andreadb wrote:

> In https://reviews.llvm.org/D24164#532594, @dblaikie wrote:
>
> > I would worry that this might produce a worse experience for a sanitizer user, for example - but I'm not sure. (the sanitizer user won't have any trail back to why the store in this example is happening - granted, the way the code currently works, they may be told the store is happening in the wrong branch, which isn't great either)
>
>
> I don't know about the sanitizer user experience. However, based on my experience with sample pgo, this kind of changes tend to improve the quality of the sample profile from autofdo.
>
> @Dehao,
>  we have identified a couple of similar issues where the debug location of a tail-merged instruction is incorrectly set. We already have plans to send a couple of small patches to fix these issues (those would need https://reviews.llvm.org/D24180); in our sample-pgo experiments, this kind of small fixes for hoisted/tail merged instructions tend to have a very positive impact on the quality of the sample profile.


I also have a related debug-info fix for tail-merging (attached below), but haven't had time to make a unittest and send out for review. It would be great if your patch can cover this case, so that I don't need to prepare a patch for it :)

Could you send us your debug info patches for sample pgo so that we can test them on our workload to see performance impact?

About https://reviews.llvm.org/D24180, we just use -mllvm -use-unknown-locations=true for similar purposes.

Dehao

Index: ../lib/CodeGen/BranchFolding.cpp
=======================================

- ../lib/CodeGen/BranchFolding.cpp	(revision 280454)

+++ ../lib/CodeGen/BranchFolding.cpp	(working copy)
@@ -939,6 +939,11 @@

  MachineBasicBlock *MBB = SameTails[commonTailIndex].getBlock();
   

+    // Remove debug info
+    for (MachineInstr &MI : *MBB)
+      if (!MI.isDebugValue())
+        MI.setDebugLoc(nullptr);
+

  // Recompute common tail MBB's edge weights and block frequency.
  setCommonTailEdgeWeights(*MBB);
   


https://reviews.llvm.org/D24164





More information about the llvm-commits mailing list