diff mbox series

middle-end: don't create LC-SSA PHI variables for PHI nodes who dominate loop

Message ID patch-17859-tamar@arm.com
State Not Applicable
Headers
Series middle-end: don't create LC-SSA PHI variables for PHI nodes who dominate loop |

Checks

Context Check Description
snail/gcc-patch-check fail Git am fail log

Commit Message

Tamar Christina Oct. 19, 2023, 12:29 p.m. UTC 
  Hi All,

As the testcase shows, when a PHI node dominates the loop there is no new
definition inside the loop.  As such there would be no PHI nodes to update.

When we maintain LCSSA form we create an intermediate node in between the two
loops to thread alongt the value.  However later on when we update the second
loop we don't have any PHI nodes to update and so adjust_phi_and_debug_stmts
does nothing.   This leaves us with an incorrect phi node.  Normally this does
nothing and just gets ignored.  But in the case of the vUSE chain we end up
corrupting the chain.

As such whenever a PHI node's argument dominates the loop, we should remove
the newly created PHI node after edge redirection.

The one exception to this is when the loop has been versioned.  In such cases
the versioned loop may not use the value but the second loop can.

When this happens and we add the loop guard unless the join block has the PHI
it can't find the original value for use inside the guard block.

The next refactoring in the series moves the formation of the guard block
inside peeling itself.  Here we have all the information and wouldn't
need to re-create it later.

Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-linux-gnu
and no issues and issues in libgomp fixed.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	PR tree-optimization/111860
	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
	Remove PHI nodes that dominate loop.

gcc/testsuite/ChangeLog:

	PR tree-optimization/111860
	* gcc.dg/vect/pr111860.c: New test.

--- inline copy of patch -- 
diff --git a/gcc/testsuite/gcc.dg/vect/pr111860.c b/gcc/testsuite/gcc.dg/vect/pr111860.c
new file mode 100644
index 0000000000000000000000000000000000000000..36f0774601040418bc6b7f27c9425b2bf93b18cb




