This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83396#c28 on ia64 (PR bootstrap/83396, take 2)


On Wed, 13 Dec 2017, Jakub Jelinek wrote:

> On Wed, Dec 13, 2017 at 03:25:07PM +0100, Jakub Jelinek wrote:
> > I think there are 2 issues.  One is that the ia64 backend emits
> > the group barrier insns before BB_HEAD label, so it isn't part of a bb,
> > but has BLOCK_FOR_INSN of the following block, that looks invalid to me
> > and the ia64.c hunk ought to fix that, except that I don't have access to
> > ia64 anymore and so can't test it.  Andreas, could you try that?
> > 
> > Another thing is that if we because of this end up with insns outside of
> > basic blocks, the vt_initialize asserts will fire again.  Here, first of
> > all, IMNSHO we should assert that debug bind insns aren't outside of basic
> > blocks, the other patches and checking should ensure that (and if any slips
> > in, we want to fix that too rather than work-around).
> > Another is that while walking from get_first_insn to one before BB_HEAD (bb->next_bb),
> > we can encounter insns outside of bb not just before BB_HEAD (bb), but also
> > after BB_END (bb), both cases are outside of a bb and thus we can
> > expect BLOCK_FOR_INSN being NULL.
> > 
> > Bootstrapped/regtested on x86_64-linux, i686-linux, powerpc64le-linux,
> > regtest on powerpc64-linux pending.  Ok for trunk perhaps without the
> > ia64.c bits until that gets tested?
> > 
> > Or, in the PR there is a variant patch which just doesn't do the asserts and
> > doesn't have to track outside_bb.
> 
> Here is another variant, without trying to change ia64 backend which
> apparently doesn't bootstrap for other reasons.
> 
> This patch instead ignores insns outside of basic blocks during var-tracking
> exactly as it has been ignoring them before, and just processes the debug
> begin stmt markers in there (and verifies no debug bind stmts appear in
> between bbs).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> 2017-12-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR bootstrap/83396
> 	* var-tracking.c (vt_initialize): Ignore non-DEBUG_INSNs outside of
> 	basic blocks.  Assert debug bind insns don't appear outside of bbs,
> 	don't reset them.  Assert insns without BLOCK_FOR_INSN are outside of
> 	bb.  Simplify.
> 
> 	* gcc.dg/pr83396.c: New test.
> 
> --- gcc/var-tracking.c.jj	2017-12-13 13:22:59.651783152 +0100
> +++ gcc/var-tracking.c	2017-12-13 19:11:13.895699735 +0100
> @@ -10157,25 +10157,31 @@ vt_initialize (void)
>  	     insns that might be before it too.  Unfortunately,
>  	     BB_HEADER and BB_FOOTER are not set while we run this
>  	     pass.  */
> -	  insn = get_first_insn (bb);
> -	  for (rtx_insn *next;
> -	       insn != BB_HEAD (bb->next_bb)
> -		 ? next = NEXT_INSN (insn), true : false;
> +	  rtx_insn *next;
> +	  bool outside_bb = true;
> +	  for (insn = get_first_insn (bb); insn != BB_HEAD (bb->next_bb);
>  	       insn = next)
>  	    {
> +	      if (insn == BB_HEAD (bb))
> +		outside_bb = false;
> +	      else if (insn == NEXT_INSN (BB_END (bb)))
> +		outside_bb = true;
> +	      next = NEXT_INSN (insn);
>  	      if (INSN_P (insn))
>  		{
> +		  if (outside_bb)
> +		    {
> +		      /* Ignore non-debug insns outside of basic blocks.  */
> +		      if (!DEBUG_INSN_P (insn))
> +			continue;
> +		      /* Debug binds shouldn't appear outside of bbs.  */
> +		      gcc_assert (!DEBUG_BIND_INSN_P (insn));
> +		    }
>  		  basic_block save_bb = BLOCK_FOR_INSN (insn);
>  		  if (!BLOCK_FOR_INSN (insn))
>  		    {
> +		      gcc_assert (outside_bb);
>  		      BLOCK_FOR_INSN (insn) = bb;
> -		      gcc_assert (DEBUG_INSN_P (insn));
> -		      /* Reset debug insns between basic blocks.
> -			 Their location is not reliable, because they
> -			 were probably not maintained up to date.  */
> -		      if (DEBUG_BIND_INSN_P (insn))
> -			INSN_VAR_LOCATION_LOC (insn)
> -			  = gen_rtx_UNKNOWN_VAR_LOC ();
>  		    }
>  		  else
>  		    gcc_assert (BLOCK_FOR_INSN (insn) == bb);
> --- gcc/testsuite/gcc.dg/pr83396.c.jj	2017-12-13 15:53:15.446687005 +0100
> +++ gcc/testsuite/gcc.dg/pr83396.c	2017-12-13 15:53:15.446687005 +0100
> @@ -0,0 +1,12 @@
> +/* PR bootstrap/83396 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +int bar (int);
> +int baz (int);
> +
> +int
> +foo (int x)
> +{
> +  return bar (x) || baz (x) != 0;
> +}
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]