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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

>>   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?

Setting a zero location on the phi shows the user that the crash occurred in 'inner()', even though it was inlined into 'outer()'. That makes things less confusing (say, if 'inner' is a lot shorter than 'outer'). Of course, the only reason this is helpful is because the zero location on the phi cascades down to the crashy StoreInst.

> 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?


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

https://reviews.llvm.org/D59417