Move match-and-simplify recursion limit from SCCVN to generic place

Message ID alpine.LSU.2.20.1807120904240.16707@zhemvz.fhfr.qr
State New
Headers show
Series
  • Move match-and-simplify recursion limit from SCCVN to generic place
Related show

Commit Message

Richard Biener July 12, 2018, 7:07 a.m.
This moves a recursion limit installed for PR80887 from 
vn_lookup_simplify_result to gimple_resimplify* where it also applies
to a testcase I'm running into with modified VN without ever
calling vn_lookup_simplify_result.  Basically with VN we are
not presenting the match.pd patterns with simplified expressions
but for example we happily feed it ((_50 + 0) + 8) via valueization
of def-stmt operands.  With the correct IL setup it can easily
happen that _50 is a leader for itself, thus with _50 = _2 + _3;
it is value-numbered as a copy but the value doesn't have a leader
here so _50 becomes the leader.  This effectively turns it into
_50 = _50;

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2018-07-12  Richard Biener  <rguenther@suse.de>

	* tree-ssa-sccvn.c (mprts_hook_cnt): Remove.
	(vn_lookup_simplify_result): Remove recursion limit applied
	here.
	(vn_nary_build_or_lookup_1): Adjust.
	(try_to_simplify): Likewise.
	* gimple-match-head.c (gimple_resimplify1): Instead apply one
	here.
	(gimple_resimplify2): Likewise.
	(gimple_resimplify3): Likewise.
	(gimple_resimplify4): Likewise.

Comments

Richard Biener July 12, 2018, 10:12 a.m. | #1
On Thu, 12 Jul 2018, Richard Biener wrote:

> 
> This moves a recursion limit installed for PR80887 from 
> vn_lookup_simplify_result to gimple_resimplify* where it also applies
> to a testcase I'm running into with modified VN without ever
> calling vn_lookup_simplify_result.  Basically with VN we are
> not presenting the match.pd patterns with simplified expressions
> but for example we happily feed it ((_50 + 0) + 8) via valueization
> of def-stmt operands.  With the correct IL setup it can easily
> happen that _50 is a leader for itself, thus with _50 = _2 + _3;
> it is value-numbered as a copy but the value doesn't have a leader
> here so _50 becomes the leader.  This effectively turns it into
> _50 = _50;
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

And I was too lazy in simplifying vn_lookup_simplify_result.

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

Richard.

2018-07-12  Richard Biener  <rguenther@suse.de>

	* tree-ssa-sccvn.c (vn_lookup_simplify_result): Remove bogus
	left-over from last patch.

Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c	(revision 262577)
+++ gcc/tree-ssa-sccvn.c	(working copy)
@@ -1669,17 +1669,8 @@ vn_lookup_simplify_result (gimple_match_
 	ops[i] = CONSTRUCTOR_ELT (res_op->ops[0], i)->value;
     }
   vn_nary_op_t vnresult = NULL;
-  tree res = vn_nary_op_lookup_pieces (length, (tree_code) res_op->code,
-				       res_op->type, ops, &vnresult);
-  if (res
-      && mprts_hook)
-    {
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	fprintf (dump_file, "Resetting mprts_hook after too many "
-		 "invocations.\n");
-      mprts_hook = NULL;
-    }
-  return res;
+  return vn_nary_op_lookup_pieces (length, (tree_code) res_op->code,
+				   res_op->type, ops, &vnresult);
 }
 
 /* Return a value-number for RCODE OPS... either by looking up an existing

Patch

Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c	(revision 262551)
+++ gcc/tree-ssa-sccvn.c	(working copy)
@@ -1648,7 +1648,6 @@  vn_reference_lookup_or_insert_for_pieces
 }
 
 static vn_nary_op_t vn_nary_op_insert_stmt (gimple *stmt, tree result);
-static unsigned mprts_hook_cnt;
 
 /* Hook for maybe_push_res_to_seq, lookup the expression in the VN tables.  */
 
