This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR87985


The following fixes split_constant_offset unbound un-CSEing of
expressions when following SSA def stmts.  Simply limiting it to
single-uses isn't good for consumers so the following instead
limits analysis by implementing a cache.  Note this may still
end up un-CSEing stuff but I didn't want to try inserting
SAVE_EXPRs in split_constant_offset result...  (maybe I should
simply try though...).  Another option would be to give up
when we see several uses of an "interesting" expression, thus
make the hash-map a visited thing instead (but the result would
be somewhat odd I guess).

Anyway, the following preserves existing behavior while fixing
the compile-time issue for the testcase (which doesn't end up
generating anything interesting).

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

>From cfe2c14173b8d2fa6e998e9895dce0cdf9b3e00e Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguenther@suse.de>
Date: Mon, 12 Nov 2018 14:45:27 +0100
Subject: [PATCH] fix-pr87985

	PR middle-end/87985
	* tree-data-ref.c (split_constant_offset): Add wrapper
	allocating a cache hash-map.
	(split_constant_offset_1): Cache results of expanding
	expressions from SSA def stmts.

	* gcc.dg/pr87985.c: New testcase.

diff --git a/gcc/testsuite/gcc.dg/pr87985.c b/gcc/testsuite/gcc.dg/pr87985.c
new file mode 100644
index 00000000000..c0d07ff918f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87985.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O -ftree-slp-vectorize" } */
+
+char *bar (void);
+__INTPTR_TYPE__ baz (void);
+
+void
+foo (__INTPTR_TYPE__ *q)
+{
+  char *p = bar ();
+  __INTPTR_TYPE__ a = baz ();
+  __INTPTR_TYPE__ b = baz ();
+  int i = 0;
+#define X q[i++] = a; q[i++] = b; a = a + b; b = b + a;
+#define Y X X X X X X X X X X
+#define Z Y Y Y Y Y Y Y Y Y Y
+  Z Z Z Z Z Z Z Z Z Z
+      p[a] = 1;
+  p[b] = 2;
+}
diff --git a/gcc/tree-data-ref.c b/gcc/tree-data-ref.c
index 6019c6168bf..0617c97eec4 100644
--- a/gcc/tree-data-ref.c
+++ b/gcc/tree-data-ref.c
@@ -95,10 +95,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-affine.h"
 #include "params.h"
 #include "builtins.h"
-#include "stringpool.h"
-#include "tree-vrp.h"
-#include "tree-ssanames.h"
 #include "tree-eh.h"
+#include "ssa.h"
 
 static struct datadep_stats
 {
@@ -584,6 +582,10 @@ debug_ddrs (vec<ddr_p> ddrs)
   dump_ddrs (stderr, ddrs);
 }
 
+static void
+split_constant_offset (tree exp, tree *var, tree *off,
+		       hash_map<tree, std::pair<tree, tree> > &cache);
+
 /* Helper function for split_constant_offset.  Expresses OP0 CODE OP1
    (the type of the result is TYPE) as VAR + OFF, where OFF is a nonzero
    constant of type ssizetype, and returns true.  If we cannot do this
@@ -592,7 +594,8 @@ debug_ddrs (vec<ddr_p> ddrs)
 
 static bool
 split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
-			 tree *var, tree *off)
+			 tree *var, tree *off,
+			 hash_map<tree, std::pair<tree, tree> > &cache)
 {
   tree var0, var1;
   tree off0, off1;
@@ -613,8 +616,10 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
       /* FALLTHROUGH */
     case PLUS_EXPR:
     case MINUS_EXPR:
-      split_constant_offset (op0, &var0, &off0);
-      split_constant_offset (op1, &var1, &off1);
+      split_constant_offset (op0, &var0, &off0, cache);
+      split_constant_offset (op1, &var1, &off1, cache);
+      if (integer_zerop (off0) && integer_zerop (off1))
+	return false;
       *var = fold_build2 (code, type, var0, var1);
       *off = size_binop (ocode, off0, off1);
       return true;
@@ -623,7 +628,9 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
       if (TREE_CODE (op1) != INTEGER_CST)
 	return false;
 
-      split_constant_offset (op0, &var0, &off0);
+      split_constant_offset (op0, &var0, &off0, cache);
+      if (integer_zerop (off0))
+	return false;
       *var = fold_build2 (MULT_EXPR, type, var0, op1);
       *off = size_binop (MULT_EXPR, off0, fold_convert (ssizetype, op1));
       return true;
