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

[Bruno Cardoso Lopes via Phabricator via llvm-commits]

bruno added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:1213
+      // These instructions are all dominated by the alloca, insert the
+      // dbg.declare in the beginning of the BB to enhance debugging
+      // experience and be able to inspect values as early as possible.
----------------
lxfind wrote:
> Do we need to insert to every BB that uses it though? Though this may be the safest way to guarantee there is at least one, so I don't object doing this.
> Also want to point out this: https://llvm.org/docs/SourceLevelDebugging.html#llvm-dbg-declare
> it says there can only be one dbg.declare. However in practice I think as long as they all look the same it should be fine.
> Do we need to insert to every BB that uses it though? Though this may be the safest way to guarantee there is at least one, so I don't object doing this.

That's a good question, I initially thought a `dbg.declare` dominating the BB in question would be enough, perhaps the debugging experts could help clarify what's going on.

> Also want to point out this: https://llvm.org/docs/SourceLevelDebugging.html#llvm-dbg-declare
> it says there can only be one dbg.declare. However in practice I think as long as they all look the same it should be fine.

Yep, I also saw this but didn't get any verifier complains (perhaps because they look the same), guess we have another question for the experts :) 


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

https://reviews.llvm.org/D90772