[PR82163/V2] New interface checking LCSSA for single loop

Hi,
This is the V2 patch fixing PR82163.  It rewrites verify_loop_closed_ssa by checking
uses of all definitions inside of loop.  This gives advantage that we can check loop
closed ssa form for a specific loop, rather than for whole function.  The interface
is used in fixing this issue.
Bootstrap and test on x86_64, is it OK?

Thanks,
bin
2017-09-21  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/82163
	* tree-ssa-loop-manip.h (verify_loop_closed_ssa): New parameter.
	(checking_verify_loop_closed_ssa): New parameter.
	* tree-ssa-loop-manip.c (check_loop_closed_ssa_use): Delete.
	(check_loop_closed_ssa_stmt): Delete.
	(check_loop_closed_ssa_def, check_loop_closed_ssa_bb): New functions.
	(verify_loop_closed_ssa): Check loop closed ssa form for LOOP.
	(tree_transform_and_unroll_loop): Check loop closed ssa form only for
	changed loops.

gcc/testsuite
2017-09-21  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/82163
	* gcc.dg/tree-ssa/pr82163.c: New test.
From ef756285db3685bd97bbe7d144d58af477251416 Mon Sep 17 00:00:00 2001
From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com>
Date: Thu, 21 Sep 2017 12:41:32 +0100
Subject: [PATCH] pr82163-20170921.txt

---
 gcc/testsuite/gcc.dg/tree-ssa/pr82163.c | 23 +++++++++
 gcc/tree-ssa-loop-manip.c               | 91 +++++++++++++++++++--------------
 gcc/tree-ssa-loop-manip.h               |  6 +--
 3 files changed, 80 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr82163.c

On Fri, Sep 22, 2017 at 1:37 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> This is the V2 patch fixing PR82163.  It rewrites verify_loop_closed_ssa by checking
> uses of all definitions inside of loop.  This gives advantage that we can check loop
> closed ssa form for a specific loop, rather than for whole function.  The interface
> is used in fixing this issue.
> Bootstrap and test on x86_64, is it OK?

Looks good to me.

Thanks,
Richard.

> Thanks,
> bin
> 2017-09-21  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/82163
>         * tree-ssa-loop-manip.h (verify_loop_closed_ssa): New parameter.
>         (checking_verify_loop_closed_ssa): New parameter.
>         * tree-ssa-loop-manip.c (check_loop_closed_ssa_use): Delete.
>         (check_loop_closed_ssa_stmt): Delete.
>         (check_loop_closed_ssa_def, check_loop_closed_ssa_bb): New functions.
>         (verify_loop_closed_ssa): Check loop closed ssa form for LOOP.
>         (tree_transform_and_unroll_loop): Check loop closed ssa form only for
>         changed loops.
>
> gcc/testsuite
> 2017-09-21  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/82163
>         * gcc.dg/tree-ssa/pr82163.c: New test.

On Fri, Sep 22, 2017 at 11:37:53AM +0000, Bin Cheng wrote:

> diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
> index d6ba305..6ad0b75 100644
> --- a/gcc/tree-ssa-loop-manip.c
> +++ b/gcc/tree-ssa-loop-manip.c
> @@ -690,48 +690,62 @@ rewrite_virtuals_into_loop_closed_ssa (struct loop *loop)
>    rewrite_into_loop_closed_ssa_1 (NULL, 0, SSA_OP_VIRTUAL_USES, loop);
>  }

> -/* Checks invariants of loop closed ssa form in statement STMT in BB.  */
> +/* Checks invariants of loop closed ssa form in BB.  */
>  
>  static void
> -check_loop_closed_ssa_stmt (basic_block bb, gimple *stmt)
> +check_loop_closed_ssa_bb (basic_block bb)
>  {
> -  ssa_op_iter iter;
> -  tree var;
> +  for (gphi_iterator bsi = gsi_start_phis (bb); !gsi_end_p (bsi);
> +       gsi_next (&bsi))
> +    {
> +      gphi *phi = bsi.phi ();
>  
> -  if (is_gimple_debug (stmt))
> -    return;
> +      if (!virtual_operand_p (PHI_RESULT (phi)))
> +	check_loop_closed_ssa_def (bb, PHI_RESULT (phi));
> +    }
> +
> +  for (gimple_stmt_iterator bsi = gsi_start_bb (bb); !gsi_end_p (bsi);
> +       gsi_next (&bsi))
> +    {
> +      ssa_op_iter iter;
> +      tree var;
> +      gimple *stmt = gsi_stmt (bsi);
> +
> +      if (is_gimple_debug (stmt))
> +	continue;

for (gimple_stmt_iterator bsi = gsi_start_nondebug_after_labels_bb (bb);
     !gsi_end_p (bsi);
     gsi_next_nondebug (&bsi))

?
>  
> -  FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_USE)
> -    check_loop_closed_ssa_use (bb, var);
> +      FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_DEF)
> +	check_loop_closed_ssa_def (bb, var);
> +    }
>  }

