On 03/12/2018 01:14 PM, Richard Biener wrote:
On Thu, 22 Feb 2018, Tom de Vries wrote:
I can
rework the bit of the patch that adds an assert after canonicalize_loop_ivs
into a patch that aborts the optimization instead of ICE-ing.
I think that's something reasonable anyway.
Patch attached below implements that approach.
The difference between before parloops, and after ompexpssa2 is this
(showing the effects of canonicalize_loop_ivs):
...
$ diff -u pr83126.c.156t.dce5 pr83126.c.158t.ompexpssa2
...
ew (short unsigned int c9, int stuff)
{
int * fd;
@@ -9,53 +103,53 @@
int _3;
short unsigned int _4;
unsigned int _5;
+ unsigned int ivtmp_8;
unsigned int _22;
unsigned int ivtmp_24;
unsigned int ivtmp_26;
+ unsigned int ivtmp_28;
<bb 2> [local count: 11811]:
<bb 3> [local count: 118111601]:
- # c9_1 = PHI <c9_11(D)(2), c9_21(11)>
+ # c9_1 = PHI <c9_11(D)(2), c9_21(7)>
_2 = (long int) c9_1;
fd_13 = (int *) _2;
_3 = *fd_13;
<bb 4> [local count: 1073741825]:
if (_3 != 0)
- goto <bb 6>; [11.00%]
+ goto <bb 15>; [11.00%]
else
goto <bb 9>; [89.00%]
<bb 9> [local count: 955630225]:
goto <bb 4>; [100.00%]
- <bb 6> [local count: 118111600]:
+ <bb 15> [local count: 94489281]:
- <bb 10> [local count: 118111600]:
-
- <bb 5> [local count: 236258638]:
- # _22 = PHI <0(10), _5(8)>
- # c9_23 = PHI <c9_1(10), c9_14(8)>
- # e1_27 = PHI <0(10), e1_17(8)>
- # ivtmp_26 = PHI <2(10), ivtmp_24(8)>
+ <bb 5> [local count: 189006911]:
+ # c9_23 = PHI <c9_14(8), c9_1(15)>
+ # e1_27 = PHI <e1_17(8), 0(15)>
+ # ivtmp_8 = PHI <ivtmp_28(8), 0(15)>
+ _22 = ivtmp_8;
+ ivtmp_26 = 2 - ivtmp_8;
_4 = (short unsigned int) e1_27;
c9_14 = _4 * c9_23;
_5 = _22 + 1;
e1_17 = (int) _5;
ivtmp_24 = ivtmp_26 - 1;
- if (ivtmp_24 != 0)
+ if (ivtmp_8 < 1)
goto <bb 8>; [66.67%]
else
goto <bb 7>; [33.33%]
- <bb 8> [local count: 157513634]:
+ <bb 8> [local count: 126010908]:
+ ivtmp_28 = ivtmp_8 + 1;
goto <bb 5>; [100.00%]
<bb 7> [local count: 78745004]:
# c9_21 = PHI <c9_14(5)>
-
- <bb 11> [local count: 78745004]:
goto <bb 3>; [100.00%]
}
...
Bootstrapped and reg-tested on x86_64.
OK for stage4 trunk?
Thanks,
- Tom
[parloops] Handle canonicalize_loop_ivs failure
2018-03-19 Tom de Vries <t...@codesourcery.com>
PR tree-optimization/83126
* tree-parloops.c (num_phis): New function.
(gen_parallel_loop): Detect and handle canonicalize_loop_ivs failure.
* gcc.dg/graphite/pr83126.c: New test.
---
gcc/testsuite/gcc.dg/graphite/pr83126.c | 19 ++++++++++++++++
gcc/tree-parloops.c | 39 +++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/gcc/testsuite/gcc.dg/graphite/pr83126.c b/gcc/testsuite/gcc.dg/graphite/pr83126.c
new file mode 100644
index 0000000..663d059
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/pr83126.c
@@ -0,0 +1,19 @@
+/* { dg-additional-options "-w -ftree-parallelize-loops=2 -floop-parallelize-all -O1" } */
+
+void
+ew (unsigned short int c9, int stuff)
+{
+ int e1;
+
+ for (;;)
+ {
+ unsigned int *by = &e1;
+ int *fd = &stuff;
+
+ *fd = c9;
+ fd = *fd;
+ if (*fd != 0)
+ for (*by = 0; *by < 2; ++*by)
+ c9 *= e1;
+ }
+}
diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c
index e44ad5e..3a788cc 100644
--- a/gcc/tree-parloops.c
+++ b/gcc/tree-parloops.c
@@ -2235,6 +2235,25 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
calculate_dominance_info (CDI_DOMINATORS);
}
+/* Return number of phis in bb. If COUNT_VIRTUAL_P is false, don't count the
+ virtual phi. */
+
+static unsigned int
+num_phis (basic_block bb, bool count_virtual_p)
+{
+ unsigned int nr_phis = 0;
+ gphi_iterator gsi;
+ for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ if (!count_virtual_p && virtual_operand_p (PHI_RESULT (gsi.phi ())))
+ continue;
+
+ nr_phis++;
+ }
+
+ return nr_phis;
+}
+
/* Generates code to execute the iterations of LOOP in N_THREADS
threads in parallel, which can be 0 if that number is to be determined
later.
@@ -2370,6 +2389,26 @@ gen_parallel_loop (struct loop *loop,
/* Base all the induction variables in LOOP on a single control one. */
canonicalize_loop_ivs (loop, &nit, true);
+ if (num_phis (loop->header, false) != reduction_list->elements () + 1)
+ {
+ /* The call to canonicalize_loop_ivs above failed to "base all the
+ induction variables in LOOP on a single control one". Do damage
+ control. */
+ basic_block preheader = loop_preheader_edge (loop)->src;
+ basic_block cond_bb = single_pred (preheader);
+ gcond *cond = as_a <gcond *> (gsi_stmt (gsi_last_bb (cond_bb)));
+ gimple_cond_make_true (cond);
+ update_stmt (cond);
+ /* We've gotten rid of the duplicate loop created by loop_version, but
+ we can't undo whatever canonicalize_loop_ivs has done.
+ TODO: Fix this properly by ensuring that the call to
+ canonicalize_loop_ivs succeeds. */
+ if (dump_file
+ && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, "canonicalize_loop_ivs failed for loop %d,"
+ " aborting transformation\n", loop->num);
+ return;
+ }
/* Ensure that the exit condition is the first statement in the loop.
The common case is that latch of the loop is empty (apart from the
