RE: [compare-debug] use call loc for nop_endbr

["Tsimbalist, Igor V" <igor dot v dot tsimbalist at intel dot com>]

> -----Original Message-----
> From: Alexandre Oliva [mailto:aoliva@redhat.com]
> Sent: Wednesday, December 13, 2017 8:34 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Subject: [compare-debug] use call loc for nop_endbr
> 
> 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.
> 
> Regstrapping with -fcompare-debug on stage3 host and target builds on
> x86_64- and i686-linux-gnu; ok to install?

Ok from me.

Am I correct the error you had was related to improper location information,
not the placement of the instruction? I will try to skip NOTE insns only.

Igor

> for  gcc/ChangeLog
> 
> 	* config/i386/i386.c (rest_of_insert_endbranch): Use call loc
> 	for its nop_endbr.
> ---
>  gcc/config/i386/i386.c |   20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index e323102cef59..8960b966b7fc 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -2609,21 +2609,25 @@ rest_of_insert_endbranch (void)
>  	{
>  	  if (INSN_P (insn) && GET_CODE (insn) == CALL_INSN)
>  	    {
> -	      rtx_insn *next_insn = insn;
> +	      if (find_reg_note (insn, REG_SETJMP, NULL) == NULL)
> +		continue;
> +	      /* Generate ENDBRANCH after CALL, which can return more than
> +		 twice, setjmp-like functions.  */
> 
> +	      /* Skip notes and debug insns that must be next to the
> +		 call insn.  ??? This might skip a lot more than
> +		 that...  ??? Skipping barriers and emitting code
> +		 after them surely looks like a mistake; we probably
> +		 won't ever hit it, for we'll hit BB_END first.  */
> +	      rtx_insn *next_insn = insn;
>  	      while ((next_insn != BB_END (bb))
>  		      && (DEBUG_INSN_P (NEXT_INSN (next_insn))
>  			  || NOTE_P (NEXT_INSN (next_insn))
>  			  || BARRIER_P (NEXT_INSN (next_insn))))
>  		next_insn = NEXT_INSN (next_insn);
> 
> -	      /* Generate ENDBRANCH after CALL, which can return more than
> -		 twice, setjmp-like functions.  */
> -	      if (find_reg_note (insn, REG_SETJMP, NULL) != NULL)
> -		{
> -		  cet_eb = gen_nop_endbr ();
> -		  emit_insn_after (cet_eb, next_insn);
> -		}
> +	      cet_eb = gen_nop_endbr ();
> +	      emit_insn_after_setloc (cet_eb, next_insn, INSN_LOCATION
> (insn));
>  	      continue;
>  	    }
> 
> 
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer