diff mbox series

tree-optimization/112310 - code hoisting undefined behavior

Message ID 20231103103446.1B3F31348C@imap2.suse-dmz.suse.de
State Unresolved
Headers
Series tree-optimization/112310 - code hoisting undefined behavior |

Checks

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

Commit Message

Richard Biener Nov. 3, 2023, 10:34 a.m. UTC 
  The following avoids hoisting expressions that may invoke undefined
behavior and are not computed on all paths.  This is realized by
noting that we have to avoid materializing expressions as part
of hoisting that are not part of the set of expressions we have
found eligible for hoisting.  Instead of picking the expression
corresponding to the hoistable values from the first successor
we now keep a union of the expressions so that hoisting can pick
the expression that has its dependences fully hoistable.

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

	PR tree-optimization/112310
	* tree-ssa-pre.cc (do_hoist_insertion): Keep the union
	of expressions, validate dependences are contained within
	the hoistable set before hoisting.

	* gcc.dg/torture/pr112310.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr112310.c | 36 +++++++++++++++++++++++++
 gcc/tree-ssa-pre.cc                     | 26 ++++++++++++------
 2 files changed, 54 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr112310.c
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/torture/pr112310.c b/gcc/testsuite/gcc.dg/torture/pr112310.c
new file mode 100644
index 00000000000..daf2390734c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr112310.c
@@ -0,0 +1,36 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
+
+extern void abort (void);
+short a, b;
+static int c, d, e, f = -7;
+char g;
+int *h = &d;
+int **i = &h;
+int j;
+short(k)(int l) { return l >= 2 ? 0 : l; }
+int(m)(int l, int n) { return l < -2147483647 / n ? l : l * n; }
+void o() { &c; }
+int main()
+{
+  int p;
+  for (; g <= 3; g++)
+    {
+      for (; c; c++)
+	;
+      a = 2;
+      for (; a <= 7; a++) {
+	  short *r = &b;
+	  p = m(*h, 2022160547);
+	  unsigned q = 2022160547;
+	  e = p * q;
+	  *r ^= e;
+	  j = k(c + 3);
+	  **i = 0;
+      }
+      *i = &f;
+    }
+  if (b != -3189)
+    abort ();
+  return 0;
+}
diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc
index 07fb165b2a8..361806ac2c9 100644
--- a/gcc/tree-ssa-pre.cc
+++ b/gcc/tree-ssa-pre.cc
@@ -3635,10 +3635,9 @@  do_hoist_insertion (basic_block block)
     return false;
 
   /* We have multiple successors, compute ANTIC_OUT by taking the intersection
-     of all of ANTIC_IN translating through PHI nodes.  Note we do not have to
-     worry about iteration stability here so just use the expression set
-     from the first set and prune that by sorted_array_from_bitmap_set.
-     This is a simplification of what we do in compute_antic_aux.  */
+     of all of ANTIC_IN translating through PHI nodes.  Track the union
+     of the expression sets so we can pick a representative that is
+     fully generatable out of hoistable expressions.  */
   bitmap_set_t ANTIC_OUT = bitmap_set_new ();
   bool first = true;
   FOR_EACH_EDGE (e, ei, block->succs)
@@ -3653,10 +3652,15 @@  do_hoist_insertion (basic_block block)
 	  bitmap_set_t tmp = bitmap_set_new ();
 	  phi_translate_set (tmp, ANTIC_IN (e->dest), e);
 	  bitmap_and_into (&ANTIC_OUT->values, &tmp->values);
+	  bitmap_ior_into (&ANTIC_OUT->expressions, &tmp->expressions);
 	  bitmap_set_free (tmp);
 	}
       else
-	bitmap_and_into (&ANTIC_OUT->values, &ANTIC_IN (e->dest)->values);
+	{
+	  bitmap_and_into (&ANTIC_OUT->values, &ANTIC_IN (e->dest)->values);
+	  bitmap_ior_into (&ANTIC_OUT->expressions,
+			   &ANTIC_IN (e->dest)->expressions);
+	}
     }
 
   /* Compute the set of hoistable expressions from ANTIC_OUT.  First compute
@@ -3697,15 +3701,13 @@  do_hoist_insertion (basic_block block)
       return false;
     }
 
-  /* Hack hoitable_set in-place so we can use sorted_array_from_bitmap_set.  */
+  /* Hack hoistable_set in-place so we can use sorted_array_from_bitmap_set.  */
   bitmap_move (&hoistable_set.values, &availout_in_some);
   hoistable_set.expressions = ANTIC_OUT->expressions;
 
   /* Now finally construct the topological-ordered expression set.  */
   vec<pre_expr> exprs = sorted_array_from_bitmap_set (&hoistable_set);
 
-  bitmap_clear (&hoistable_set.values);
-
   /* If there are candidate values for hoisting, insert expressions
      strategically to make the hoistable expressions fully redundant.  */
   pre_expr expr;
@@ -3737,6 +3739,13 @@  do_hoist_insertion (basic_block block)
 	  && FLOAT_TYPE_P (get_expr_type (expr)))
 	continue;
 
+      /* Only hoist if the full expression is available for hoisting.
+	 This avoids hoisting values that are not common and for
+	 example evaluate an expression that's not valid to evaluate
+	 unconditionally (PR112310).  */
+      if (!valid_in_sets (&hoistable_set, AVAIL_OUT (block), expr))
+	continue;
+
       /* OK, we should hoist this value.  Perform the transformation.  */
       pre_stats.hoist_insert++;
       if (dump_file && (dump_flags & TDF_DETAILS))
@@ -3774,6 +3783,7 @@  do_hoist_insertion (basic_block block)
     }
 
   exprs.release ();
+  bitmap_clear (&hoistable_set.values);
   bitmap_set_free (ANTIC_OUT);
 
   return new_stuff;