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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

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. Phis often cannot assume a previous location (say, if they're at the start of a block).

If it's ok for an instruction to have no location, I think we should try to either define those cases in a way that can be checked, or relax any verifiers if that proves too difficult. I'm not sure what those cases are, though. I think it's reasonable to require locations on newly-created instructions, or to update the location of a moved instruction using a neighboring instruction.


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

https://reviews.llvm.org/D59417