[PATCH] D146543: [Coroutines] Look for dbg.declare for temp spills

[Wei Wang via Phabricator via llvm-commits]

weiwang added a comment.

In D146543#4218316 <https://reviews.llvm.org/D146543#4218316>, @ChuanqiXu wrote:

> In D146543#4217194 <https://reviews.llvm.org/D146543#4217194>, @weiwang wrote:
>
>> In D146543#4215777 <https://reviews.llvm.org/D146543#4215777>, @ChuanqiXu wrote:
>>
>>>> Does your downstream patch address the same issue? Maybe we can consolidate here.
>>>
>>> Oh, not exactly. It looks like you're saving the debug information under O0. While we're trying to save the debug information within optimization, which might not be so natural. And it looks like the problem occurs within opaque pointer and our downstream compiler didn't start to use opaque pointer yet. So in another word, this should be regression bug if I understand things correctly.
>>
>> Yeah, we generally use `-O0` for better debugging experiences. The issue was first reported from our internal release, which is based on clang-12, so I think it exists before and after opaque pointer. My plan is to fix upstream and then backport.
>
> Got it. But let's  first try to handle it in `coro::salvageDebugInfo()` to make the code smell better.

I took a look at  `coro::salvageDebugInfo()`, but can't find an easy way to handle my case there.

In particular, I have the following code, in which the `dbg.declare` is for `%this.addr`, and its alias `%this1` is spilled.

  %this.addr = alloca ptr, align 8
  call void @llvm.dbg.declare(metadata ptr %this.addr, metadata !658, metadata !DIExpression()), !dbg !660
  %this1 = load ptr, ptr %this.addr, align 8
  ... ...
  ; spill
  %this1.spill.addr = getelementptr inbounds %_ZN9Container3fooEi.Frame, ptr %0, i32 0, i32 4, !dbg !1782
  store ptr %this1, ptr %this1.spill.addr, align 8, !dbg !1782
  .. ...
  ; reload
  %this1.reload.addr = getelementptr inbounds %_ZN9Container3fooEi.Frame, ptr %0, i32 0, i32 4, !dbg !1794
  %this1.reload = load ptr, ptr %this1.reload.addr, align 8, !dbg !1794

And I need to update the `dbg.declare` to describe the reload GEP `%this1.reload.addr`, something like

  call void @llvm.dbg.declare(metadata ptr %this1.reload.addr, metadata !658, metadata !DIExpression(DW_OP_plus_uconst, 32)), !dbg !660

Under `-O0`, it would be

  call void @llvm.dbg.declare(metadata ptr %frame, metadata !658, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg !660

And if the spill has multiple reloads, I'll need one `dbg.declare` for every reload.

In `coro::salvageDebugInfo()`, I'll need to trace from `%this.addr` to its reload GEP `%this1.reload.addr`. I feel I need to have an extra struct to record spill to its reloads (probably when the reloads are created in `insertSpills()`).  Also since I don't know if an alloca is an alias of a spill and leads to a reload GEP (such information is easier to get in `insertSpills()`), I'll have to do such check on every alloca, which is extra overhead.

Any suggestions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146543/new/

https://reviews.llvm.org/D146543