[PATCH] D60831: [DebugInfo at O2][LoopVectorize] pr39024: Vectorized code linenos step through loop even after completion

Wed Apr 24 07:38:29 PDT 2019

bjope added a comment.

In D60831#1476952 <https://reviews.llvm.org/D60831#1476952>, @probinson wrote:

> In D60831#1472621 <https://reviews.llvm.org/D60831#1472621>, @jmellorcrummey wrote:
>
> > This will be my last comment on the topic and then everyone can get back to work :-).   I agree with the LLVM IR requirement that one not to lie to application developers; lies are worse than nothing. However, I don't feel that you have adequately explained why attributing to any of the available (file, line) mappings for an instruction that has been merged is incorrect and not 100% reliable. Just because we can't include all contributing mappings for a merged instruction doesn't make any one unreliable; I see attributing to any one of a set of mappings as 100% correct, even though it is only partial information.
>
>
> Sorry for jumping in after you said you would stop, but I have been away.
>  Personally I find this example compelling: When there's an if/then/else construct, and some instruction is hoisted above the if, you could assign it to (for example) the source location of the 'then' block. Now in your training run, the 'then' block can be given 100% of executions (because that's what the hoisted instruction says) even if the 'else' block was chosen 100% of the time.  I find the resulting profile is "incorrect and not 100% reliable" and when used for PGO will produce sub-optimal code. It is hard to imagine any other valid conclusion for this use-case.
>
> As Adrian mentioned, being able to attribute multiple locations would be preferable to attributing line 0, and I hope we can make that happen in the future.

Even with multiple locations it might be weird when using those locations for profiling (or code coverage) and not just as debug information to see which source line the code origins from. If the hoisted instruction has locations from both the 'if', 'elseif' and 'else' blocks it might appear as if we have executed all paths, even if the 'else' block was chosen 100% of the time. So I guess that the samples with multiple locations should be discarded when doing profiling (perhaps depending on the goal with the profiling).

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

https://reviews.llvm.org/D60831