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

Andrea Di Biagio via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 2 11:29:19 PDT 2016

andreadb added a comment.

> > @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?

Sure, I will send you our patches (we have a few fixes - not only the tail-merging - that we can share). If you don't mind, I will send those patches to you on monday as it is almost end of the day here..

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

Interesting, I will look at it.
At the moment I am using https://reviews.llvm.org/D24180 and it seems to be doing a good job with sample pgo. In my experiments, it fixes a few nasty cases where instructions at the top of a basic block are implicitly attributed to the source line for the physically preceding instruction simply because they don't have a debug loc. Anyway, I will explain all the details in the email that I will send to you :-).

> 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);

I guess that works well because you specify -use-unknown-locations=true (our patch is almost identical btw).



More information about the llvm-commits mailing list