Re: [PATCH] tree-optimization/106722 - fix CD-DCE edge marking

Re: [PATCH] tree-optimization/106722 - fix CD-DCE edge marking

[Jeff Law]

On 2/10/23 03:12, Richard Biener via Gcc-patches wrote:
> The following fixes a latent issue when we mark control edges but
> end up with marking a block with no stmts necessary.  In this case
> we fail to mark dependent control edges of that block.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Does this look OK?
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/106722
> 	* tree-ssa-dce.cc (mark_last_stmt_necessary): Return
> 	whether we marked a stmt.
> 	(mark_control_dependent_edges_necessary): When
> 	mark_last_stmt_necessary didn't mark any stmt make sure
> 	to mark its control dependent edges.
> 	(propagate_necessity): Likewise.
> 
> 	* gcc.dg/torture/pr108737.c: New testcase.

> diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
> index b2fe9f4f55e..21b3294fc86 100644
> --- a/gcc/tree-ssa-dce.cc
> +++ b/gcc/tree-ssa-dce.cc
> @@ -327,17 +327,23 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
>   
>   /* Mark the last statement of BB as necessary.  */
>   
> -static void
> +static bool
Function comment probably needs an update for the new return value.

OK with that fix.

jeff

Re: [PATCH] tree-optimization/106722 - fix CD-DCE edge marking

[Jan Hubicka]

> The following fixes a latent issue when we mark control edges but
> end up with marking a block with no stmts necessary.  In this case
> we fail to mark dependent control edges of that block.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Does this look OK?
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/106722
> 	* tree-ssa-dce.cc (mark_last_stmt_necessary): Return
> 	whether we marked a stmt.
> 	(mark_control_dependent_edges_necessary): When
> 	mark_last_stmt_necessary didn't mark any stmt make sure
> 	to mark its control dependent edges.
> 	(propagate_necessity): Likewise.
> 
> 	* gcc.dg/torture/pr108737.c: New testcase.
> diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
> index b2fe9f4f55e..21b3294fc86 100644
> --- a/gcc/tree-ssa-dce.cc
> +++ b/gcc/tree-ssa-dce.cc
> @@ -327,17 +327,23 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
>  
>  /* Mark the last statement of BB as necessary.  */
>  
> -static void
> +static bool
>  mark_last_stmt_necessary (basic_block bb)
>  {
>    gimple *stmt = last_stmt (bb);
>  
> -  bitmap_set_bit (last_stmt_necessary, bb->index);
> +  if (!bitmap_set_bit (last_stmt_necessary, bb->index))
> +    return true;
> +
>    bitmap_set_bit (bb_contains_live_stmts, bb->index);
>  
>    /* We actually mark the statement only if it is a control statement.  */
>    if (stmt && is_ctrl_stmt (stmt))
> -    mark_stmt_necessary (stmt, true);
> +    {
> +      mark_stmt_necessary (stmt, true);
> +      return true;
> +    }
> +  return false;
>  }
>  
>  
> @@ -369,8 +375,8 @@ mark_control_dependent_edges_necessary (basic_block bb, bool ignore_self)
>  	  continue;
>  	}
>  
> -      if (!bitmap_bit_p (last_stmt_necessary, cd_bb->index))
> -	mark_last_stmt_necessary (cd_bb);
> +      if (!mark_last_stmt_necessary (cd_bb))
> +	mark_control_dependent_edges_necessary (cd_bb, false);

Makes sense to me, though I am bit surprised it took such a long time to
show up.

Honza

[PATCH] tree-optimization/106722 - fix CD-DCE edge marking

[Richard Biener]

The following fixes a latent issue when we mark control edges but
end up with marking a block with no stmts necessary.  In this case
we fail to mark dependent control edges of that block.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Does this look OK?

Thanks,
Richard.

	PR tree-optimization/106722
	* tree-ssa-dce.cc (mark_last_stmt_necessary): Return
	whether we marked a stmt.
	(mark_control_dependent_edges_necessary): When
	mark_last_stmt_necessary didn't mark any stmt make sure
	to mark its control dependent edges.
	(propagate_necessity): Likewise.

	* gcc.dg/torture/pr108737.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr108737.c | 19 +++++++++++++++++++
 gcc/tree-ssa-dce.cc                     | 20 +++++++++++++-------
 2 files changed, 32 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr108737.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr108737.c b/gcc/testsuite/gcc.dg/torture/pr108737.c
new file mode 100644
index 00000000000..c8388bcabeb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr108737.c
@@ -0,0 +1,19 @@
+/* { dg-do run } */
+
+extern void exit (int);
+extern void abort (void);
+
+void __attribute__((noipa)) foo () { exit (0); }
+
+void __attribute__((noipa)) blah (int x)
+{
+  while (1) {
+      if(x) foo();
+  }
+}
+
+int main()
+{
+  blah (1);
+  abort ();
+}
diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index b2fe9f4f55e..21b3294fc86 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -327,17 +327,23 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
 
 /* Mark the last statement of BB as necessary.  */
 
-static void
+static bool
 mark_last_stmt_necessary (basic_block bb)
 {
   gimple *stmt = last_stmt (bb);
 
-  bitmap_set_bit (last_stmt_necessary, bb->index);
+  if (!bitmap_set_bit (last_stmt_necessary, bb->index))
+    return true;
+
   bitmap_set_bit (bb_contains_live_stmts, bb->index);
 
   /* We actually mark the statement only if it is a control statement.  */
   if (stmt && is_ctrl_stmt (stmt))
-    mark_stmt_necessary (stmt, true);
+    {
+      mark_stmt_necessary (stmt, true);
+      return true;
+    }
+  return false;
 }
 
 
@@ -369,8 +375,8 @@ mark_control_dependent_edges_necessary (basic_block bb, bool ignore_self)
 	  continue;
 	}
 
-      if (!bitmap_bit_p (last_stmt_necessary, cd_bb->index))
-	mark_last_stmt_necessary (cd_bb);
+      if (!mark_last_stmt_necessary (cd_bb))
+	mark_control_dependent_edges_necessary (cd_bb, false);
     }
 
   if (!skipped)
@@ -790,8 +796,8 @@ propagate_necessity (bool aggressive)
 		  if (gimple_bb (stmt)
 		      != get_immediate_dominator (CDI_POST_DOMINATORS, arg_bb))
 		    {
-		      if (!bitmap_bit_p (last_stmt_necessary, arg_bb->index))
-			mark_last_stmt_necessary (arg_bb);
+		      if (!mark_last_stmt_necessary (arg_bb))
+			mark_control_dependent_edges_necessary (arg_bb, false);
 		    }
 		  else if (arg_bb != ENTRY_BLOCK_PTR_FOR_FN (cfun)
 		           && !bitmap_bit_p (visited_control_parents,
-- 
2.35.3