@@ -1672,13 +1671,8 @@  vn_lookup_simplify_result (gimple_match_
   vn_nary_op_t vnresult = NULL;
   tree res = vn_nary_op_lookup_pieces (length, (tree_code) res_op->code,
 				       res_op->type, ops, &vnresult);
-  /* We can end up endlessly recursing simplifications if the lookup above
-     presents us with a def-use chain that mirrors the original simplification.
-     See PR80887 for an example.  Limit successful lookup artificially
-     to 10 times if we are called as mprts_hook.  */
   if (res
-      && mprts_hook
-      && --mprts_hook_cnt == 0)
+      && mprts_hook)
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
 	fprintf (dump_file, "Resetting mprts_hook after too many "
@@ -1701,7 +1695,6 @@  vn_nary_build_or_lookup_1 (gimple_match_
      So first simplify and lookup this expression to see if it
      is already available.  */
   mprts_hook = vn_lookup_simplify_result;
-  mprts_hook_cnt = 9;
   bool res = false;
   switch (TREE_CODE_LENGTH ((tree_code) res_op->code))
     {
@@ -4051,7 +4044,6 @@  try_to_simplify (gassign *stmt)
 
   /* First try constant folding based on our current lattice.  */
   mprts_hook = vn_lookup_simplify_result;
-  mprts_hook_cnt = 9;
   tem = gimple_fold_stmt_to_constant_1 (stmt, vn_valueize, vn_valueize);
   mprts_hook = NULL;
   if (tem
Index: gcc/gimple-match-head.c
===================================================================
--- gcc/gimple-match-head.c	(revision 262551)
+++ gcc/gimple-match-head.c	(working copy)
@@ -97,13 +97,30 @@  gimple_resimplify1 (gimple_seq *seq, gim
 	}
     }
 
+  /* Limit recursion, there are cases like PR80887 and others, for
+     example when value-numbering presents us with unfolded expressions
+     that we are really not prepared to handle without eventual
+     oscillation like ((_50 + 0) + 8) where _50 gets mapped to _50
+     itself as available expression.  */
+  static unsigned depth;
+  if (depth > 10)
+    {
+      if (dump_file && (dump_flags & TDF_FOLDING))
+	fprintf (dump_file, "Aborting expression simplification due to "
+		 "deep recursion\n");
+      return false;
+    }
+
+  ++depth;
   gimple_match_op res_op2 (*res_op);
   if (gimple_simplify (&res_op2, seq, valueize,
 		       res_op->code, res_op->type, res_op->ops[0]))
     {
+      --depth;
       *res_op = res_op2;
       return true;
     }
+  --depth;
 
   return false;
 }
@@ -151,14 +168,27 @@  gimple_resimplify2 (gimple_seq *seq, gim
       canonicalized = true;
     }
 
+  /* Limit recursion, see gimple_resimplify1.  */
+  static unsigned depth;
+  if (depth > 10)
+    {
+      if (dump_file && (dump_flags & TDF_FOLDING))
+	fprintf (dump_file, "Aborting expression simplification due to "
+		 "deep recursion\n");
+      return false;
+    }
+
+  ++depth;
   gimple_match_op res_op2 (*res_op);
   if (gimple_simplify (&res_op2, seq, valueize,
 		       res_op->code, res_op->type,
 		       res_op->ops[0], res_op->ops[1]))
     {
+      --depth;
       *res_op = res_op2;
       return true;
     }
+  --depth;
 
   return canonicalized;
 }
@@ -205,14 +235,27 @@  gimple_resimplify3 (gimple_seq *seq, gim
       canonicalized = true;
     }
 
+  /* Limit recursion, see gimple_resimplify1.  */
+  static unsigned depth;
+  if (depth > 10)
+    {
+      if (dump_file && (dump_flags & TDF_FOLDING))
+	fprintf (dump_file, "Aborting expression simplification due to "
+		 "deep recursion\n");
+      return false;
+    }
+
+  ++depth;
   gimple_match_op res_op2 (*res_op);
   if (gimple_simplify (&res_op2, seq, valueize,
 		       res_op->code, res_op->type,
 		       res_op->ops[0], res_op->ops[1], res_op->ops[2]))
     {
+      --depth;
       *res_op = res_op2;
       return true;
     }
+  --depth;
 
   return canonicalized;
 }
@@ -229,15 +272,28 @@  gimple_resimplify4 (gimple_seq *seq, gim
 {
   /* No constant folding is defined for four-operand functions.  */
 
+  /* Limit recursion, see gimple_resimplify1.  */
+  static unsigned depth;
+  if (depth > 10)
+    {
+      if (dump_file && (dump_flags & TDF_FOLDING))
+	fprintf (dump_file, "Aborting expression simplification due to "
+		 "deep recursion\n");
+      return false;
+    }
+
+  ++depth;
   gimple_match_op res_op2 (*res_op);
   if (gimple_simplify (&res_op2, seq, valueize,
 		       res_op->code, res_op->type,
 		       res_op->ops[0], res_op->ops[1], res_op->ops[2],
 		       res_op->ops[3]))
     {
+      --depth;
       *res_op = res_op2;
       return true;
     }
+  --depth;
 
   return false;
 }