[PATCH] D25611: When moving a zext near to its associated load, do not retain the origial debug location.

[Bjorn Pettersson via llvm-commits]

bjope added a comment.

I'm not an expert on debug info either, but to me the solution sound weird.
Isn't it a fact then when having optimisations turned on debugging might get tricky.

What if for example the target does not combine the zext and the load into a zextload. Then there would be a zext with apparently incorrect debug location. I'm not fully convinced that incorrect debug locations will aid the cause of making debugging easier.

Isn't the tricky part optimisations that combines instructions with different debug locations. Only one can be kept, so all such optimisations should choose wisely (if possible).  I'm not sure about the strategy used by DAGCombiner here. Is it always keeping the "outer" instructions debug locations per design, or could it be that when combining a zext and and load it should keep the debug location of the load?

My feeling is that the choice should be made when doing the optimisations that needs to pick one debug location and discard the other. In this scenario CodeGenPrepare only moves instructions.

If something should be done when moving the zext, then I guess it should be dropping the debug information. Isn't inheriting it from the load simply wrong? That would just yield incorrect debug location for the zext in case it isn't combined with the load later.


https://reviews.llvm.org/D25611