Re: [PATCH] Fix df-related ICE due to bbpart pass bug (PR target/81621)

[Jakub Jelinek]

On Thu, Aug 03, 2017 at 11:02:51AM +0200, Richard Biener wrote:
> On Thu, 3 Aug 2017, Jakub Jelinek wrote:
> 
> > Hi!
> > 
> > The following testcase ICEs on s390x.  The problem is that the bbpart pass
> > calls
> > df_set_flags (DF_DEFER_INSN_RESCAN);
> > because it wants to defer rescanning, but doesn't actually df_finish_pass
> > (it does in one case, but then calls df_set_flags with another changeable 
> > flag,
> > so it has the same issue), and if the IRA pass is invoked soon after it
> > without any df_finish_pass calls in between, we end up with deferred insn
> > rescanning during IRA which heavily relies on immediate insn rescanning.
> > 
> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> 
> Maybe add a comment in case somebody wonders later?

Ok.

> > --- gcc/bb-reorder.c.jj 2017-07-21 10:28:13.0 +0200
> > +++ gcc/bb-reorder.c2017-08-02 19:43:58.797243254 +0200
> > @@ -2904,7 +2904,7 @@ pass_partition_blocks::execute (function
> >  
> >crossing_edges = find_rarely_executed_basic_blocks_and_crossing_edges ();
> >if (!crossing_edges.exists ())
> > -return 0;
> > +return TODO_df_finish;
> 
> I suppose we can avoid this if we move the df_set_flags after this?
> I doubt find_rarely_executed_basic_blocks_and_crossing_edges modifies
> anything.
> 
> Ok with those changes (if the latter is possible).

I was looking through find_rarely_executed_basic_blocks_and_crossing_edges
before writing the patch and while I could prove for some functions that
it doesn't modify anything, but e.g. for fix_up_crossing_landing_pad I'm
pretty sure it can modify instructions in several ways.
So I think we can't do that.

Jakub

Re: [PATCH] Fix df-related ICE due to bbpart pass bug (PR target/81621)

[Richard Biener]

On Thu, 3 Aug 2017, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs on s390x.  The problem is that the bbpart pass
> calls
> df_set_flags (DF_DEFER_INSN_RESCAN);
> because it wants to defer rescanning, but doesn't actually df_finish_pass
> (it does in one case, but then calls df_set_flags with another changeable 
> flag,
> so it has the same issue), and if the IRA pass is invoked soon after it
> without any df_finish_pass calls in between, we end up with deferred insn
> rescanning during IRA which heavily relies on immediate insn rescanning.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

Maybe add a comment in case somebody wonders later?


> 2017-08-03  Jakub Jelinek  
> 
>   PR target/81621
>   * bb-reorder.c (pass_partition_blocks::execute): Return TODO_df_finish
>   after setting changeable df flags.
> 
>   * gcc.dg/pr81621.c: New test.
> 
> --- gcc/bb-reorder.c.jj   2017-07-21 10:28:13.0 +0200
> +++ gcc/bb-reorder.c  2017-08-02 19:43:58.797243254 +0200
> @@ -2904,7 +2904,7 @@ pass_partition_blocks::execute (function
>  
>crossing_edges = find_rarely_executed_basic_blocks_and_crossing_edges ();
>if (!crossing_edges.exists ())
> -return 0;
> +return TODO_df_finish;

I suppose we can avoid this if we move the df_set_flags after this?
I doubt find_rarely_executed_basic_blocks_and_crossing_edges modifies
anything.

Ok with those changes (if the latter is possible).

Thanks,
Richard.

>crtl->has_bb_partition = true;
>  
> @@ -2970,7 +2970,7 @@ pass_partition_blocks::execute (function
>df_analyze ();
>  }
>
> -  return 0;
> +  return TODO_df_finish;
>  }
>  
>  } // anon namespace
> --- gcc/testsuite/gcc.dg/pr81621.c.jj 2017-08-02 19:52:08.435831121 +0200
> +++ gcc/testsuite/gcc.dg/pr81621.c2017-08-02 19:52:00.026924067 +0200
> @@ -0,0 +1,5 @@
> +/* PR target/81621 */
> +/* { dg-do compile { target freorder } } */
> +/* { dg-options "-Og -fno-split-wide-types -freorder-blocks-and-partition" } 
> */
> +
> +#include "graphite/scop-10.c"
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

[PATCH] Fix df-related ICE due to bbpart pass bug (PR target/81621)

[Jakub Jelinek]

Hi!

The following testcase ICEs on s390x.  The problem is that the bbpart pass
calls
df_set_flags (DF_DEFER_INSN_RESCAN);
because it wants to defer rescanning, but doesn't actually df_finish_pass
(it does in one case, but then calls df_set_flags with another changeable flag,
so it has the same issue), and if the IRA pass is invoked soon after it
without any df_finish_pass calls in between, we end up with deferred insn
rescanning during IRA which heavily relies on immediate insn rescanning.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2017-08-03  Jakub Jelinek  

PR target/81621
* bb-reorder.c (pass_partition_blocks::execute): Return TODO_df_finish
after setting changeable df flags.

* gcc.dg/pr81621.c: New test.

--- gcc/bb-reorder.c.jj 2017-07-21 10:28:13.0 +0200
+++ gcc/bb-reorder.c2017-08-02 19:43:58.797243254 +0200
@@ -2904,7 +2904,7 @@ pass_partition_blocks::execute (function
 
   crossing_edges = find_rarely_executed_basic_blocks_and_crossing_edges ();
   if (!crossing_edges.exists ())
-return 0;
+return TODO_df_finish;
 
   crtl->has_bb_partition = true;
 
@@ -2970,7 +2970,7 @@ pass_partition_blocks::execute (function
   df_analyze ();
 }
 
-  return 0;
+  return TODO_df_finish;
 }
 
 } // anon namespace
--- gcc/testsuite/gcc.dg/pr81621.c.jj   2017-08-02 19:52:08.435831121 +0200
+++ gcc/testsuite/gcc.dg/pr81621.c  2017-08-02 19:52:00.026924067 +0200
@@ -0,0 +1,5 @@
+/* PR target/81621 */
+/* { dg-do compile { target freorder } } */
+/* { dg-options "-Og -fno-split-wide-types -freorder-blocks-and-partition" } */
+
+#include "graphite/scop-10.c"

Jakub