[RFC][Debugify] False positives elimination

IR & Optimizations
1

Remove false positve debug location losses caught by Debugify

The problem of “false positive results” in Debugify reports was discussed at one of the LLVM Dev meetings. This RFC aims at solving this problem for reports about lost Debug Locations (instruction source locations).

About debugify

Debugify utility, is a pair of IR passes, where the first one runs before an optimization pass and the second one runs after it. It has two modes:

  • Synthetic mode (debugify) - check preservation of the artificially added debug info
  • Original mode (verify-debuginfo-preserve) - check preservation of the debug info previously produced by compiler (with -g option)

First debugify pass records the DebugInfo state of the code in the following way:

  • Synthetic mode - adds artificial debug info to each possible location (ex. DebugLoc to each instruction)
  • Original mode - collects existing debug info in the code (ex. saves DebugLoc of each instruction)

Second debugify pass checks the state of debug info in the code, after the optimization, and then
compares it to the state before the optmization, and then reports the losses:

  • Synthetic mode - checks if each possible location in code, contains corresponding debug-info (ex. each instruction should have a DebugLoc)
  • Original mode - collects existing debug info in the code and compares to the saved state, before the pass (ex. if instruction had DebugLoc before the pass, it should have it after the pass)

DebugLoc drop, but not DebugLoc loss

There are several scenarios where debug locations are dropped, but should not be reported by Debugify as losses:

  • instruction’s DebugLoc should be dropped if it cannot be unambiguously determined (according to How-to-update-debug-info document)
  • instruction’s DebugLoc is copied from an other instruction (loss should not be reported for both instructions)

In my opinion, those cases should not be treated as losses, since DebugLocs are explicitly updated, but couldn’t be reconstructed.

Solution proposal

My proposal, for resolving this, is to add a flag to the Instruction (droppedLocation), which should be set if the DebugLoc is explicitly set to empty for that instruction. Then, during Debugify analysis, we should not report DebugLoc loss if instruction’s DebugLoc is explicitly updated (by checking the flag). (RFC patch)

Results

Proposed solution was tested by compiling clang-14 with -fverify-debuginfo-preserve. The results are shown bellow (eliminated around 65% of reported DebugLoc losses).

LLVM Pass Name Number of losses (before) Number of losses (after) Percentage of eliminated
ADCEPass 4 4 0.00%
AlignmentFromAssumptionsPass 3069 12 99.61%
CoroElidePass 2134 91 95.74%
CorrelatedValuePropagationPass 255 252 1.18%
DeadArgumentEliminationPass 3120 2229 28.56%
DivRemPairsPass 7 7 0.00%
GCOVProfilerPass 3 3 0.00%
GVNPass 12972 153 98.82%
GlobalOptPass 12 12 0.00%
IPSCCPPass 23239 23125 0.49%
InstCombinePass 7647 6607 13.60%
JumpThreadingPass 17724 13551 23.54%
LoopDistributePass 72 35 51.39%
LoopLoadEliminationPass 14 0 100.00%
LoopSimplifyPass 7735 14 99.82%
LoopUnrollPass 2430 814 66.50%
LoopVectorizePass 10884 4732 56.52%
MemCpyOptPass 5 0 100.00%
MergedLoadStoreMotionPass 3 3 0.00%
ReassociatePass 3544 3444 2.82%
RelLookupTableConverterPass 1 1 0.00%
SCCPPass 15505 16068 -3.63%
SLPVectorizerPass 357 347 2.80%
SROAPass 29683 10654 64.11%
SimplifyCFGPass 343408 86807 74.72%
TailCallElimPass 9 9 0.00%
TOTAL 483836 168974 65.08%

I am not sure if there is a better way for resolving this, maybe without changing Instruction class implementation.
Please, share your thoughts.

Thanks,
Nikola

4 Likes
2

Thanks for looking at this, the basic approach of explicitly ignoring intentionally dropped locations seems sensible. I’m curious about the results for SCCPPass: I would expect this change to strictly filter out existing losses, but the number of losses has increased following this patch, do you know why that is? Other than that this seems broadly good, will take a look at the patch later.