@@ -647,7 +654,7 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
 
 	if (poffset)
 	  {
-	    split_constant_offset (poffset, &poffset, &off1);
+	    split_constant_offset (poffset, &poffset, &off1, cache);
 	    off0 = size_binop (PLUS_EXPR, off0, off1);
 	    if (POINTER_TYPE_P (TREE_TYPE (base)))
 	      base = fold_build_pointer_plus (base, poffset);
@@ -691,11 +698,40 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
 	if (gimple_code (def_stmt) != GIMPLE_ASSIGN)
 	  return false;
 
-	var0 = gimple_assign_rhs1 (def_stmt);
 	subcode = gimple_assign_rhs_code (def_stmt);
+
+	/* We are using a cache to avoid un-CSEing large amounts of code.  */
+	bool use_cache = false;
+	if (!has_single_use (op0)
+	    && (subcode == POINTER_PLUS_EXPR
+		|| subcode == PLUS_EXPR
+		|| subcode == MINUS_EXPR
+		|| subcode == MULT_EXPR
+		|| subcode == ADDR_EXPR
+		|| CONVERT_EXPR_CODE_P (subcode)))
+	  {
+	    use_cache = true;
+	    bool existed;
+	    std::pair<tree, tree> &e = cache.get_or_insert (op0, &existed);
+	    if (existed)
+	      {
+		if (integer_zerop (e.second))
+		  return false;
+		*var = e.first;
+		*off = e.second;
+		return true;
+	      }
+	    e = std::make_pair (op0, ssize_int (0));
+	  }
+
+	var0 = gimple_assign_rhs1 (def_stmt);
 	var1 = gimple_assign_rhs2 (def_stmt);
 
-	return split_constant_offset_1 (type, var0, subcode, var1, var, off);
+	bool res = split_constant_offset_1 (type, var0, subcode, var1,
+					    var, off, cache);
+	if (res && use_cache)
+	  *cache.get (op0) = std::make_pair (*var, *off);
+	return res;
       }
     CASE_CONVERT:
       {
@@ -714,7 +750,7 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
 		/* Split the unconverted operand and try to prove that
 		   wrapping isn't a problem.  */
 		tree tmp_var, tmp_off;
-		split_constant_offset (op0, &tmp_var, &tmp_off);
+		split_constant_offset (op0, &tmp_var, &tmp_off, cache);
 
 		/* See whether we have an SSA_NAME whose range is known
 		   to be [A, B].  */
@@ -749,7 +785,7 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
 		*off = wide_int_to_tree (ssizetype, diff);
 	      }
 	    else
-	      split_constant_offset (op0, &var0, off);
+	      split_constant_offset (op0, &var0, off, cache);
 	    *var = fold_convert (type, var0);
 	    return true;
 	  }
@@ -764,8 +800,9 @@ split_constant_offset_1 (tree type, tree op0, enum tree_code code, tree op1,
 /* Expresses EXP as VAR + OFF, where off is a constant.  The type of OFF
    will be ssizetype.  */
 
-void
-split_constant_offset (tree exp, tree *var, tree *off)
+static void
+split_constant_offset (tree exp, tree *var, tree *off,
+		       hash_map<tree, std::pair<tree, tree> > &cache)
 {
   tree type = TREE_TYPE (exp), op0, op1, e, o;
   enum tree_code code;
@@ -779,13 +816,23 @@ split_constant_offset (tree exp, tree *var, tree *off)
 
   code = TREE_CODE (exp);
   extract_ops_from_tree (exp, &code, &op0, &op1);
-  if (split_constant_offset_1 (type, op0, code, op1, &e, &o))
+  if (split_constant_offset_1 (type, op0, code, op1, &e, &o, cache))
     {
       *var = e;
       *off = o;
     }
 }
 
+void
+split_constant_offset (tree exp, tree *var, tree *off)
+{
+  static hash_map<tree, std::pair<tree, tree> > *cache;
+  if (!cache)
+    cache = new hash_map<tree, std::pair<tree, tree> > (37);
+  split_constant_offset (exp, var, off, *cache);
+  cache->empty ();
+}
+
 /* Returns the address ADDR of an object in a canonical shape (without nop
    casts, and with type of pointer to the object).  */
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]