[PATCH] D82129: [DebugInfo] Drop location ranges for variables which exist entirely outside the variable's scope

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

Orlando added a comment.

In D82129#2111602 <https://reviews.llvm.org/D82129#2111602>, @probinson wrote:

> In D82129#2110934 <https://reviews.llvm.org/D82129#2110934>, @Orlando wrote:
>
> > In D82129#2107017 <https://reviews.llvm.org/D82129#2107017>, @probinson wrote:
> >
> > > This patch depends on the ranges for all scopes to be (reasonably) correct,
> >
> >
> > I'd say instead that 'variable locations depend on the ranges for all scopes to be (reasonably) correct'. And that this patch just acknowledges that relationship and clears away what we cannot use/see in a debugger.
>
>
> Either way, the question remains: when we find cases where we need to "clear away" something, is that a bug, or is this merely a cleanup pass?  In the case of the test I commented on, it's a bug, and I'd rather not be hiding bugs.


Looking closer at COFF/register-variables.ll, it doesn't look like a bug but instead another victim of how we model debug info. Before running -branch-folder (Control Flow Optimizer) all the instructions in the else block belong to the else block scope. The branch folder merges the common tails from 'then' and 'else' into 'else', merging the debug-locations while it does so. @jmorse summarised the situation well offline: Every time we call getMergedLocation, we are creating the conditions where this occurs, and eliminating it during compilation would be a high burden.


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

https://reviews.llvm.org/D82129