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

[Andrea Di Biagio via llvm-commits]

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

Cheers,
Andrea


https://reviews.llvm.org/D24164