On Sat, Sep 23, 2017 at 6:31 PM, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On Fri, Sep 22, 2017 at 11:37:53AM +0000, Bin Cheng wrote:
>
>> diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
>> index d6ba305..6ad0b75 100644
>> --- a/gcc/tree-ssa-loop-manip.c
>> +++ b/gcc/tree-ssa-loop-manip.c
>> @@ -690,48 +690,62 @@ rewrite_virtuals_into_loop_closed_ssa (struct loop *loop)
>>    rewrite_into_loop_closed_ssa_1 (NULL, 0, SSA_OP_VIRTUAL_USES, loop);
>>  }
>
>> -/* Checks invariants of loop closed ssa form in statement STMT in BB.  */
>> +/* Checks invariants of loop closed ssa form in BB.  */
>>
>>  static void
>> -check_loop_closed_ssa_stmt (basic_block bb, gimple *stmt)
>> +check_loop_closed_ssa_bb (basic_block bb)
>>  {
>> -  ssa_op_iter iter;
>> -  tree var;
>> +  for (gphi_iterator bsi = gsi_start_phis (bb); !gsi_end_p (bsi);
>> +       gsi_next (&bsi))
>> +    {
>> +      gphi *phi = bsi.phi ();
>>
>> -  if (is_gimple_debug (stmt))
>> -    return;
>> +      if (!virtual_operand_p (PHI_RESULT (phi)))
>> +     check_loop_closed_ssa_def (bb, PHI_RESULT (phi));
>> +    }
>> +
>> +  for (gimple_stmt_iterator bsi = gsi_start_bb (bb); !gsi_end_p (bsi);
>> +       gsi_next (&bsi))
>> +    {
>> +      ssa_op_iter iter;
>> +      tree var;
>> +      gimple *stmt = gsi_stmt (bsi);
>> +
>> +      if (is_gimple_debug (stmt))
>> +     continue;
>
> for (gimple_stmt_iterator bsi = gsi_start_nondebug_after_labels_bb (bb);
>      !gsi_end_p (bsi);
>      gsi_next_nondebug (&bsi))
>
> ?
Thanks for the suggestion, patch updated.  I will commit it later
since it's an obvious update.

Thanks,
bin
>>
>> -  FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_USE)
>> -    check_loop_closed_ssa_use (bb, var);
>> +      FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_DEF)
>> +     check_loop_closed_ssa_def (bb, var);
>> +    }
>>  }
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr82163.c b/gcc/testsuite/gcc.dg/tree-ssa/pr82163.c
new file mode 100644
index 0000000..389d5c3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr82163.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+int a, b, c[4], d, e, f, g;
+
+void h ()
+{
+  for (; a; a++)
+    {
+      c[a + 3] = g;
+      if (b)
+        c[a] = f;
+      else
+        {
+          for (; d; d++)
+            c[d + 3] = c[d];
+          for (e = 1; e == 2; e++)
+            ;
+          if (e)
+            break;
+        }
+    }
+}
diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index d6ba305..b08b8b9 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -690,48 +690,59 @@ rewrite_virtuals_into_loop_closed_ssa (struct loop *loop)
   rewrite_into_loop_closed_ssa_1 (NULL, 0, SSA_OP_VIRTUAL_USES, loop);
 }
 
-/* Check invariants of the loop closed ssa form for the USE in BB.  */
+/* Check invariants of the loop closed ssa form for the def in DEF_BB.  */
 
 static void
-check_loop_closed_ssa_use (basic_block bb, tree use)
+check_loop_closed_ssa_def (basic_block def_bb, tree def)
 {
-  gimple *def;
-  basic_block def_bb;
+  use_operand_p use_p;
+  imm_use_iterator iterator;
+  FOR_EACH_IMM_USE_FAST (use_p, iterator, def)
+    {
+      if (is_gimple_debug (USE_STMT (use_p)))
+	continue;
 
-  if (TREE_CODE (use) != SSA_NAME || virtual_operand_p (use))
-    return;
+      basic_block use_bb = gimple_bb (USE_STMT (use_p));
+      if (is_a <gphi *> (USE_STMT (use_p)))
+	use_bb = EDGE_PRED (use_bb, PHI_ARG_INDEX_FROM_USE (use_p))->src;
 
-  def = SSA_NAME_DEF_STMT (use);
-  def_bb = gimple_bb (def);
-  gcc_assert (!def_bb
-	      || flow_bb_inside_loop_p (def_bb->loop_father, bb));
+      gcc_assert (flow_bb_inside_loop_p (def_bb->loop_father, use_bb));
+    }
 }
 
-/* Checks invariants of loop closed ssa form in statement STMT in BB.  */
+/* Checks invariants of loop closed ssa form in BB.  */
 
 static void
