On Wed, Dec 13, 2017 at 05:34:22AM -0200, Alexandre Oliva wrote:
> We skip debug insns and notes after a call that needs a nop_endbr, but
> since a debug insn could be the last in a block, it may affect the loc
> in the emitted nop_endbr insn. Although this has no effect on
> codegen, it does mess with debug info a bit, and it causes
> -fcompare-debug to fail for e.g. libsanitizer's
> tsan/tsan_platform_linux.cc on x86_64.
>
> So, pick the location of the call insn for the nop_endbr insn, to
> avoid the line number differences in dumps, including -fcompare-debug
> ones.
>
> Also, we don't need to determine what the insert point would be unless
> we're actually emitting the nop_endbr insn after the call, so
> rearrange the code to avoid wasting cycles.
>
> Finally, it seems like testing for barriers is a mistake. We probably
> never actually pass that test, for the barriers would hit BB_END
> first. If we did, we'd end up emitting the nop_endbr outside any BB,
> even after the end of the function! That would be Very Bad (TM).
> Now, since the test as it is can't hurt, I figured I wouldn't change
> the logic right now, just add a comment so that someone involved in
> endbr stuff can have a second look and hopefully fix it.
>
> I'd appreciate if you'd try to drop the BARRIER_P from the loop test,
> Igor, so as to address the final ??? in the comment I add. Narrowing
> the skipped notes to only the relevant post-call ones might make sense
> as well, but it's not quite as important IMHO.
I believe the only insn that needs to be skipped is
NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_CALL_ARG_LOCATION
and there should be at most one of these after the call.
Anything else I believe can be separated from the call without problems.
Jakub
