[PATCH] D90772: [Coroutines] Add missing llvm.dbg.declare's to cover more allocas

[Bruno Cardoso Lopes via Phabricator via llvm-commits]

bruno created this revision.
bruno added reviewers: jmorse, lxfind, aprantl, GorNishanov, vsk.
Herald added subscribers: llvm-commits, modimo, wenlei, ributzka, jkorous, hiraditya.
Herald added a project: LLVM.
bruno requested review of this revision.

Tracking local variables across suspend points is still somewhat incomplete. Consider this coroutine:

  // Complete code here: https://gist.github.com/bcardosolopes/a992950bdfc66ab4ce6dc0c75920f4ef
  resumable foo() {
    int x[10] = {};
    int a = 3;
    co_await std::experimental::suspend_always();
    a++;
    x[0] = 1;
    a += 2;
    x[1] = 2;
    a += 3;
    x[2] = 3;
  }

Can't manage to print `a` or `x` if they turn out to be allocas during CoroSplit (which happens if you build this code with `-O0` against ToT):

  * thread #1, queue = 'com.apple.main-thread', stop reason = step over
      frame #0: 0x0000000100003729 main-noprint`foo() at main-noprint.cpp:43:5
     40     co_await std::experimental::suspend_always();
     41     a++;
     42     x[0] = 1;
  -> 43     a += 2;
     44     x[1] = 2;
     45     a += 3;
     46     x[2] = 3;
  (lldb) p x
  error: <user expression 21>:1:1: use of undeclared identifier 'x'
  x
  ^

The generated IR contains a `llvm.dbg.declare` for `x` in it's initialization basic block. However, even though this BB dominates all other BBs where `x` is manipulated, that doesn't seem to be enough debug info for the debugger to be happy. By adding extra `llvm.dbg.declare`s in these BBs, lldb prints `x` successfully. Is this how `llvm.dbg.declare`s supposed to work or am I missing something? Given the perceived behavior, this patch improves CoroSplit by placing extra `llvm.dbg.declare`s in all basic blocks that need some "refresh" for the frame location to be found, so this:

  await.ready:
    ...
    %arrayidx = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 0, !dbg !760
    ...
    %arrayidx19 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 1, !dbg !763
    ...
    %arrayidx21 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 2, !dbg !766

becomes:

  await.ready:
    ...
    call void @llvm.dbg.declare(metadata [10 x i32]* %x.reload.addr, metadata !751, metadata !DIExpression()), !dbg !753
    ...
    %arrayidx = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 0, !dbg !760
    ...
    %arrayidx19 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 1, !dbg !763
    ...
    %arrayidx21 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 2, !dbg !766

For additional context, this builds up on top of changes from D75338 <https://reviews.llvm.org/D75338> back in Feb. I also plan to add a LLDB end-to-end test for coroutines in a followup patch once this is fixed.


https://reviews.llvm.org/D90772

Files:
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90772.302877.patch
Type: text/x-patch
Size: 8943 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201104/dffda7bc/attachment.bin>