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

Message ID 20171213222436.GO2353@tucnak
State New
Headers show
Series
  • Fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83396#c28 on ia64 (PR bootstrap/83396, take 2)
Related show

Commit Message

Jakub Jelinek Dec. 13, 2017, 10:24 p.m.
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?

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.



	Jakub

Comments

Richard Biener Dec. 14, 2017, 10:31 a.m. | #1
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)

Patch

--- 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;
+}