[PATCH] D59417: [GVN] Add default debug location when constructing PHI nodes

Mon Mar 18 11:13:58 PDT 2019

probinson added a comment.

In D59417#1433403 <https://reviews.llvm.org/D59417#1433403>, @vsk wrote:

> In D59417#1431496 <https://reviews.llvm.org/D59417#1431496>, @probinson wrote:
>
> > In D59417#1431369 <https://reviews.llvm.org/D59417#1431369>, @vsk wrote:
> >
> > > > As for instructions at the start of the block - I'm pretty sure that a while back @probinson made a change to ensure that cascade locations couldn't occur at the start of a block (that we'd set to line zero down in the AsmPrinter/DwarfDebug if that occurred) so we shouldn't end up with super weird/layout-based cascade locations for instructions generated from phis at the start of blocks.
> > >
> > > That makes sense. What if a block has multiple phis? I.e. if the first phi in a block has a location but the second phi doesn't, is that a bug, or do we expect the location to cascade?
> >
> >
> > Locations cascade, with one exception: If the first instruction after a label does not have an explicit source location, we emit line 0.
>
>
> That sounds like a sensible rule. Presumably that line 0 location has the correct scope set. Shall I relax the debugify verifier so that it only diagnoses non-phi instructions without locations? I.e. are phis special in some way, or would the relaxed check still be too strict (& any 'no location' diagnostic could be noisy)?

AsmPrinter/DwarfDebug are careful to track the most recent emitted location, and change only the line number. This gets the desired effect in DWARF (line 0 = no specific location) without a lot of bloat resetting the file & column numbers as well.

Somebody else should chime in about relaxing the verifier, as it was my idea and I often don't know what I'm talking about. :-)

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

https://reviews.llvm.org/D59417