Page MenuHomePhabricator

[DebugInfo] Improve debug info accuracy for locals after inlining alloca uses [2/3]
Needs ReviewPublic

Authored by Orlando on Nov 13 2020, 7:17 AM.

Details

Summary

See PR47946.

Improve debug info accuracy for locals after inlining alloca uses by teaching
the inliner to run a localized LowerDbgDeclare over those uses.

We need to do this because InstCombine's LowerDbgDeclare marks variables backed
by an alloca as located in that alloca with a dbg.value+deref at call sites
using the alloca. These locations may be incorrect after inlining. For example,
it might become possible to eliminate some stores. It would be misleading to
use the alloca as the variable location after DSE has taken place because the
alloca location won't always hold the expected value. To counter this, we need
to track assignments to the caller's alloca-backed variables through the
inlined blocks, which we achieve with a localized LowerDbgDeclare.

Just as LowerDbgDeclare deletes the dbg.declare, InlineLowerDbgDeclare deletes
the dbg.value+deref as it's no longer always true that the variable lives on
the stack at this point. The example below illustrates why we must remove the
dbg.value+deref (marked XXX) after inlining. If we do not then we will emit
debug info saying that 'param' is located on the stack throughout the inlined
function and on line STEP, which isn't true due to DSE having removed the
redundant store 'param = 2'.

int g;
void escape(int*);
__attribute__((__always_inline__))
void no_deref(int* p) {
  if (p)
    g = 5;
}
int fun(int param) {
  param = 2;                   //< Store killed after inlining 'no_deref'.

  // XXX dbg.value(%param.alloca, "param", DW_OP_deref)
  no_deref(&param);

  volatile int step = 0;       //< STEP.
  param = 4;                   //< The killing store.

  escape(&param);
  return 0;
}

debuginfo-test/dexter-test changes:
Added memvars/inlining-unused-param-dse.c
Added memvars/inlining-dse-alloca-survives.c
Added memvars/two-inlined-calls.c, currently XFAILS, addressed in next patch.
Fixed memvars/inlining-dse.c and remove XFAIL as this patch fixes the issue.

Testing:
No unexpected failures with 'ninja check-clang' and 'llvm-lit llvm/test'.
Added llvm/test/DebugInfo/Generic/inline-alloca-use.ll

Diff Detail

Event Timeline

Orlando created this revision.Nov 13 2020, 7:17 AM
Orlando updated this revision to Diff 316593.Jan 14 2021, 2:00 AM

Rebase and fix a typo in a comment.

Orlando added a subscriber: bjope.EditedJan 14 2021, 2:58 AM

As @bjope points out on D91425, it should be mentioned that while this solution will improve the debugging experience in a number of cases it isn't bulletproof. For example a pass could insert a non-debug instruction between the dbg.value+deref and the to-be-inlined call.