-check_loop_closed_ssa_stmt (basic_block bb, gimple *stmt)
+check_loop_closed_ssa_bb (basic_block bb)
 {
-  ssa_op_iter iter;
-  tree var;
+  for (gphi_iterator bsi = gsi_start_phis (bb); !gsi_end_p (bsi);
+       gsi_next (&bsi))
+    {
+      gphi *phi = bsi.phi ();
 
-  if (is_gimple_debug (stmt))
-    return;
+      if (!virtual_operand_p (PHI_RESULT (phi)))
+	check_loop_closed_ssa_def (bb, PHI_RESULT (phi));
+    }
 
-  FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_USE)
-    check_loop_closed_ssa_use (bb, var);
+  for (gimple_stmt_iterator bsi = gsi_start_nondebug_bb (bb); !gsi_end_p (bsi);
+       gsi_next_nondebug (&bsi))
+    {
+      ssa_op_iter iter;
+      tree var;
+      gimple *stmt = gsi_stmt (bsi);
+
+      FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_DEF)
+	check_loop_closed_ssa_def (bb, var);
+    }
 }
 
 /* Checks that invariants of the loop closed ssa form are preserved.
-   Call verify_ssa when VERIFY_SSA_P is true.  */
+   Call verify_ssa when VERIFY_SSA_P is true.  Note all loops are checked
+   if LOOP is NULL, otherwise, only LOOP is checked.  */
 
 DEBUG_FUNCTION void
-verify_loop_closed_ssa (bool verify_ssa_p)
+verify_loop_closed_ssa (bool verify_ssa_p, struct loop *loop)
 {
-  basic_block bb;
-  edge e;
-  edge_iterator ei;
-
   if (number_of_loops (cfun) <= 1)
     return;
 
@@ -740,20 +751,22 @@ verify_loop_closed_ssa (bool verify_ssa_p)
 
   timevar_push (TV_VERIFY_LOOP_CLOSED);
 
-  FOR_EACH_BB_FN (bb, cfun)
+  if (loop == NULL)
     {
-      for (gphi_iterator bsi = gsi_start_phis (bb); !gsi_end_p (bsi);
-	   gsi_next (&bsi))
-	{
-	  gphi *phi = bsi.phi ();
-	  FOR_EACH_EDGE (e, ei, bb->preds)
-	    check_loop_closed_ssa_use (e->src,
-				       PHI_ARG_DEF_FROM_EDGE (phi, e));
-	}
+      basic_block bb;
 
-      for (gimple_stmt_iterator bsi = gsi_start_bb (bb); !gsi_end_p (bsi);
-	   gsi_next (&bsi))
-	check_loop_closed_ssa_stmt (bb, gsi_stmt (bsi));
+      FOR_EACH_BB_FN (bb, cfun)
+	if (bb->loop_father && bb->loop_father->num > 0)
+	  check_loop_closed_ssa_bb (bb);
+    }
+  else
+    {
+      basic_block *bbs = get_loop_body (loop);
+
+      for (unsigned i = 0; i < loop->num_nodes; ++i)
+	check_loop_closed_ssa_bb (bbs[i]);
+
+      free (bbs);
     }
 
   timevar_pop (TV_VERIFY_LOOP_CLOSED);
@@ -1405,7 +1418,8 @@ tree_transform_and_unroll_loop (struct loop *loop, unsigned factor,
 
   checking_verify_flow_info ();
   checking_verify_loop_structure ();
-  checking_verify_loop_closed_ssa (true);
+  checking_verify_loop_closed_ssa (true, loop);
+  checking_verify_loop_closed_ssa (true, new_loop);
 }
 
 /* Wrapper over tree_transform_and_unroll_loop for case we do not
diff --git a/gcc/tree-ssa-loop-manip.h b/gcc/tree-ssa-loop-manip.h
index a139050..3f5b3ee 100644
--- a/gcc/tree-ssa-loop-manip.h
+++ b/gcc/tree-ssa-loop-manip.h
@@ -28,13 +28,13 @@ extern void rewrite_into_loop_closed_ssa_1 (bitmap, unsigned, int,
 					    struct loop *);
 extern void rewrite_into_loop_closed_ssa (bitmap, unsigned);
 extern void rewrite_virtuals_into_loop_closed_ssa (struct loop *);
-extern void verify_loop_closed_ssa (bool);
+extern void verify_loop_closed_ssa (bool, struct loop * = NULL);
 
 static inline void
-checking_verify_loop_closed_ssa (bool verify_ssa_p)
+checking_verify_loop_closed_ssa (bool verify_ssa_p, struct loop *loop = NULL)
 {
   if (flag_checking)
-    verify_loop_closed_ssa (verify_ssa_p);
+    verify_loop_closed_ssa (verify_ssa_p, loop);
 }
 
 extern basic_block split_loop_exit_edge (edge);