Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs

[Tom de Vries <Tom_deVries at mentor dot com>]

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  <tom@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