[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 :-).
> 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