--
diff --git a/gcc/testsuite/gcc.dg/vect/pr111860.c b/gcc/testsuite/gcc.dg/vect/pr111860.c
new file mode 100644
index 0000000000000000000000000000000000000000..36f0774601040418bc6b7f27c9425b2bf93b18cb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr111860.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+
+int optimize_path_n, optimize_path_d;
+int *optimize_path_d_0;
+extern void path_threeOpt( long);
+void optimize_path() {
+  int i;
+  long length;
+  i = 0;
+  for (; i <= optimize_path_n; i++)
+    optimize_path_d = 0;
+  i = 0;
+  for (; i < optimize_path_n; i++)
+    length += optimize_path_d_0[i];
+  path_threeOpt(length);
+}
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 1f7779b9834c3aef3c6a993fab916224fab03147..db1d4f867ead5c6079cda3ff0d0870234d11e39d 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -1633,6 +1633,21 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
 	{
 	  tree new_arg = gimple_phi_arg (phi, 0)->def;
 	  new_phi_args.put (new_arg, gimple_phi_result (phi));
+
+	  if (TREE_CODE (new_arg) != SSA_NAME)
+	    continue;
+	  /* If the PHI node dominates the loop then we shouldn't create
+	      a new LC-SSSA PHI for it in the intermediate block.  Unless the
+	      the loop has been versioned.  If it has then we need the PHI
+	      node such that later when the loop guard is added the original
+	      dominating PHI can be found.  */
+	  basic_block def_bb = gimple_bb (SSA_NAME_DEF_STMT (new_arg));
+	  if (loop == scalar_loop
+	      && (!def_bb || !flow_bb_inside_loop_p (loop, def_bb)))
+	    {
+	      auto gsi = gsi_for_stmt (phi);
+	      remove_phi_node (&gsi, true);
+	    }
 	}
 
       /* Copy the current loop LC PHI nodes between the original loop exit

Comments

Richard Biener Oct. 19, 2023, 12:31 p.m. UTC | #1 
On Thu, 19 Oct 2023, Tamar Christina wrote:

> Hi All,
> 
> As the testcase shows, when a PHI node dominates the loop there is no new
> definition inside the loop.  As such there would be no PHI nodes to update.
> 
> When we maintain LCSSA form we create an intermediate node in between the two
> loops to thread alongt the value.  However later on when we update the second
> loop we don't have any PHI nodes to update and so adjust_phi_and_debug_stmts
> does nothing.   This leaves us with an incorrect phi node.  Normally this does
> nothing and just gets ignored.  But in the case of the vUSE chain we end up
> corrupting the chain.
> 
> As such whenever a PHI node's argument dominates the loop, we should remove
> the newly created PHI node after edge redirection.
> 
> The one exception to this is when the loop has been versioned.  In such cases
> the versioned loop may not use the value but the second loop can.
> 
> When this happens and we add the loop guard unless the join block has the PHI
> it can't find the original value for use inside the guard block.
> 
> The next refactoring in the series moves the formation of the guard block
> inside peeling itself.  Here we have all the information and wouldn't
> need to re-create it later.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-linux-gnu
> and no issues and issues in libgomp fixed.
> 
> Ok for master?

OK.

Thanks,
Richard.

> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/111860
> 	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> 	Remove PHI nodes that dominate loop.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/111860
> 	* gcc.dg/vect/pr111860.c: New test.
> 
> --- inline copy of patch -- 
> diff --git a/gcc/testsuite/gcc.dg/vect/pr111860.c b/gcc/testsuite/gcc.dg/vect/pr111860.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..36f0774601040418bc6b7f27c9425b2bf93b18cb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr111860.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +
> +int optimize_path_n, optimize_path_d;
> +int *optimize_path_d_0;
> +extern void path_threeOpt( long);
> +void optimize_path() {
> +  int i;
> +  long length;
> +  i = 0;
> +  for (; i <= optimize_path_n; i++)
> +    optimize_path_d = 0;
> +  i = 0;
> +  for (; i < optimize_path_n; i++)
> +    length += optimize_path_d_0[i];
> +  path_threeOpt(length);
> +}
> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index 1f7779b9834c3aef3c6a993fab916224fab03147..db1d4f867ead5c6079cda3ff0d0870234d11e39d 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -1633,6 +1633,21 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
>  	{
>  	  tree new_arg = gimple_phi_arg (phi, 0)->def;
>  	  new_phi_args.put (new_arg, gimple_phi_result (phi));
> +
> +	  if (TREE_CODE (new_arg) != SSA_NAME)
> +	    continue;
> +	  /* If the PHI node dominates the loop then we shouldn't create
> +	      a new LC-SSSA PHI for it in the intermediate block.  Unless the
> +	      the loop has been versioned.  If it has then we need the PHI
> +	      node such that later when the loop guard is added the original
> +	      dominating PHI can be found.  */
> +	  basic_block def_bb = gimple_bb (SSA_NAME_DEF_STMT (new_arg));
> +	  if (loop == scalar_loop
> +	      && (!def_bb || !flow_bb_inside_loop_p (loop, def_bb)))
> +	    {
> +	      auto gsi = gsi_for_stmt (phi);
> +	      remove_phi_node (&gsi, true);
> +	    }
>  	}
>  
>        /* Copy the current loop LC PHI nodes between the original loop exit
> 
> 
> 
> 
>
diff mbox series

Patch

--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr111860.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+
+int optimize_path_n, optimize_path_d;
+int *optimize_path_d_0;
+extern void path_threeOpt( long);
+void optimize_path() {
+  int i;
+  long length;
+  i = 0;
+  for (; i <= optimize_path_n; i++)
+    optimize_path_d = 0;
+  i = 0;
+  for (; i < optimize_path_n; i++)
+    length += optimize_path_d_0[i];
+  path_threeOpt(length);
+}
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 1f7779b9834c3aef3c6a993fab916224fab03147..db1d4f867ead5c6079cda3ff0d0870234d11e39d 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -1633,6 +1633,21 @@  slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
 	{
 	  tree new_arg = gimple_phi_arg (phi, 0)->def;
 	  new_phi_args.put (new_arg, gimple_phi_result (phi));
+
+	  if (TREE_CODE (new_arg) != SSA_NAME)
+	    continue;
+	  /* If the PHI node dominates the loop then we shouldn't create
+	      a new LC-SSSA PHI for it in the intermediate block.  Unless the
+	      the loop has been versioned.  If it has then we need the PHI
+	      node such that later when the loop guard is added the original
+	      dominating PHI can be found.  */
+	  basic_block def_bb = gimple_bb (SSA_NAME_DEF_STMT (new_arg));
+	  if (loop == scalar_loop
+	      && (!def_bb || !flow_bb_inside_loop_p (loop, def_bb)))
+	    {
+	      auto gsi = gsi_for_stmt (phi);
+	      remove_phi_node (&gsi, true);
+	    }
 	}
 
       /* Copy the current loop LC PHI nodes between the original loop exit