[RFC] [[Coroutine] [Debug] Salvage dbg.values

[ChuanqiXu]

Summary

The previous implementation of coro-split didn't collect values used by dbg instructions into the spills which made a log debug info unavailable with optimization on.
This patch tries to collect these uses which are used by dbg.values. In this way, the debugbility of coroutine could be as powerful as normal functions with optimization on.

To avoid enlarging the coroutine frame, this patch only collects dbg.value whose value is already in the coroutine frame. This decision may make some debug info getting unavailable. But if we are with optimization on, the performance issue should be considered first. And this patch would make the debugbility of coroutine to be better only without changing the layout of the frame.

Test-plan: check-llvm

[aprantl]

I know this is is tempting, but unfortunately we can't have codegen change based on a debug info intrinsic (see more detailed comment inline).

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2618

Unfortunately, this is violates a guiding principle in LLVM: Debug info may under no circumstances affect the generated code. In other words, the output of clang and clang -g followed by strip must be identical. If you want this behavior it must either be controlled by a separate flag, or it must be on all the time, even when compiling without debug info.

[ChuanqiXu]

Add option to control the behavior.

[ChuanqiXu]

@aprantl gentle ping~

[lxfind]

Looks like it's calling "llvm::dbgs()" directly.

btw is this patch based off main branch or some other internal patch? I wasn't able to arc patch this one. I would like to play with it locally.

[ChuanqiXu]

rebase with trunk

[ChuanqiXu]

In D97673#2632571, @lxfind wrote:

Looks like it's calling "llvm::dbgs()" directly.

btw is this patch based off main branch or some other internal patch? I wasn't able to arc patch this one. I would like to play with it locally.

Thanks for reminding. I just rebased it.

[lxfind]

Curious, is %0 = bitcast %a to ... the majority of the cases we are missing? If so, could we turn all the dbg.value instructions to use their original pointer by stripping pointer cast?

[ChuanqiXu]

In D97673#2646423, @lxfind wrote:

Curious, is %0 = bitcast %a to ... the majority of the cases we are missing? If so, could we turn all the dbg.value instructions to use their original pointer by stripping pointer cast?

I am not sure if %0 = bit cast %a to ... is the majority redundant fields of coroutine frame if we enable this patch. My original idea is to add a rematerialization process to reduce the coroutine frame like register allocation. However, it maybe a little hard and benefit less. It sounds more easy and beneficial to use original pointer by value tracking. Then I think this problem may be not introduced by this patch. Here is my example:

%0 = bitcast %a to ...
call @llvm.coro.suspend 
; ....
use of %a
use of %0

Then both %a and %0 would be put in the frame, which is totally redundant. We could see if there are examples other than bit cast in other patch.

[lxfind]

I am not sure if %0 = bit cast %a to ... is the majority redundant fields of coroutine frame if we enable this patch. My original idea is to add a rematerialization process to reduce the coroutine frame like register allocation. However, it maybe a little hard and benefit less. It sounds more easy and beneficial to use original pointer by value tracking. Then I think this problem may be not introduced by this patch. Here is my example:

%0 = bitcast %a to ...
call @llvm.coro.suspend 
; ....
use of %a
use of %0

Then both %a and %0 would be put in the frame, which is totally redundant. We could see if there are examples other than bit cast in other patch.

We already do materialization though. I wonder why that doesn't cover the case? Can we take advantage of that?

[ChuanqiXu]

In D97673#2646737, @lxfind wrote:

I am not sure if %0 = bit cast %a to ... is the majority redundant fields of coroutine frame if we enable this patch. My original idea is to add a rematerialization process to reduce the coroutine frame like register allocation. However, it maybe a little hard and benefit less. It sounds more easy and beneficial to use original pointer by value tracking. Then I think this problem may be not introduced by this patch. Here is my example:

%0 = bitcast %a to ...
call @llvm.coro.suspend 
; ....
use of %a
use of %0

Then both %a and %0 would be put in the frame, which is totally redundant. We could see if there are examples other than bit cast in other patch.

We already do materialization though. I wonder why that doesn't cover the case? Can we take advantage of that?

Yes, it makes since to cover the case. I would work on this later. Thank you!

[ChuanqiXu]

Materialize bit cast for dbg.values as suggested.

[jmorse]

When looping over the debug users of instructions and replacing the dbg.values operands, you should now use the "replaceVariableLocationOp" helper instead -- this avoids re-creating a dbg.value intrinsic, and should handle variadic variable locations (the patches for which are 95% landed) seamlessly.

Overall the aim of the patch makes sense to me, it's storing values used by debug intrinsics in the coroutine frame for later retrieval right? Is there a risk that some later coroutine optimisation will try to optimise the frame and overwrite the stored values-for-debug-users -- this can happen with stack slot colouring at the other end of the compiler.

I'm not sure how the rest of the community will feel about the approach; IIRC there's a "fake use" patch floating around that does this for all (non-coroutine) variables, which wasn't landed in the end.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

54

IMO: the description should explicitly refer to the fact that codegen will change as a result of -g, to avoid any users experiencing unexpected behaviour.

llvm/test/Transforms/Coroutines/coro-debug-dbg.values-O2.ll

6–8 ↗

(On Diff #332907)

nit: IMO you should check that the dbg.values operand is a specific LLVM-IR value (i.e., the frame pointer), rather than just checking the type. That protects against some future optimisation or error producing a pointer-typed undef as the dbg.value operand.

[jmorse]

llvm/test/Transforms/Coroutines/coro-debug-dbg.values-O2.ll

6–8 ↗

(On Diff #332907)

Scratch that, I didn't notice you were already doing that.

[lxfind]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

52

Do we still need this if we can materialize them?

[ChuanqiXu]

Use replaceVariableLocationOp and remove EnhanceDebugability option.

[ChuanqiXu]

In D97673#2647831, @jmorse wrote:

When looping over the debug users of instructions and replacing the dbg.values operands, you should now use the "replaceVariableLocationOp" helper instead -- this avoids re-creating a dbg.value intrinsic, and should handle variadic variable locations (the patches for which are 95% landed) seamlessly.

Overall the aim of the patch makes sense to me, it's storing values used by debug intrinsics in the coroutine frame for later retrieval right? Is there a risk that some later coroutine optimisation will try to optimise the frame and overwrite the stored values-for-debug-users -- this can happen with stack slot colouring at the other end of the compiler.

To my knowledge, if other passes try to optimize the frame, other passes are responsible to maintain the debug information.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

52

Yeah, I think so. If there is anything we are missing, we should improve on the materialization part.

[ChuanqiXu]

@aprantl gentle ping~

[ChuanqiXu]

Rebase with trunk and add option to control whether we would collect variables used only by dbg.vaues.
Although we can use materializing process to reduce the extra space, we can't prove we could get the same result with -g or not.
Previously, I think if we can't get the same result, the materialization part should take responsibility. However, I find that a pattern recently:
State before:

%a = alloca  ...
; ... some uses who wouldn't cross suspend points
call to coro.suspend()
; ... alternative path
store %v to %a
dbg.value(metadata %v, dbg variable for %a, ...

Then after some optimization, the store in some path would be eliminated:

%a = alloca  ...
; ... some uses who wouldn't cross suspend points
call to coro.suspend()
; ... alternative path
dbg.value(metadata %v, dbg variable for %a, ...

Then %a wouldn't be put into the frame before this patch. And it would be in the frame after this patch if we don't offer an option to control this. And materialization couldn't do much about this.

So I think the assumption before isn't solid, we should add option to control it indeed.

[aprantl]

I would recommend to pick a name for -enhance-debug-with-coroutine that does not contain debug. The name should make clear that this changes codegen, so nobody gets the idea of turning it as part of a -g option. For example, something like -coroutine-spill-all-locals.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2654

Can you add a comment explaining what is being done and why?

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

650

The changes in this file are NFC and look good.

[ChuanqiXu]

In D97673#2677198, @aprantl wrote:

I would recommend to pick a name for -enhance-debug-with-coroutine that does not contain debug. The name should make clear that this changes codegen, so nobody gets the idea of turning it as part of a -g option. For example, something like -coroutine-spill-all-locals.

The accurate semantic for this option is collect values used by dbg.values, which would make coroutine frame get larger in some cases. This option would work only if user turns -g on. The name -coroutine-spill-all-locals is not correct and it is hard for me to get a name which shows it would change the codegen and enhance the debugbility. I am wondering if we can remove this option and make collecting values used by dbg.values a default behavior. From our analysis above, it should be unusual to make the coroutine frame larger. @lxfind what's your opinion?

[aprantl]

In D97673#2678499, @ChuanqiXu wrote:

In D97673#2677198, @aprantl wrote:

I would recommend to pick a name for -enhance-debug-with-coroutine that does not contain debug. The name should make clear that this changes codegen, so nobody gets the idea of turning it as part of a -g option. For example, something like -coroutine-spill-all-locals.

The accurate semantic for this option is collect values used by dbg.values, which would make coroutine frame get larger in some cases. This option would work only if user turns -g on. The name -coroutine-spill-all-locals is not correct and it is hard for me to get a name which shows it would change the codegen and enhance the debugbility. I am wondering if we can remove this option and make collecting values used by dbg.values a default behavior. From our analysis above, it should be unusual to make the coroutine frame larger. @lxfind what's your opinion?

That's just not something allowed by LLVM policy. The presence of -g may not affect the generated code. Similarly, the presence or absence of debug intrinsics may not affect the generated code either. Is there a way to control this behavior without relying on debug info? For, example, could we make sure all allocas are spilled, regardless of whether they are referred to by a dbg.declare?

[lxfind]

I am also a bit skeptical about this patch.
Specifically I agree that dbg info should not affect the coroutine frame.
From what I can tell, dbg intrinsics are location insensitive, i.e. they can be put at any location. (correct me if I am wrong)
So a use of any value by dbg intrinsics should not cause the value to be put on the frame.
Perhaps we could first move all dbg intrinsics to a dedicated location (e.g. right after corobegin) before creating the frame, and copy them during function cloning?

[ChuanqiXu]

In D97673#2680725, @lxfind wrote:

I am also a bit skeptical about this patch.
Specifically I agree that dbg info should not affect the coroutine frame.
From what I can tell, dbg intrinsics are location insensitive, i.e. they can be put at any location. (correct me if I am wrong)
So a use of any value by dbg intrinsics should not cause the value to be put on the frame.
Perhaps we could first move all dbg intrinsics to a dedicated location (e.g. right after corobegin) before creating the frame, and copy them during function cloning?

It looks better to collect the dbg.values who wouldn't change the layout of the frame. I would try to make it.

[ChuanqiXu]

Address the comments. Now we would only collect dbg.values who would't change the layout of the frame.

[lxfind]

Could you update the description of this patch as well?
It's still not fully clear to me what problem this patch is aiming at resolving.

[lxfind]

Also, it seems that the newly added test "coro-debug-dbg.values-O2-nouse.ll" passes even without this patch

[ChuanqiXu]

In D97673#2693328, @lxfind wrote:

Also, it seems that the newly added test "coro-debug-dbg.values-O2-nouse.ll" passes even without this patch

It is intended behavior. This patch tries to test whether we would collect dbg.values whose value isn't in the coroutine frame. This test can't pass for previous revision. If someone in the future wants to enhance the debugbility for coroutine frame, this test would tell him that we should think more to change the layout of the coroutine frame for debugability.

[ChuanqiXu]

In D97673#2693308, @lxfind wrote:

Could you update the description of this patch as well?
It's still not fully clear to me what problem this patch is aiming at resolving.

Previously all dbg.values would be lost after CoroSplit pass. dbg.value is debug info compiler produced under optimization. This patch wants to remain as many dbg.value as possible while it don't want to change the layout of frame for debug info.

[lxfind]

Previously all dbg.values would be lost after CoroSplit pass. dbg.value is debug info compiler produced under optimization. This patch wants to remain as many dbg.value as possible while it don't want to change the layout of frame for debug info.

Why would all dbg.values be lost after CoroSplit pass? As long as they are in the resume code path, they would be kept in the .resume function, right?
I also tried to run coro-split on coro-debug-dbg.values-O2.ll, I do see a ton of dbg.value in the generate functions even without this patch.
Could you elaborate what is the exact problem, perhaps ideally with an example?

[ChuanqiXu]

In D97673#2697366, @lxfind wrote:

Previously all dbg.values would be lost after CoroSplit pass. dbg.value is debug info compiler produced under optimization. This patch wants to remain as many dbg.value as possible while it don't want to change the layout of frame for debug info.

Why would all dbg.values be lost after CoroSplit pass? As long as they are in the resume code path, they would be kept in the .resume function, right?
I also tried to run coro-split on coro-debug-dbg.values-O2.ll, I do see a ton of dbg.value in the generate functions even without this patch.
Could you elaborate what is the exact problem, perhaps ideally with an example?

The problem here is when we collect values to be put on the frame and insert values to the frame, we didn't care about the dbg.values.
Here is the example:

define void @f(i32 %i, i32 %j) {
; ...
; coro.suspend
call void @llvm.dbg.value(metadata i32 %i, metadata !1, metadata !DIExpression()) ; tell the value !1 from %i
call void @llvm.dbg.value(metadata i32 %j, metadata !2, metadata !DIExpression()) ; tell the value !2 from %j
}

Then in the .resume function:

define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr)
; ...
call void @llvm.dbg.value(metadata i32 %i, metadata !1, metadata !DIExpression())
call void @llvm.dbg.value(metadata i32 %j, metadata !2, metadata !DIExpression())

These two dbg.values are trying to tell values of !1 and !2 from values %i and %j. But wait, there is no definition for %I and %j in the resume function. So finally, the resume function would become:

define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr)
; ...
call void @llvm.dbg.value(metadata i32 undef, metadata !1, metadata !DIExpression())
call void @llvm.dbg.value(metadata i32 undef, metadata !2, metadata !DIExpression())

This patch wants to salvage the dbg.values whose first operand is in the frame already. The reason why you can find some dbg.values in resume function is that their first operand is either constant or defined in the resume function (like alloca who wouldn't across suspend points).

[lxfind]

In D97673#2700185, @ChuanqiXu wrote:

In D97673#2697366, @lxfind wrote:

Previously all dbg.values would be lost after CoroSplit pass. dbg.value is debug info compiler produced under optimization. This patch wants to remain as many dbg.value as possible while it don't want to change the layout of frame for debug info.

Thanks for explaining. It makes sense.
Does it only happen to parameters, or does it also happen to allocas? (I would be more interested to see how this affects allocas, since parameters are somewhat special)
And how is this related to O2?

[ChuanqiXu]

In D97673#2703656, @lxfind wrote:

In D97673#2700185, @ChuanqiXu wrote:

In D97673#2697366, @lxfind wrote:

Previously all dbg.values would be lost after CoroSplit pass. dbg.value is debug info compiler produced under optimization. This patch wants to remain as many dbg.value as possible while it don't want to change the layout of frame for debug info.

Thanks for explaining. It makes sense.
Does it only happen to parameters, or does it also happen to allocas? (I would be more interested to see how this affects allocas, since parameters are somewhat special)
And how is this related to O2?

It happens to all the value possible in the frame including parameters, allocas and other values possible (if any). Since we only collect values who are already in the frame, this patch shouldn't affect any thing.

And how is this related to O2?

Since dbg.value would be generated with optimization enabled. If optimization isn't enabled, compiler would generates dbg.declare as the debug info, which is already handled by previous patches. It is confusing to use O2 here, my bad. I would edit it.

[ChuanqiXu]

Address the comments. Removing confusing describes about O2.

[aprantl]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

1636

// Also update metadata uses in dbg.value intrinsics.

2584

// Manually add dbg.value metadata uses of I.

[aprantl]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

172

We should either handle unhandled cases gracefully (by ignoring them) or make sure they are impossible and put an llvm_unreachable here.

[lxfind]

The test is insufficient. I would like to see that we cover dbg.value used on different cases: parameters, allocas, spills.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

1638

Why doesn't replaceUsesOfWith work? Could you double check that replaceUsesOfWith doesn't work for DbgValueInst?

1694–1699

If replaceUsesOfWith works for DbgValueInst, then I think you can just append all DIs into UsersToUpdate above.

1943

Similarly, if replaceUsesOfWith works for DbgValueInst, this isn't necessary

2664–2676

I think it's cleaner to move this code into the loop above.
After the check of all users, you can check if FrameData.Spills contains &I, and if so, do this DbgValues thing.

[aprantl]

@lxfind wrote:

Why doesn't replaceUsesOfWith work? Could you double check that replaceUsesOfWith doesn't work for DbgValueInst?

This is by design. Metadata-uses of llvm:Value may not affect code generation, thus they also aren't found by any operation working on llvm::Uses.

[lxfind]

In D97673#2706610, @aprantl wrote:

@lxfind wrote:

Why doesn't replaceUsesOfWith work? Could you double check that replaceUsesOfWith doesn't work for DbgValueInst?

This is by design. Metadata-uses of llvm:Value may not affect code generation, thus they also aren't found by any operation working on llvm::Uses.

I don't think so.
Take a look at this: https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/User.cpp#L34-L37
I think replaceUsesOfWith works for DbgValueInst.

It's true though that .users() does not include DbgValueInst, but that's a separate question than whether replaceUsesOfWith works.

[ChuanqiXu]

Address the comments.

[ChuanqiXu]

In D97673#2706798, @lxfind wrote:

In D97673#2706610, @aprantl wrote:

@lxfind wrote:

Why doesn't replaceUsesOfWith work? Could you double check that replaceUsesOfWith doesn't work for DbgValueInst?

This is by design. Metadata-uses of llvm:Value may not affect code generation, thus they also aren't found by any operation working on llvm::Uses.

I don't think so.
Take a look at this: https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/User.cpp#L34-L37
I think replaceUsesOfWith works for DbgValueInst.

It's true though that .users() does not include DbgValueInst, but that's a separate question than whether replaceUsesOfWith works.

Good Catcha. It looks like you are right. I have thought DbgVariablesInst uses different def-use architecture since users() didn't count DbgValueInst. I should have looked into the details.

[lxfind]

Also please add tests to cover more types of variables (allocas, spills..)

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2682–2690

I think it's cleaner to move this code into the loop above.
After the check of all users, you can check if FrameData.Spills contains &I, and if so, do this DbgValues thing.

[ChuanqiXu]

Address the comments:

• Add test case for alloca and spills.

[ChuanqiXu]

Address the comments.

[ChuanqiXu]

In D97673#2707257, @lxfind wrote:

Also please add tests to cover more types of variables (allocas, spills..)

Sorry for forgetting addressing the inline comments.

[lxfind]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2665

This might not work. The iteration of instructions is not guaranteed to be in dominator order.
This part of the code should be:

if (FrameData.Spills.count(&I)) {
  SmallVector<DbgValueInst *, 16> DVIs;
  findDbgValues(DVIs, &I);
  ...
}

[lxfind]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

1686–1691

I don't know enough about this, but I wonder if these code (handling of dbgdeclare and dbgvalues) could be merged by simply traversing thorough all DVIs in findDbgUsers and do a replaceUsesOfWith on each of them?

[ChuanqiXu]

Address the comments.

[ChuanqiXu]

Address the comments.

[ChuanqiXu]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2665

Sorry for not considering the dominator order. The example in the inline comment don't cover the Argument. I prefer to move these codes out of the loop instead of add it here and in the loop traversing arguments. I think it may not be much confusing if we move the codes out of the current loop.

[lxfind]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

1668–1669

Comments need to be updated

[ChuanqiXu]

Address the comments.

[ChuanqiXu]

Address the comments.

[lxfind]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2667–2670

This is basically iterating over FrameData.Spills and obtain .first on the iterator

[ChuanqiXu]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2667–2670

Yes, I am just wondering if the code now makes the semantics more clear. If we write

for (auto *Iter : FrameData.Spills) {
     auto *V = Iter.first;
     // ...
}

we need to comment that we would handle alloca specially. And the style tells it in the level of code.

[lxfind]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2667–2670

Well for one, we process Spills and Allocas separately everywhere (that's the intention of separating them as two fields anyway). And another reason is getAllDefs() reconstruct a new vector to hold all elements, which is costly.

[ChuanqiXu]

Address the comments.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2667–2670

Done, your suggestion makes sense.

[lxfind]

LGTM

[ChuanqiXu]

@aprantl gentle ping~

[ChuanqiXu]

@aprantl gentle ping~

[aprantl]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

1669

gdb -> dbg (2x)

[hoy]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2515

I've seen this caused issue when Storage is a block terminator, e.g, an invoke instruction returning a piece of allocated memory. I don't quite get the full context of how we are salvaging debug info, but do you think we can just skip such case here?

[StephenTozer]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2515

Not my patch, but do you have a reproducer for this? I'm not sure what circumstances are causing Storage to be a block terminator, but it doesn't sound like something that should happen.

[hoy]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2515

I've been trying to get a small repro with llvm-reduce which has already run 12 hours on our internal giant functions resulted by a combination of AutoFDO and aggressive inlining. The block terminator I'm seeing is an invoke instruction calling new and the code here result in the following pattern

%3516 = invoke noalias nonnull i8* @_Znwm(i64 120) #79 to label %3517 unwind label %3585, !dbg !332197 
call void @llvm.dbg.declare(metadata i8* %3516, metadata !332198, metadata !DIExpression(DW_OP_plus_uconst, 16, DW_OP_plus_uconst, 56)), !dbg !332217 
call void @llvm.dbg.declare(metadata i8* %3516, metadata !332278, metadata !DIExpression(DW_OP_plus_uconst, 16, DW_OP_plus_uconst, 8)), !dbg !332298

[hoy]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2515

So I managed to get a repro case that crashed the compiler: https://reviews.llvm.org/D114994 . Unfortunately it is still not small though with a single function only.

[ChuanqiXu]

@hoy The rationale here is that the dbg.declare intrinsics might be lost due to coroutine split. So we move the dbg.declare intrinsics to the place where the Storage is just created. So if it caused crash, I think we could skip the kind of case. Or it would be better to insert dbg.declare to the start of successor blocks in the case of invoke. (Would any other terminator return a value?)
BTW, there is nothing in your repro link.

[hoy]

In D97673#3168709, @ChuanqiXu wrote:

@hoy The rationale here is that the dbg.declare intrinsics might be lost due to coroutine split. So we move the dbg.declare intrinsics to the place where the Storage is just created. So if it caused crash, I think we could skip the kind of case. Or it would be better to insert dbg.declare to the start of successor blocks in the case of invoke. (Would any other terminator return a value?)
BTW, there is nothing in your repro link.

An invoke instruction is a call instruction that can throw exceptions. It is a block terminator and can return a value. Looks like dbg.declare can only appear once so maybe we could only move it to one of the successors, or just skip this case.

There is a test file in my repro link. The file is probably too large (40MB) to be rendered. Sorry for not being able to get a smaller repro case.

[ChuanqiXu]

In D97673#3168737, @hoy wrote:

In D97673#3168709, @ChuanqiXu wrote:

@hoy The rationale here is that the dbg.declare intrinsics might be lost due to coroutine split. So we move the dbg.declare intrinsics to the place where the Storage is just created. So if it caused crash, I think we could skip the kind of case. Or it would be better to insert dbg.declare to the start of successor blocks in the case of invoke. (Would any other terminator return a value?)
BTW, there is nothing in your repro link.

An invoke instruction is a call instruction that can throw exceptions. It is a block terminator and can return a value. Looks like dbg.declare can only appear once so maybe we could only move it to one of the successors, or just skip this case.

There is a test file in my repro link. The file is probably too large (40MB) to be rendered. Sorry for not being able to get a smaller repro case.

Got it. I would try to fix this. Thanks for reporting this.

[hoy]

In D97673#3168739, @ChuanqiXu wrote:

In D97673#3168737, @hoy wrote:

In D97673#3168709, @ChuanqiXu wrote:

@hoy The rationale here is that the dbg.declare intrinsics might be lost due to coroutine split. So we move the dbg.declare intrinsics to the place where the Storage is just created. So if it caused crash, I think we could skip the kind of case. Or it would be better to insert dbg.declare to the start of successor blocks in the case of invoke. (Would any other terminator return a value?)
BTW, there is nothing in your repro link.

An invoke instruction is a call instruction that can throw exceptions. It is a block terminator and can return a value. Looks like dbg.declare can only appear once so maybe we could only move it to one of the successors, or just skip this case.

There is a test file in my repro link. The file is probably too large (40MB) to be rendered. Sorry for not being able to get a smaller repro case.

Got it. I would try to fix this. Thanks for reporting this.

Thanks. I'm pasting the problematic code snippet in case my link doesn't work for you, but please let me know so that I'll find another way to share with you.

%660 = invoke noalias nonnull i8* @_Znwm(i64 120) #20
          to label %.noexc unwind label %702, !dbg !304801

.noexc:                                           ; preds = %653
  call void @llvm.dbg.value(metadata %"class.std::allocator.6883"* undef, metadata !304802, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !304803
  call void @llvm.dbg.value(metadata i8* %660, metadata !304802, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !304803
  call void @llvm.dbg.value(metadata i8* %660, metadata !304804, metadata !DIExpression()), !dbg !304803
  call void @llvm.dbg.value(metadata i8* %660, metadata !303143, metadata !DIExpression()), !dbg !304805
  %661 = bitcast i8* %660 to %"class.std::_Sp_counted_base"*, !dbg !304808
  call void @_ZNSt16_Sp_counted_baseILN9__gnu_cxx12_Lock_policyE2EEC2Ev(%"class.std::_Sp_counted_base"* nonnull dereferenceable(16) %661) #15, !dbg !304809
  %662 = bitcast i8* %660 to i32 (...)***, !dbg !304808
  store i32 (...)** bitcast (i8** getelementptr inbounds ({ [7 x i8*] }, { [7 x i8*] }* @_ZTVSt23_Sp_counted_ptr_inplaceIN7scribex17ScribeReadFlavourESaIS1_ELN9__gnu_cxx12_Lock_policyE2EE, i64 0, inrange i32 0, i64 2) to i32 (...)**), i32 (...)*** %662, align 8, !dbg !304808, !tbaa !302581
  call void @llvm.dbg.value(metadata i8* %660, metadata !303156, metadata !DIExpression()), !dbg !304810
  ....
  call void @llvm.dbg.declare(metadata i8* %660, metadata !303236, metadata !DIExpression(DW_OP_plus_uconst, 16, DW_OP_plus_uconst, 8)), !dbg !304825

The last dbg.declare will be moved to right after the first invoke instruction.

[ChuanqiXu]

@hoy Hi, the problem should be addressed within 84980761a.

[hoy]

In D97673#3168934, @ChuanqiXu wrote:

@hoy Hi, the problem should be addressed within 84980761a.

Thanks for the quick turnaround! I have verified that it fixes our build break.

[StephenTozer]

It's a relatively obscure instruction I think, but we should also handle callbr in the same manner as invoke, as that is also a terminator instruction that defines an SSA value. A similar solution should suffice, as it also has a single destination where the value is expected to be valid (getDefaultDest()) that the dbg.declare can be inserted at the start of.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

 
 static cl::opt<bool> EnableReuseStorageInFrame(
     "reuse-storage-in-coroutine-frame", cl::Hidden,
     cl::desc(
         "Enable the optimization which would reuse the storage in the coroutine \
          frame for allocas whose liferanges are not overlapped, for testing purposes"),
     llvm::cl::init(false));
 
 enum { SmallVectorThreshold = 32 };
 
 // Provides two way mapping between the blocks and numbers.
 namespace {
 class BlockToIndexMapping {
   SmallVector<BasicBlock *, SmallVectorThreshold> V;
 
 public:
   size_t size() const { return V.size(); }
 
   BlockToIndexMapping(Function &F) {
bool isDefinitionAcrossSuspend(Instruction &I, User *U) const {
     // conceptually occur after the suspend.
     if (isa<AnyCoroSuspendInst>(I)) {
       DefBB = DefBB->getSingleSuccessor();
       assert(DefBB && "should have split coro.suspend into its own block");
     }
 
     return isDefinitionAcrossSuspend(DefBB, U);
   }
+
+  bool isDefinitionAcrossSuspend(Value &V, User *U) const {
+    if (auto *Arg = dyn_cast<Argument>(&V))
+      return isDefinitionAcrossSuspend(*Arg, U);
+    if (auto *Inst = dyn_cast<Instruction>(&V))
+      return isDefinitionAcrossSuspend(*Inst, U);
+
+    llvm_unreachable(
+        "Coroutine could only collect Argument and Instruction now.");
+  }
 };
 } // end anonymous namespace
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(StringRef Label,
                                                 BitVector const &BV) const {
   dbgs() << Label << ":";
   for (size_t I = 0, N = BV.size(); I < N; ++I)
for (auto *U : E.second) {
         assert(PN->getNumIncomingValues() == 1 &&
                "unexpected number of incoming "
                "values in the PHINode");
         PN->replaceAllUsesWith(CurrentReload);
         PN->eraseFromParent();
         continue;
       }
 
       // Replace all uses of CurrentValue in the current instruction with
       // reload.
       U->replaceUsesOfWith(Def, CurrentReload);
     }
   }
 
   BasicBlock *FramePtrBB = FramePtr->getParent();
 
   auto SpillBlock =
       FramePtrBB->splitBasicBlock(FramePtr->getNextNode(), "AllocaSpillBB");
   SpillBlock->splitBasicBlock(&SpillBlock->front(), "PostSpill");
for (const auto &P : FrameData.Allocas) {
       G->takeName(Alloca);
       Alloca->replaceAllUsesWith(G);
       Alloca->eraseFromParent();
     }
     return FramePtr;
   }
 
   // If we found any alloca, replace all of their remaining uses with GEP
-  // instructions. Because new dbg.declare have been created for these alloca,
-  // we also delete the original dbg.declare and replace other uses with undef.
+  // instructions. To remain debugbility, we replace the uses of allocas for
+  // dbg.declares and dbg.values with the reload from the frame.
   // Note: We cannot replace the alloca with GEP instructions indiscriminately,
   // as some of the uses may not be dominated by CoroBegin.
   Builder.SetInsertPoint(&Shape.AllocaSpillBlock->front());
   SmallVector<Instruction *, 4> UsersToUpdate;
   for (const auto &A : FrameData.Allocas) {
     AllocaInst *Alloca = A.Alloca;
     UsersToUpdate.clear();
     for (User *U : Alloca->users()) {
       auto *I = cast<Instruction>(U);
       if (DT.dominates(CB, I))
         UsersToUpdate.push_back(I);
     }
     if (UsersToUpdate.empty())
       continue;
     auto *G = GetFramePointer(Alloca);
     G->setName(Alloca->getName() + Twine(".reload.addr"));
 
-    TinyPtrVector<DbgDeclareInst *> DIs = FindDbgDeclareUses(Alloca);
-    if (!DIs.empty())
-      DIBuilder(*Alloca->getModule(),
-                /*AllowUnresolved*/ false)
-          .insertDeclare(G, DIs.front()->getVariable(),
-                         DIs.front()->getExpression(),
-                         DIs.front()->getDebugLoc(), DIs.front());
-    for (auto *DI : FindDbgDeclareUses(Alloca))
-      DI->eraseFromParent();
-    replaceDbgUsesWithUndef(Alloca);
+    SmallVector<DbgVariableIntrinsic *, 4> DIs;
+    findDbgUsers(DIs, Alloca);
+    for (auto *DVI : DIs)
+      DVI->replaceUsesOfWith(Alloca, G);
 
     for (Instruction *I : UsersToUpdate)
       I->replaceUsesOfWith(Alloca, G);
   }
   Builder.SetInsertPoint(FramePtr->getNextNode());
   for (const auto &A : FrameData.Allocas) {
     AllocaInst *Alloca = A.Alloca;
     if (A.MayWriteBeforeCoroBegin) {
       // isEscaped really means potentially modified before CoroBegin.
       if (Alloca->isArrayAllocation())
         report_fatal_error(
             "Coroutines cannot handle copying of array allocas yet");
 
       auto *G = GetFramePointer(Alloca);
       auto *Value = Builder.CreateLoad(Alloca->getAllocatedType(), Alloca);
       Builder.CreateStore(Value, G);
     }
for (Instruction *U : E.second) {
       if (auto *PN = dyn_cast<PHINode>(U)) {
         assert(PN->getNumIncomingValues() == 1 &&
                "unexpected number of incoming "
                "values in the PHINode");
         PN->replaceAllUsesWith(CurrentMaterialization);
         PN->eraseFromParent();
         continue;
       }
       // Replace all uses of Def in the current instruction with the
       // CurrentMaterialization for the block.
       U->replaceUsesOfWith(Def, CurrentMaterialization);
     }
   }
 }
 
 // Splits the block at a particular instruction unless it is the first
 // instruction in the block with a single predecessor.
if (!Visitor.getShouldLiveOnFrame())
       continue;
     Allocas.emplace_back(AI, Visitor.getAliasesCopy(),
                          Visitor.getMayWriteBeforeCoroBegin());
   }
 }
 
 void coro::salvageDebugInfo(
     SmallDenseMap<llvm::Value *, llvm::AllocaInst *, 4> &DbgPtrAllocaCache,
-    DbgDeclareInst *DDI, bool ReuseFrameSlot) {
-  Function *F = DDI->getFunction();
+    DbgVariableIntrinsic *DVI, bool ReuseFrameSlot) {
+  Function *F = DVI->getFunction();
   IRBuilder<> Builder(F->getContext());
   auto InsertPt = F->getEntryBlock().getFirstInsertionPt();
   while (isa<IntrinsicInst>(InsertPt))
     ++InsertPt;
   Builder.SetInsertPoint(&F->getEntryBlock(), InsertPt);
-  DIExpression *Expr = DDI->getExpression();
+  DIExpression *Expr = DVI->getExpression();
   // Follow the pointer arithmetic all the way to the incoming
   // function argument and convert into a DIExpression.
   bool OutermostLoad = true;
-  Value *Storage = DDI->getAddress();
+  Value *Storage = DVI->getVariableLocationOp(0);
   Value *OriginalStorage = Storage;
   while (Storage) {
     if (auto *LdInst = dyn_cast<LoadInst>(Storage)) {
       Storage = LdInst->getOperand(0);
       // FIXME: This is a heuristic that works around the fact that
       // LLVM IR debug intrinsics cannot yet distinguish between
       // memory and value locations: Because a dbg.declare(alloca) is
       // implicitly a memory location no DW_OP_deref operation for the
if (auto *Arg = dyn_cast<llvm::Argument>(Storage)) {
       // turn a dbg.declare(alloca, ..., DIExpression()) into a memory
       // location. Thus, if there are deref and offset operations in the
       // expression, we need to add a DW_OP_deref at the *start* of the
       // expression to first load the contents of the alloca before
       // adjusting it with the expression.
       if (Expr && Expr->isComplex())
         Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
     }
-  DDI->replaceVariableLocationOp(OriginalStorage, Storage);
-  DDI->setExpression(Expr);
+
+  DVI->replaceVariableLocationOp(OriginalStorage, Storage);
+  DVI->setExpression(Expr);
+  /// It makes no sense to move the dbg.value intrinsic.
+  if (!isa<DbgValueInst>(DVI)) {
     if (auto *InsertPt = dyn_cast<Instruction>(Storage))
-    DDI->moveAfter(InsertPt);
+      DVI->moveAfter(InsertPt);
     else if (isa<Argument>(Storage))
-    DDI->moveAfter(F->getEntryBlock().getFirstNonPHI());
+      DVI->moveAfter(F->getEntryBlock().getFirstNonPHI());
+  }
 }
 
 void coro::buildCoroutineFrame(Function &F, Shape &Shape) {
   // Don't eliminate swifterror in async functions that won't be split.
   if (Shape.ABI != coro::ABI::Async || !Shape.CoroSuspends.empty())
     eliminateSwiftError(F, Shape);
 
   if (Shape.ABI == coro::ABI::Switch &&
void coro::buildCoroutineFrame(Function &F, Shape &Shape) {
   SmallVector<CoroAllocaAllocInst*, 4> LocalAllocas;
   SmallVector<Instruction*, 4> DeadInstructions;
 
   {
     SpillInfo Spills;
     for (int Repeat = 0; Repeat < 4; ++Repeat) {
       // See if there are materializable instructions across suspend points.
       for (Instruction &I : instructions(F))
-        if (materializable(I))
+        if (materializable(I)) {
           for (User *U : I.users())
             if (Checker.isDefinitionAcrossSuspend(I, U))
               Spills[&I].push_back(cast<Instruction>(U));
 
+          // Manually add dbg.value metadata uses of I.
+          SmallVector<DbgValueInst *, 16> DVIs;
+          findDbgValues(DVIs, &I);
+          for (auto *DVI : DVIs)
+            if (Checker.isDefinitionAcrossSuspend(I, DVI))
+              Spills[&I].push_back(DVI);
+        }
+
       if (Spills.empty())
         break;
 
       // Rewrite materializable instructions to be materialized at the use
       // point.
       LLVM_DEBUG(dumpSpills("Materializations", Spills));
       rewriteMaterializableInstructions(Builder, Spills);
       Spills.clear();
for (User *U : A.users())
       if (Checker.isDefinitionAcrossSuspend(A, U))
         FrameData.Spills[&A].push_back(cast<Instruction>(U));
 
   for (Instruction &I : instructions(F)) {
     // Values returned from coroutine structure intrinsics should not be part
     // of the Coroutine Frame.
     if (isCoroutineStructureIntrinsic(I) || &I == Shape.CoroBegin)
       continue;
 
     // The Coroutine Promise always included into coroutine frame, no need to
     // check for suspend crossing.
     if (Shape.ABI == coro::ABI::Switch &&
         Shape.SwitchLowering.PromiseAlloca == &I)
       continue;
 
     // Handle alloca.alloc specially here.
     if (auto AI = dyn_cast<CoroAllocaAllocInst>(&I)) {
for (Instruction &I : instructions(F)) {
 
     // Ignore alloca.get; we process this as part of coro.alloca.alloc.
     if (isa<CoroAllocaGetInst>(I))
       continue;
 
     if (isa<AllocaInst>(I))
       continue;
 
     for (User *U : I.users())
       if (Checker.isDefinitionAcrossSuspend(I, U)) {
         // We cannot spill a token.
         if (I.getType()->isTokenTy())
           report_fatal_error(
               "token definition is separated from the use by a suspend point");
         FrameData.Spills[&I].push_back(cast<Instruction>(U));
       }
   }
+
+  // We don't want the layout of coroutine frame to be affected
+  // by debug information. So we only choose to salvage DbgValueInst for
+  // whose value is already in the frame.
+  // We would handle the dbg.values for allocas specially
+  for (auto &Iter : FrameData.Spills) {
+    auto *V = Iter.first;
+    SmallVector<DbgValueInst *, 16> DVIs;
+    findDbgValues(DVIs, V);
+    llvm::for_each(DVIs, [&](DbgValueInst *DVI) {
+      if (Checker.isDefinitionAcrossSuspend(*V, DVI))
+        FrameData.Spills[V].push_back(DVI);
+    });
+  }
+
   LLVM_DEBUG(dumpSpills("Spills", FrameData.Spills));
   if (Shape.ABI == coro::ABI::Retcon || Shape.ABI == coro::ABI::RetconOnce ||
       Shape.ABI == coro::ABI::Async)
     sinkSpillUsesAfterCoroBegin(F, FrameData, Shape.CoroBegin);
   Shape.FrameTy = buildFrameType(F, Shape, FrameData);
   createFramePtr(Shape);
   // For now, this works for C++ programs only.
   buildFrameDebugInfo(F, Shape, FrameData);
   insertSpills(FrameData, Shape);
   lowerLocalAllocas(LocalAllocas, DeadInstructions);
 
   for (auto I : DeadInstructions)
     I->eraseFromParent();
 }

llvm/lib/Transforms/Coroutines/CoroInternal.h

bool declaresIntrinsics(const Module &M,
                         const std::initializer_list<StringRef>);
 void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
 void updateCallGraph(Function &Caller, ArrayRef<Function *> Funcs,
                      CallGraph &CG, CallGraphSCC &SCC);
 /// Recover a dbg.declare prepared by the frontend and emit an alloca
 /// holding a pointer to the coroutine frame.
 void salvageDebugInfo(
     SmallDenseMap<llvm::Value *, llvm::AllocaInst *, 4> &DbgPtrAllocaCache,
-    DbgDeclareInst *DDI, bool ReuseFrameSlot);
+    DbgVariableIntrinsic *DVI, bool ReuseFrameSlot);
 
 // Keeps data and helper functions for lowering coroutine intrinsics.
 struct LowererBase {
   Module &TheModule;
   LLVMContext &Context;
   PointerType *const Int8Ptr;
   FunctionType *const ResumeFnType;
   ConstantPointerNull *const NullPtr;

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape,
   }
 }
 
 void CoroCloner::replaceSwiftErrorOps() {
   ::replaceSwiftErrorOps(*NewF, Shape, &VMap);
 }
 
 void CoroCloner::salvageDebugInfo() {
-  SmallVector<DbgDeclareInst *, 8> Worklist;
+  SmallVector<DbgVariableIntrinsic *, 8> Worklist;
   SmallDenseMap<llvm::Value *, llvm::AllocaInst *, 4> DbgPtrAllocaCache;
   for (auto &BB : *NewF)
     for (auto &I : BB)
-      if (auto *DDI = dyn_cast<DbgDeclareInst>(&I))
-        Worklist.push_back(DDI);
-  for (DbgDeclareInst *DDI : Worklist)
-    coro::salvageDebugInfo(DbgPtrAllocaCache, DDI, Shape.ReuseFrameSlot);
+      if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
+        Worklist.push_back(DVI);
+  for (DbgVariableIntrinsic *DVI : Worklist)
+    coro::salvageDebugInfo(DbgPtrAllocaCache, DVI, Shape.ReuseFrameSlot);
 
   // Remove all salvaged dbg.declare intrinsics that became
   // either unreachable or stale due to the CoroSplit transformation.
   auto IsUnreachableBlock = [&](BasicBlock *BB) {
     return BB->hasNPredecessors(0) && BB != &NewF->getEntryBlock();
   };
-  for (DbgDeclareInst *DDI : Worklist) {
-    if (IsUnreachableBlock(DDI->getParent()))
-      DDI->eraseFromParent();
-    else if (dyn_cast_or_null<AllocaInst>(DDI->getAddress())) {
+  for (DbgVariableIntrinsic *DVI : Worklist) {
+    if (IsUnreachableBlock(DVI->getParent()))
+      DVI->eraseFromParent();
+    else if (dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0))) {
       // Count all non-debuginfo uses in reachable blocks.
       unsigned Uses = 0;
-      for (auto *User : DDI->getAddress()->users())
+      for (auto *User : DVI->getVariableLocationOp(0)->users())
         if (auto *I = dyn_cast<Instruction>(User))
           if (!isa<AllocaInst>(I) && !IsUnreachableBlock(I->getParent()))
             ++Uses;
       if (!Uses)
-        DDI->eraseFromParent();
+        DVI->eraseFromParent();
     }
   }
 }
 
 void CoroCloner::replaceEntryBlock() {
   // In the original function, the AllocaSpillBlock is a block immediately
   // following the allocation of the frame object which defines GEPs for
   // all the allocas that have been moved into the frame, and it ends by

llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll

+; Tests whether resume function would remain dbg.value infomation if corresponding values are not used in the frame.
+; RUN: opt < %s -coro-early -coro-split -coro-split -S | FileCheck %s
+;
+; This file is based on coro-debug-frame-variable.ll.
+; CHECK:  define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK:       await.ready:
+; CHECK:         call void @llvm.dbg.value(metadata i32 undef, metadata ![[IVAR_RESUME:[0-9]+]], metadata !DIExpression(
+; CHECK:         call void @llvm.dbg.value(metadata i32 undef, metadata ![[JVAR_RESUME:[0-9]+]], metadata !DIExpression(
+;
+; CHECK: ![[RESUME_FN_DBG_NUM]] = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov"
+; CHECK: ![[IVAR_RESUME]] = !DILocalVariable(name: "i"
+; CHECK: ![[JVAR_RESUME]] = !DILocalVariable(name: "j"
+
+source_filename = "../llvm/test/Transforms/Coroutines/coro-debug-dbg.values-O2.ll"
+
+define void @f(i32 %i, i32 %j) !dbg !8 {
+entry:
+  %__promise = alloca i8, align 8
+  %x = alloca [10 x i32], align 16
+  %id = call token @llvm.coro.id(i32 16, i8* %__promise, i8* null, i8* null)
+  %alloc = call i1 @llvm.coro.alloc(token %id)
+  br i1 %alloc, label %coro.alloc, label %coro.init
+
+coro.alloc:                                       ; preds = %entry
+  %size = call i64 @llvm.coro.size.i64()
+  %memory = call i8* @new(i64 %size)
+  br label %coro.init
+
+coro.init:                                        ; preds = %coro.alloc, %entry
+  %phi.entry.alloc = phi i8* [ null, %entry ], [ %memory, %coro.alloc ]
+  %begin = call i8* @llvm.coro.begin(token %id, i8* %phi.entry.alloc)
+  %ready = call i1 @await_ready()
+  br i1 %ready, label %init.ready, label %init.suspend
+
+init.suspend:                                     ; preds = %coro.init
+  %save = call token @llvm.coro.save(i8* null)
+  call void @await_suspend()
+  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
+  switch i8 %suspend, label %coro.ret [
+    i8 0, label %init.ready
+    i8 1, label %init.cleanup
+  ]
+
+init.cleanup:                                     ; preds = %init.suspend
+  br label %cleanup
+
+init.ready:                                       ; preds = %init.suspend, %coro.init
+  call void @await_resume()
+  call void @llvm.dbg.value(metadata i32 0, metadata !6, metadata !DIExpression()), !dbg !11
+  %i.init.ready.inc = add nsw i32 0, 1
+  call void @llvm.dbg.value(metadata i32 %i.init.ready.inc, metadata !6, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.declare(metadata [10 x i32]* %x, metadata !12, metadata !DIExpression()), !dbg !17
+  %memset = bitcast [10 x i32]* %x to i8*, !dbg !17
+  call void @llvm.memset.p0i8.i64(i8* align 16 %memset, i8 0, i64 40, i1 false), !dbg !17
+  call void @print(i32 %i.init.ready.inc)
+  %ready.again = call zeroext i1 @await_ready()
+  br i1 %ready.again, label %await.ready, label %await.suspend
+
+await.suspend:                                    ; preds = %init.ready
+  %save.again = call token @llvm.coro.save(i8* null)
+  %from.address = call i8* @from_address(i8* %begin)
+  call void @await_suspend()
+  %suspend.again = call i8 @llvm.coro.suspend(token %save.again, i1 false)
+  switch i8 %suspend.again, label %coro.ret [
+    i8 0, label %await.ready
+    i8 1, label %await.cleanup
+  ]
+
+await.cleanup:                                    ; preds = %await.suspend
+  br label %cleanup
+
+await.ready:                                      ; preds = %await.suspend, %init.ready
+  call void @await_resume()
+  call void @llvm.dbg.value(metadata i32 0, metadata !18, metadata !DIExpression()), !dbg !11
+  %arrayidx0 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 0, !dbg !19
+  store i32 1, i32* %arrayidx0, align 16, !dbg !20
+  %arrayidx1 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 1, !dbg !21
+  store i32 2, i32* %arrayidx1, align 4, !dbg !22
+  %i.await.ready.inc = add nsw i32 %i.init.ready.inc, 1
+  call void @llvm.dbg.value(metadata i32 %i, metadata !6, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.value(metadata i32 %j, metadata !18, metadata !DIExpression()), !dbg !11
+  call void @print(i32 %i.await.ready.inc)
+  call void @return_void()
+  br label %coro.final
+
+coro.final:                                       ; preds = %await.ready
+  call void @final_suspend()
+  %coro.final.await_ready = call i1 @await_ready()
+  br i1 %coro.final.await_ready, label %final.ready, label %final.suspend
+
+final.suspend:                                    ; preds = %coro.final
+  %final.suspend.coro.save = call token @llvm.coro.save(i8* null)
+  %final.suspend.from_address = call i8* @from_address(i8* %begin)
+  call void @await_suspend()
+  %final.suspend.coro.suspend = call i8 @llvm.coro.suspend(token %final.suspend.coro.save, i1 true)
+  switch i8 %final.suspend.coro.suspend, label %coro.ret [
+    i8 0, label %final.ready
+    i8 1, label %final.cleanup
+  ]
+
+final.cleanup:                                    ; preds = %final.suspend
+  br label %cleanup
+
+final.ready:                                      ; preds = %final.suspend, %coro.final
+  call void @await_resume()
+  br label %cleanup
+
+cleanup:                                          ; preds = %final.ready, %final.cleanup, %await.cleanup, %init.cleanup
+  %cleanup.dest.slot.0 = phi i32 [ 0, %final.ready ], [ 2, %final.cleanup ], [ 2, %await.cleanup ], [ 2, %init.cleanup ]
+  %free.memory = call i8* @llvm.coro.free(token %id, i8* %begin)
+  %free = icmp ne i8* %free.memory, null
+  br i1 %free, label %coro.free, label %after.coro.free
+
+coro.free:                                        ; preds = %cleanup
+  call void @delete(i8* %free.memory)
+  br label %after.coro.free
+
+after.coro.free:                                  ; preds = %coro.free, %cleanup
+  switch i32 %cleanup.dest.slot.0, label %unreachable [
+    i32 0, label %cleanup.cont
+    i32 2, label %coro.ret
+  ]
+
+cleanup.cont:                                     ; preds = %after.coro.free
+  br label %coro.ret
+
+coro.ret:                                         ; preds = %cleanup.cont, %after.coro.free, %final.suspend, %await.suspend, %init.suspend
+  %end = call i1 @llvm.coro.end(i8* null, i1 false)
+  ret void
+
+unreachable:                                      ; preds = %after.coro.free
+  unreachable
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+; Function Attrs: argmemonly nounwind readonly
+declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*) #1
+
+; Function Attrs: nounwind
+declare i1 @llvm.coro.alloc(token) #2
+
+; Function Attrs: nounwind readnone
+declare i64 @llvm.coro.size.i64() #3
+
+; Function Attrs: nounwind
+declare token @llvm.coro.save(i8*) #2
+
+; Function Attrs: nounwind
+declare i8* @llvm.coro.begin(token, i8* writeonly) #2
+
+; Function Attrs: nounwind
+declare i8 @llvm.coro.suspend(token, i1) #2
+
+; Function Attrs: argmemonly nounwind readonly
+declare i8* @llvm.coro.free(token, i8* nocapture readonly) #1
+
+; Function Attrs: nounwind
+declare i1 @llvm.coro.end(i8*, i1) #2
+
+declare i8* @new(i64)
+
+declare void @delete(i8*)
+
+declare i1 @await_ready()
+
+declare void @await_suspend()
+
+declare void @await_resume()
+
+declare void @print(i32)
+
+declare i8* @from_address(i8*)
+
+declare void @return_void()
+
+declare void @final_suspend()
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #4
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+attributes #0 = { nofree nosync nounwind readnone speculatable willreturn }
+attributes #1 = { argmemonly nounwind readonly }
+attributes #2 = { nounwind }
+attributes #3 = { nounwind readnone }
+attributes #4 = { argmemonly nofree nosync nounwind willreturn writeonly }
+
+!llvm.dbg.cu = !{!0}
+!llvm.linker.options = !{}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 11.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "repro.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{!"clang version 11.0.0"}
+!6 = !DILocalVariable(name: "i", scope: !7, file: !1, line: 24, type: !10)
+!7 = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
+!8 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 23, type: !9, scopeLine: 23, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !2)
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DILocation(line: 0, scope: !7)
+!12 = !DILocalVariable(name: "x", scope: !13, file: !1, line: 34, type: !14)
+!13 = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
+!14 = !DICompositeType(tag: DW_TAG_array_type, baseType: !10, size: 320, elements: !15)
+!15 = !{!16}
+!16 = !DISubrange(count: 10)
+!17 = !DILocation(line: 24, column: 7, scope: !7)
+!18 = !DILocalVariable(name: "j", scope: !7, file: !1, line: 32, type: !10)
+!19 = !DILocation(line: 42, column: 3, scope: !7)
+!20 = !DILocation(line: 42, column: 8, scope: !7)
+!21 = !DILocation(line: 43, column: 3, scope: !7)
+!22 = !DILocation(line: 43, column: 8, scope: !7)

llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll

+; Tests whether resume function would remain dbg.value infomation.
+; RUN: opt < %s -coro-early -coro-split -coro-split -S | FileCheck %s
+;
+; This file is based on coro-debug-frame-variable.ll.
+; CHECK:  define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK:       init.ready:
+; CHECK:         call void @llvm.dbg.value(metadata %f.Frame** %FramePtr.debug, metadata ![[XVAR_RESUME:[0-9]+]],
+; CHECK:       await.ready:
+; CHECK:         call void @llvm.dbg.value(metadata %f.Frame** %FramePtr.debug, metadata ![[SPILL_RESUME:[0-9]+]]
+; CHECK:         call void @llvm.dbg.value(metadata %f.Frame** %FramePtr.debug, metadata ![[IVAR_RESUME:[0-9]+]], metadata !DIExpression(
+; CHECK:         call void @llvm.dbg.value(metadata %f.Frame** %FramePtr.debug, metadata ![[JVAR_RESUME:[0-9]+]], metadata !DIExpression(
+;
+; CHECK: ![[RESUME_FN_DBG_NUM]] = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov"
+; CHECK: ![[IVAR_RESUME]] = !DILocalVariable(name: "i"
+; CHECK: ![[XVAR_RESUME]] = !DILocalVariable(name: "x"
+; CHECK: ![[JVAR_RESUME]] = !DILocalVariable(name: "j"
+; CHECK: ![[SPILL_RESUME]] = !DILocalVariable(name: "produced"
+
+source_filename = "../llvm/test/Transforms/Coroutines/coro-debug-dbg.values-O2.ll"
+declare void @consume(i32)
+
+define void @f(i32 %i, i32 %j) !dbg !8 {
+entry:
+  %__promise = alloca i8, align 8
+  %x = alloca [10 x i32], align 16
+  %produced = call i32 @value_producer()
+  %id = call token @llvm.coro.id(i32 16, i8* %__promise, i8* null, i8* null)
+  %alloc = call i1 @llvm.coro.alloc(token %id)
+  br i1 %alloc, label %coro.alloc, label %coro.init
+
+coro.alloc:                                       ; preds = %entry
+  %size = call i64 @llvm.coro.size.i64()
+  %memory = call i8* @new(i64 %size)
+  br label %coro.init
+
+coro.init:                                        ; preds = %coro.alloc, %entry
+  %phi.entry.alloc = phi i8* [ null, %entry ], [ %memory, %coro.alloc ]
+  %begin = call i8* @llvm.coro.begin(token %id, i8* %phi.entry.alloc)
+  %ready = call i1 @await_ready()
+  br i1 %ready, label %init.ready, label %init.suspend
+
+init.suspend:                                     ; preds = %coro.init
+  %save = call token @llvm.coro.save(i8* null)
+  call void @await_suspend()
+  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
+  switch i8 %suspend, label %coro.ret [
+    i8 0, label %init.ready
+    i8 1, label %init.cleanup
+  ]
+
+init.cleanup:                                     ; preds = %init.suspend
+  br label %cleanup
+
+init.ready:                                       ; preds = %init.suspend, %coro.init
+  call void @await_resume()
+  call void @llvm.dbg.value(metadata i32 0, metadata !6, metadata !DIExpression()), !dbg !11
+  %i.init.ready.inc = add nsw i32 0, 1
+  call void @llvm.dbg.value(metadata i32 %i.init.ready.inc, metadata !6, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.value(metadata [10 x i32]* %x, metadata !12, metadata !DIExpression()), !dbg !17
+  %memset = bitcast [10 x i32]* %x to i8*, !dbg !17
+  call void @llvm.memset.p0i8.i64(i8* align 16 %memset, i8 0, i64 40, i1 false), !dbg !17
+  call void @print(i32 %i.init.ready.inc)
+  %ready.again = call zeroext i1 @await_ready()
+  br i1 %ready.again, label %await.ready, label %await.suspend
+
+await.suspend:                                    ; preds = %init.ready
+  %save.again = call token @llvm.coro.save(i8* null)
+  %from.address = call i8* @from_address(i8* %begin)
+  call void @await_suspend()
+  %suspend.again = call i8 @llvm.coro.suspend(token %save.again, i1 false)
+  switch i8 %suspend.again, label %coro.ret [
+    i8 0, label %await.ready
+    i8 1, label %await.cleanup
+  ]
+
+await.cleanup:                                    ; preds = %await.suspend
+  br label %cleanup
+
+await.ready:                                      ; preds = %await.suspend, %init.ready
+  call void @await_resume()
+  call void @llvm.dbg.value(metadata i32 0, metadata !18, metadata !DIExpression()), !dbg !11
+  %arrayidx0 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 0, !dbg !19
+  store i32 1, i32* %arrayidx0, align 16, !dbg !20
+  %arrayidx1 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 1, !dbg !21
+  store i32 2, i32* %arrayidx1, align 4, !dbg !22
+  %i.await.ready.inc = add nsw i32 %i.init.ready.inc, 1
+  call void @consume(i32 %produced)
+  call void @consume(i32 %i)
+  call void @consume(i32 %j)
+  call void @llvm.dbg.value(metadata i32 %produced, metadata !23, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.value(metadata i32 %i, metadata !6, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.value(metadata i32 %j, metadata !18, metadata !DIExpression()), !dbg !11
+  call void @print(i32 %i.await.ready.inc)
+  call void @return_void()
+  br label %coro.final
+
+coro.final:                                       ; preds = %await.ready
+  call void @final_suspend()
+  %coro.final.await_ready = call i1 @await_ready()
+  br i1 %coro.final.await_ready, label %final.ready, label %final.suspend
+
+final.suspend:                                    ; preds = %coro.final
+  %final.suspend.coro.save = call token @llvm.coro.save(i8* null)
+  %final.suspend.from_address = call i8* @from_address(i8* %begin)
+  call void @await_suspend()
+  %final.suspend.coro.suspend = call i8 @llvm.coro.suspend(token %final.suspend.coro.save, i1 true)
+  switch i8 %final.suspend.coro.suspend, label %coro.ret [
+    i8 0, label %final.ready
+    i8 1, label %final.cleanup
+  ]
+
+final.cleanup:                                    ; preds = %final.suspend
+  br label %cleanup
+
+final.ready:                                      ; preds = %final.suspend, %coro.final
+  call void @await_resume()
+  br label %cleanup
+
+cleanup:                                          ; preds = %final.ready, %final.cleanup, %await.cleanup, %init.cleanup
+  %cleanup.dest.slot.0 = phi i32 [ 0, %final.ready ], [ 2, %final.cleanup ], [ 2, %await.cleanup ], [ 2, %init.cleanup ]
+  %free.memory = call i8* @llvm.coro.free(token %id, i8* %begin)
+  %free = icmp ne i8* %free.memory, null
+  br i1 %free, label %coro.free, label %after.coro.free
+
+coro.free:                                        ; preds = %cleanup
+  call void @delete(i8* %free.memory)
+  br label %after.coro.free
+
+after.coro.free:                                  ; preds = %coro.free, %cleanup
+  switch i32 %cleanup.dest.slot.0, label %unreachable [
+    i32 0, label %cleanup.cont
+    i32 2, label %coro.ret
+  ]
+
+cleanup.cont:                                     ; preds = %after.coro.free
+  br label %coro.ret
+
+coro.ret:                                         ; preds = %cleanup.cont, %after.coro.free, %final.suspend, %await.suspend, %init.suspend
+  %end = call i1 @llvm.coro.end(i8* null, i1 false)
+  ret void
+
+unreachable:                                      ; preds = %after.coro.free
+  unreachable
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+; Function Attrs: argmemonly nounwind readonly
+declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*) #1
+
+; Function Attrs: nounwind
+declare i1 @llvm.coro.alloc(token) #2
+
+; Function Attrs: nounwind readnone
+declare i64 @llvm.coro.size.i64() #3
+
+; Function Attrs: nounwind
+declare token @llvm.coro.save(i8*) #2
+
+; Function Attrs: nounwind
+declare i8* @llvm.coro.begin(token, i8* writeonly) #2
+
+; Function Attrs: nounwind
+declare i8 @llvm.coro.suspend(token, i1) #2
+
+; Function Attrs: argmemonly nounwind readonly
+declare i8* @llvm.coro.free(token, i8* nocapture readonly) #1
+
+; Function Attrs: nounwind
+declare i1 @llvm.coro.end(i8*, i1) #2
+
+declare i8* @new(i64)
+
+declare void @delete(i8*)
+
+declare i1 @await_ready()
+
+declare void @await_suspend()
+
+declare void @await_resume()
+
+declare void @print(i32)
+
+declare i8* @from_address(i8*)
+
+declare void @return_void()
+
+declare void @final_suspend()
+
+declare i32 @value_producer()
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #4
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+attributes #0 = { nofree nosync nounwind readnone speculatable willreturn }
+attributes #1 = { argmemonly nounwind readonly }
+attributes #2 = { nounwind }
+attributes #3 = { nounwind readnone }
+attributes #4 = { argmemonly nofree nosync nounwind willreturn writeonly }
+
+!llvm.dbg.cu = !{!0}
+!llvm.linker.options = !{}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 11.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "repro.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{!"clang version 11.0.0"}
+!6 = !DILocalVariable(name: "i", scope: !7, file: !1, line: 24, type: !10)
+!7 = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
+!8 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 23, type: !9, scopeLine: 23, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !2)
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DILocation(line: 0, scope: !7)
+!12 = !DILocalVariable(name: "x", scope: !13, file: !1, line: 34, type: !14)
+!13 = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
+!14 = !DICompositeType(tag: DW_TAG_array_type, baseType: !10, size: 320, elements: !15)
+!15 = !{!16}
+!16 = !DISubrange(count: 10)
+!17 = !DILocation(line: 24, column: 7, scope: !7)
+!18 = !DILocalVariable(name: "j", scope: !7, file: !1, line: 32, type: !10)
+!19 = !DILocation(line: 42, column: 3, scope: !7)
+!20 = !DILocation(line: 42, column: 8, scope: !7)
+!21 = !DILocation(line: 43, column: 3, scope: !7)
+!22 = !DILocation(line: 43, column: 8, scope: !7)
+!23 = !DILocalVariable(name: "produced", scope: !7, file: !1, line:24, type: !10)