[PATCH] D70121: [DebugInfo][LDV] Teach LDV how to identify source variables and handle fragments

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

> Let me see if I can restate that correctly. So, the problem here is: for the list of user-values tracked by the interval map for the range containing [DV1, DV3], there are two entries: one for {DV1, DV3} (coalesced), and another for {DV2}. This would result in "x" being interpreted as `%1 + 4` instead of `%0` in that interval (by the debugger)?

Short answer: yes, that is the behaviour we observe in the debugger.  Longer
answer: the cause of the issue is slightly different to how I read your
explanation. The core problem here is that in this situation _two_ interval maps
are created instead of one:

  A: { [DV1, DV3), [DV3, ...) }
     ** Coalesce these adjacent ranges because they both use the same location (%0) **
     { [DV1, ...) }
  B: { [DV2, ...) }

This results in "x" being interpreted as `%1 + 4` in the range [DV2, ...). As
stated previously this happens because of the method we use currently to filter
DBG_VALUEs into the 'correct' interval maps.

In this example, to get correct debuginfo, we would want to see the filter
changed so that all of these DBG_VALUEs end up in the same map, and slightly
different coalescing rules (e.g. D66415 <https://reviews.llvm.org/D66415>):

  C: { [DV1, DV2), [DV2, DV3), [DV3, ...) }
     ** No coalescing because no adjacent ranges use the same location AND expression. **

> Anything I'd write here would be highly contrived, but I don't believe that these overlaps are senseless. E.g. we see this in IR all the time:
> 
>   dbg.value(i32 0, "x", DIExpression())
>   dbg.value(i32 1, "x", DIExpression())
> 
> 
> and this is seen as well-defined.

Sorry, this question was badly written. I meant to ask if we could ever consider
banning _paritally_ overlapping fragments. But you do answer this elsewhere anyway:

> I don't think we should ban these partial overlaps from showing up in IR, as I can
>  imagine situations where this sort of IR is legitimately describing the known bits
>  in a value. What we might consider doing to make LDV more conservative is a)
>  identifying partial overlaps in a dbg.value pack using a backwards scan and b)
>  deleting any earlier dbg.values which overlap with bits described later in the
>  pack. That should be conservatively correct, and wouldn't require changing the
>  interval map structure?

IIUC I'm not sure that works with this example?

  DBG_VALUE %0, "x", (DW_OP_LLVM_fragment 00 16) ; DV1
  ...
  DBG_VALUE %1, "x", (DW_OP_LLVM_fragment 00 16) ; DV2
  DBG_VALUE %0, "x", (DW_OP_LLVM_fragment 08 32) ; DV3
  ...
  DBG_VALUE %0, "x", (DW_OP_LLVM_fragment 00 16) ; DV4

Going forward, I will:

1. Abandon this patch (for now...)
2. Touch-up and reopen D66415 <https://reviews.llvm.org/D66415> (fixes the 99.99995% case)

Thank you for sticking with me on this one!


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

https://reviews.llvm.org/D70121