3

Hi @StephenTozer, thanks for the comment. I will check what is the actual case with SCCP pass. I agree that is a strange behavior, which is not wanted.

4

This sounds like a good way of scanning for source locations where we could do better – I think others have chipped in with most of my thoughts, such as what Min suggests in ⚙ D147134 [RFC][Debugify] Remove false positve debug location losses . Adding a new field to Instruction that’s only used during compiler evaluation and internal tests will add an avoidable cost to using LLVM in production.

The results are interesting by themselves – I’d expect any pass that did dead-code-elimination to have a large number of remaining losses, so it’s no surprise that SimplifyCFG and SROA still have a lot of missing locations. However, I would have expected a lot of reduction for Reassociate, so it’s curious that there are still many missing locations there.

5

Hi @StephenTozer
I’ve run the analysis again, this time with the droppedLocation flag introduced (but just not used) before fix and used after the fix, and the results (table below) show no regression for any pass.

LLVM Pass Name Number of bugs (begore - flag used) Number of bugs (after) Percentage of eliminated
ADCEPass 4 4 0.00%
AlignmentFromAssumptionsPass 1386 12 99.13%
CoroElidePass 2462 91 96.30%
CorrelatedValuePropagationPass 256 252 1.56%
DeadArgumentEliminationPass 3103 2229 28.17%
DivRemPairsPass 7 7 0.00%
GCOVProfilerPass 3 3 0.00%
GVNPass 12585 153 98.78%
GlobalOptPass 12 12 0.00%
IPSCCPPass 23323 23125 0.85%
InstCombinePass 7364 6607 10.28%
JumpThreadingPass 17625 13551 23.11%
LoopDistributePass 72 35 51.39%
LoopLoadEliminationPass 14 0 100.00%
LoopSimplifyPass 7975 14 99.82%
LoopUnrollPass 2687 814 69.71%
LoopVectorizePass 5078 4732 6.81%
MemCpyOptPass 5 0 100.00%
MergedLoadStoreMotionPass 3 3 0.00%
ReassociatePass 3566 3444 3.42%
RelLookupTableConverterPass 1 1 0.00%
SCCPPass 16069 16068 0.01%
SLPVectorizerPass 347 347 0.00%
SROAPass 30247 10654 64.78%
SimplifyCFGPass 341093 86807 74.55%
TailCallElimPass 9 9 0.00%
TOTAL 475296 168974 64.45%

What comes to my mind, is that an additional field, increases the size of Instruction objects, which causes more frequent moves of those objects. The Instruction object with different address can be detected as a new instruction, with no debug-loc.

@jmorse Thanks for the comment! Two ideas came up instead of a new Instruction field:

  1. Using spare bit in an existing bit vector (if there is any)
  2. Using a special DILocation (more special than (0,0))

I agree, the results are interesting. However, detected debug-loc losses are not necessary going to affect the final debug-info. For example, there is an instruction without debug-loc, generated in SCCP pass, which causes most of the detected debug-loc drops for that pass. But, that is an unconditional branch instruction, which is usually optimized out later, so its debug location is irrelevant to the debugging. (the diff below from the comment in D100371 )

diff --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
  index 86705b8622a3..3ac195a5c01a 100644
  --- a/llvm/lib/Transforms/Scalar/SCCP.cpp
  +++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
  @@ -1903,7 +1903,8 @@ static bool removeNonFeasibleEdges(const SCCPSolver &Solver, BasicBlock *BB,
         Updates.push_back({DominatorTree::Delete, BB, Succ});
       }
  
  -    BranchInst::Create(OnlyFeasibleSuccessor, BB);
  +    auto BranchInst = BranchInst::Create(OnlyFeasibleSuccessor, BB);
  +    BranchInst->setDebugLoc(TI->getDebugLoc());
       TI->eraseFromParent();
       DTU.applyUpdatesPermissive(Updates);
     } else if (FeasibleSuccessors.size() > 1) {