Fix PR81354 (SLSR, insert on edges)

Hi,

PR81354 describes an ICE in SLSR that occurs when a GIMPLE PHI statement changes
address during the SLSR pass.  SLSR contains a mapping table from GIMPLE statements
to entries in the candidate table that relies on this address remaining constant.

The address change occurs during gimple_split_edge (), as the PHIs in the dest
block of the edge first have their number of predecessors grow by 1 and then
shrink back to their original size.  The growth causes the PHIs to usually be
reallocated.  We've discussed some attempts to fix this in gimple_split_edge,
but the simplest solution is to fix this directly in SLSR by deferring the splitting
of the edge until the end of the pass, using gsi_insert_on_edge and
gsi_commit_edge_inserts.  That's what this patch does.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is this ok
for trunk, and for backports to GCC 5, 6, and 7 after burn-in?

Thanks,
Bill


[gcc]

2017-08-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/81354
	* gimple-ssa-strength-reduction.c (create_add_on_incoming_edge):
	Insert on edges rather than explicitly creating landing pads.
	(analyze_candidates_and_replace): Commit edge inserts.


[gcc/testsuite]

2017-08-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/81354
	* g++.dg/torture/pr81354.C: New file.

On Mon, Aug 7, 2017 at 8:51 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> PR81354 describes an ICE in SLSR that occurs when a GIMPLE PHI statement changes
> address during the SLSR pass.  SLSR contains a mapping table from GIMPLE statements
> to entries in the candidate table that relies on this address remaining constant.
>
> The address change occurs during gimple_split_edge (), as the PHIs in the dest
> block of the edge first have their number of predecessors grow by 1 and then
> shrink back to their original size.  The growth causes the PHIs to usually be
> reallocated.  We've discussed some attempts to fix this in gimple_split_edge,
> but the simplest solution is to fix this directly in SLSR by deferring the splitting
> of the edge until the end of the pass, using gsi_insert_on_edge and
> gsi_commit_edge_inserts.  That's what this patch does.
>
> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is this ok
> for trunk, and for backports to GCC 5, 6, and 7 after burn-in?

Ok.

Thanks,
Richard.

> Thanks,
> Bill
>
>
> [gcc]
>
> 2017-08-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR tree-optimization/81354
>         * gimple-ssa-strength-reduction.c (create_add_on_incoming_edge):
>         Insert on edges rather than explicitly creating landing pads.
>         (analyze_candidates_and_replace): Commit edge inserts.
>
>
> [gcc/testsuite]
>
> 2017-08-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR tree-optimization/81354
>         * g++.dg/torture/pr81354.C: New file.
>
>
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c (revision 250791)
> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
> @@ -2224,8 +2224,6 @@ create_add_on_incoming_edge (slsr_cand_t c, tree b
>                              widest_int increment, edge e, location_t loc,
>                              bool known_stride)
>  {
> -  basic_block insert_bb;
> -  gimple_stmt_iterator gsi;
>    tree lhs, basis_type;
>    gassign *new_stmt, *cast_stmt = NULL;
>
> @@ -2294,39 +2292,25 @@ create_add_on_incoming_edge (slsr_cand_t c, tree b
>        }
>      }
>
> -  insert_bb = single_succ_p (e->src) ? e->src : split_edge (e);
> -  gsi = gsi_last_bb (insert_bb);
> -
> -  if (!gsi_end_p (gsi) && stmt_ends_bb_p (gsi_stmt (gsi)))
> +  if (cast_stmt)
>      {
> -      gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
> -      if (cast_stmt)
> -       {
> -         gsi_insert_before (&gsi, cast_stmt, GSI_SAME_STMT);
> -         gimple_set_location (cast_stmt, loc);
> -       }
> +      gimple_set_location (cast_stmt, loc);
> +      gsi_insert_on_edge (e, cast_stmt);
>      }
> -  else
> -    {
> -      if (cast_stmt)
> -       {
> -         gsi_insert_after (&gsi, cast_stmt, GSI_NEW_STMT);
> -         gimple_set_location (cast_stmt, loc);
> -       }
> -      gsi_insert_after (&gsi, new_stmt, GSI_NEW_STMT);
> -    }
>
>    gimple_set_location (new_stmt, loc);
> +  gsi_insert_on_edge (e, new_stmt);
>
>    if (dump_file && (dump_flags & TDF_DETAILS))
>      {
>        if (cast_stmt)
>         {
> -         fprintf (dump_file, "Inserting cast in block %d: ",
> -                  insert_bb->index);
> +         fprintf (dump_file, "Inserting cast on edge %d->%d: ",
> +                  e->src->index, e->dest->index);
>           print_gimple_stmt (dump_file, cast_stmt, 0);
>         }
> -      fprintf (dump_file, "Inserting in block %d: ", insert_bb->index);
> +      fprintf (dump_file, "Inserting on edge %d->%d: ", e->src->index,
> +              e->dest->index);
>        print_gimple_stmt (dump_file, new_stmt, 0);
>      }
>
> @@ -3770,6 +3754,10 @@ analyze_candidates_and_replace (void)
>           free (incr_vec);
>         }
>      }
> +
> +  /* For conditional candidates, we may have uncommitted insertions
> +     on edges to clean up.  */
> +  gsi_commit_edge_inserts ();
>  }
>
>  namespace {
> Index: gcc/testsuite/g++.dg/torture/pr81354.C
> ===================================================================
> --- gcc/testsuite/g++.dg/torture/pr81354.C      (nonexistent)
> +++ gcc/testsuite/g++.dg/torture/pr81354.C      (working copy)
> @@ -0,0 +1,24 @@
> +// PR81354 reported this test as crashing in a limited range of revisions.
> +// { dg-do compile }
> +
> +struct T { double a; double b; };
> +
> +void foo(T Ad[], int As[2])
> +{
> +  int j;
> +  int i;
> +  int Bs[2] = {0,0};
> +  T Bd[16];
> +
> +  for (j = 0; j < 4; j++) {
> +    for (i = 0; i + 1 <= j + 1; i++) {
> +      Ad[i + As[0] * j] = Bd[i + Bs[0] * j];
> +    }
> +
> +    i = j + 1;  // <- comment out this line and it does not crash
> +    for (; i + 1 < 5; i++) {
> +      Ad[i + As[0] * j].a = 0.0;
> +      Ad[i + As[0] * j].b = 0.0;
> +    }
> +  }
> +}
>