[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.

Thanks @djtodoro for looking at this. Sorry for this really delayed response!

This was initially going to be a ping, but as a result of some light offline
discussion I have a few questions. For clarification an "overlap" here refers to
a partial overlap between fragments in DIExpressions.

There are other places in llvm which do not consider overlaps, and patches
which ignore overlaps are being accepted (e.g. D71478 <https://reviews.llvm.org/D71478>).

1. Do overlapping fragments in dbg.values ever occur before isel currently? I would be happy to write an IRVerifier check for this and document it somewhere if this is appropriate.

2. More radically and long term, would it be feasible to ever ban overlapping fragments in DBG_VALUEs (MIR) somehow?

As demonstrated by this patch overlapping fragments are a pain to handle
properly. The original patch D66415 <https://reviews.llvm.org/D66415> was much smaller but didn't handle
overlaps.

I have gathered some stats from clang and llvm-as RelWithDebInfo builds @
llvm 3.4 by counting overlaps found in the livedebugvars pass with this patch:

Terminology:
`Def`: A `{ Variable, Scope, Fragment }` tuple representing a dbg.value instance.
`VarSet`: A set containing all the `Defs` with any given `Variable` and `Scope`.
`Overlaps`: Sum of unique `Fragment` overlaps between `Def`s in a `VarSet` for each `VarSet`.

  llvm 3.4 binary |     llvm-as |       clang
  ----------------|--------------------------
  Num VarSets     |     2284961 |    14430359
  Overlaps        |           2 |           7

These stats are only anecdotal and the results are C++ specific so any conclusion
would have to be drawn very tentatively. But:

3. I would be okay with dropping this patch in favour of cleaning up D66415 <https://reviews.llvm.org/D66415> and adding a FIXME comment since this patch may be useless in the real world. What do you think?


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

https://reviews.llvm.org/D70121