* [PATCH] Fix PR81354 (SLSR, insert on edges)
@ 2017-08-07 18:51 Bill Schmidt
2017-08-08 10:49 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Bill Schmidt @ 2017-08-07 18:51 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener
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.
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;
+ }
+ }
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix PR81354 (SLSR, insert on edges)
2017-08-07 18:51 [PATCH] Fix PR81354 (SLSR, insert on edges) Bill Schmidt
@ 2017-08-08 10:49 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-08-08 10:49 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches
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;
> + }
> + }
> +}
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-08-08 10:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 18:51 [PATCH] Fix PR81354 (SLSR, insert on edges) Bill Schmidt
2017-08-08 10:49 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).