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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Hi Bruno,

> Can you clarify what you mean by "silently transformed into something like dbg.value..."? In what stage do you see that happening? Those don't show up for me unless the local variables are already promoted to regs.

During instruction selection, so quite late: if you run "llc -stop-before=finalize-isel" on some IR that contains a dbg.declare, if it refers to a stack slot then the variable location is attached to the stack slot:

  stack:
    - { id: 0, name: __x.addr, type: default, offset: 0, size: 8, alignment : 8,
        stack-id: default, callee-saved-register: '', callee-saved-restored : true,
        debug-info-variable: '!692', debug-info-expression: '!DIExpression()',
        debug-info-location: '!693' }

However if the dbg.declare can't be tracked back to a stack slot, it becomes a "DBG_VALUE" machine instruction (equivalent to a dbg.value intrinsic):

  DBG_VALUE %26, 0, !704, !DIExpression(), debug-location !706  

The former doesn't need to worry about control flow because the variable is always homed in a stack slot; the latter does need to worry about control flow. I imagine that everything to do with coroutines will take the latter path.

> What's the impact of having reductant dbg.declares in already dominated paths? Is there a pass that cleans those up? Thoughts?

Zero functional change, and  a tiny performance cost from having one extra metadata instruction hanging around. If you go down this route, I'd recommend using the `dbg.value` intrinsic with a DIExpression with a single DW_OP_deref -- this is effectively what `dbg.declare` becomes as shown above, and avoids any unexpected surprises if it turns out some code somewhere really does expect only one `dbg.declare` to exist.

junparser wrote:

> This patch works for llvm.dbg.addr. So what is status of llvm.dbg.addr? does anyone know about it ?

In theory this kind of behaviour is exactly what dbg.addr is for (variable lives in memory, maybe changes due to control flow). However moving everything to use dbg.addr stalled a long time ago, I believe @Orlando found that it's often unexpectedly dropped by optimisations, any opinion on whether it's usable @Orlando? We're hoping to use it for things someday soon, but we're definitely not there yet.


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

https://reviews.llvm.org/D90772