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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

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

> In D59417#1431111 <https://reviews.llvm.org/D59417#1431111>, @dblaikie wrote:
>
> > In D59417#1431042 <https://reviews.llvm.org/D59417#1431042>, @probinson wrote:
> >
> > > I believe this is a patch to fix PR37964?
> >
> >
> > Hrm - if debugify diagnoses all instances of instructions without locations as buggy, that sounds very noisy to me. How does it handle all the other cases of merged locations that end up with no location? (if my recollection is accurate and the code is still doing this - merged locations that aren't the same and that aren't calls get no location, not a zero location)
>
>
> It looks like DILocation::getMergedLocation() always returns a zero location, and that Instruction::applyMergedLocation() no longer restricts itself to calls. This changed with D45396 <https://reviews.llvm.org/D45396>+D45397 <https://reviews.llvm.org/D45397>: we wanted mem2reg to set merged locations on phis to help users narrow down the scope of a crashing instruction.


Thanks for citing the previous reviews & reminding me of the facts of the matter (clearly I asked the same questions then and got the right answers/data to justify the change).

>   Phis often cannot assume a previous location (say, if they're at the start of a block).

Not sure I follow this bit - setting a zero location doesn't make them any easier to debug, does it? Or was it that the "cascade location" (whatever the previous instruction was) the source of confusion?

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.


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

https://reviews.llvm.org/D59417