diff mbox series

tree-optimization/116906 - unsafe PRE with never executed edges

Message ID 20241001094401.3D3FC38449DA@sourceware.org
State New
Headers show
Series tree-optimization/116906 - unsafe PRE with never executed edges | expand

Commit Message

Richard Biener Oct. 1, 2024, 9:43 a.m. UTC 
When we're computing ANTIC for PRE we treat edges to not yet visited
blocks as having a maximum ANTIC solution to get at an optimistic
solution in the iteration.  That assumes the edges visted eventually
execute.  This is a wrong assumption that can lead to wrong code
(and not only non-optimality) when possibly trapping expressions
are involved as the testcases in the PR show.  The following mitigates
this by pruning trapping expressions from ANTIC computed when
maximum sets are involved.

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

	PR tree-optimization/116906
	* tree-ssa-pre.cc (prune_clobbered_mems): Add clean_traps
	argument.
	(compute_antic_aux): Direct prune_clobbered_mems to prune
	all traps when any MAX solution was involved in the ANTIC
	computation.
	(compute_partial_antic_aux): Adjust.

	* gcc.dg/pr116906-1.c: New testcase.
	* gcc.dg/pr116906-2.c: Likewise.
---
 gcc/testsuite/gcc.dg/pr116906-1.c | 43 +++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/pr116906-2.c | 40 ++++++++++++++++++++++++++++
 gcc/tree-ssa-pre.cc               | 16 +++++++-----
 3 files changed, 93 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr116906-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr116906-2.c
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/pr116906-1.c b/gcc/testsuite/gcc.dg/pr116906-1.c
new file mode 100644
index 00000000000..27b1fdae02b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr116906-1.c
@@ -0,0 +1,43 @@ 
+/* { dg-do run { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
+/* { dg-options "-O2" } */
+
+#include <stdint.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+int a = 1, b = 0;
+
+uint64_t safe_mod(uint64_t a, uint64_t b)
+{
+    if (b == 0) return a;
+    else return a % b;
+}
+
+int __attribute__((noipa))
+f(uint64_t p)
+{
+    int c = 0;
+j:
+    b = safe_mod(
+        (c = ((a &= (0 < p)) && 1), 1), p);
+    if (!c)
+        goto j;
+    return 0;
+}
+
+void do_exit (int i)
+{
+  exit (0);
+}
+
+int main()
+{
+  struct sigaction s;
+  sigemptyset (&s.sa_mask);
+  s.sa_handler = do_exit;
+  s.sa_flags = 0;
+  sigaction (SIGALRM, &s, NULL);
+  alarm (1);
+  f(b);
+}
diff --git a/gcc/testsuite/gcc.dg/pr116906-2.c b/gcc/testsuite/gcc.dg/pr116906-2.c
new file mode 100644
index 00000000000..3478771678c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr116906-2.c
@@ -0,0 +1,40 @@ 
+/* { dg-do run { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
+/* { dg-options "-O2 -fno-tree-ch" } */
+
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+int x;
+
+void __attribute__((noipa))
+foo (int *p, unsigned n)
+{
+  unsigned i = 0;
+  do
+    {
+      if (i == n)
+        break;
+      if (p)
+        x = *p;
+      i += 2;
+    }
+  while (1);
+  x = *p;
+}
+
+void do_exit (int i)
+{
+  exit (0);
+}
+
+int main()
+{
+  struct sigaction s;
+  sigemptyset (&s.sa_mask);
+  s.sa_handler = do_exit;
+  s.sa_flags = 0;
+  sigaction (SIGALRM, &s, NULL);
+  alarm (1);
+  foo ((int *)0, 1);
+}
diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc
index 6c0436b8b15..13f2c8a5420 100644
--- a/gcc/tree-ssa-pre.cc
+++ b/gcc/tree-ssa-pre.cc
@@ -2008,10 +2008,11 @@  clean (bitmap_set_t set1, bitmap_set_t set2 = NULL)
 }
 
 /* Clean the set of expressions that are no longer valid in SET because
-   they are clobbered in BLOCK or because they trap and may not be executed.  */
+   they are clobbered in BLOCK or because they trap and may not be executed.
+   When CLEAN_TRAPS is true remove all possibly trapping expressions.  */
 
 static void
-prune_clobbered_mems (bitmap_set_t set, basic_block block)
+prune_clobbered_mems (bitmap_set_t set, basic_block block, bool clean_traps)
 {
   bitmap_iterator bi;
   unsigned i;
@@ -2049,7 +2050,7 @@  prune_clobbered_mems (bitmap_set_t set, basic_block block)
 	     a possible exit point.
 	     ???  This is overly conservative if we translate AVAIL_OUT
 	     as the available expression might be after the exit point.  */
-	  if (BB_MAY_NOTRETURN (block)
+	  if ((BB_MAY_NOTRETURN (block) || clean_traps)
 	      && vn_reference_may_trap (ref))
 	    to_remove = i;
 	}
@@ -2060,7 +2061,7 @@  prune_clobbered_mems (bitmap_set_t set, basic_block block)
 	     a possible exit point.
 	     ???  This is overly conservative if we translate AVAIL_OUT
 	     as the available expression might be after the exit point.  */
-	  if (BB_MAY_NOTRETURN (block)
+	  if ((BB_MAY_NOTRETURN (block) || clean_traps)
 	      && vn_nary_may_trap (nary))
 	    to_remove = i;
 	}
@@ -2114,6 +2115,8 @@  compute_antic_aux (basic_block block, bool block_has_abnormal_pred_edge)
 
   bool was_visited = BB_VISITED (block);
   bool changed = ! BB_VISITED (block);
+  bool any_max_on_edge = false;
+
   BB_VISITED (block) = 1;
   old = ANTIC_OUT = S = NULL;
 
@@ -2158,6 +2161,7 @@  compute_antic_aux (basic_block block, bool block_has_abnormal_pred_edge)
 		 maximal set to arrive at a maximum ANTIC_IN solution.
 		 We can ignore them in the intersection operation and thus
 		 need not explicitely represent that maximum solution.  */
+	      any_max_on_edge = true;
 	      if (dump_file && (dump_flags & TDF_DETAILS))
 		fprintf (dump_file, "ANTIC_IN is MAX on %d->%d\n",
 			 e->src->index, e->dest->index);
@@ -2223,7 +2227,7 @@  compute_antic_aux (basic_block block, bool block_has_abnormal_pred_edge)
 
   /* Prune expressions that are clobbered in block and thus become
      invalid if translated from ANTIC_OUT to ANTIC_IN.  */
-  prune_clobbered_mems (ANTIC_OUT, block);
+  prune_clobbered_mems (ANTIC_OUT, block, any_max_on_edge);
 
   /* Generate ANTIC_OUT - TMP_GEN.  */
   S = bitmap_set_subtract_expressions (ANTIC_OUT, TMP_GEN (block));
@@ -2396,7 +2400,7 @@  compute_partial_antic_aux (basic_block block,
 
   /* Prune expressions that are clobbered in block and thus become
      invalid if translated from PA_OUT to PA_IN.  */
-  prune_clobbered_mems (PA_OUT, block);
+  prune_clobbered_mems (PA_OUT, block, false);
 
   /* PA_IN starts with PA_OUT - TMP_GEN.
      Then we subtract things from ANTIC_IN.  */