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]

[PR52001] too many cse reverse equiv exprs (take2)


Jakub asked to have a closer look at the problem, and I found we could
do somewhat better.  The first thing I noticed was that the problem was
that, in each block that computed a (base+const), we created a new VALUE
for the expression (with the same const and global base), and a new
reverse operation.

This was wrong.  Clearly we should reuse the same expression.  I had to
arrange for the expression to be retained across basic blocks, for it
was function invariant.  I split out the code to detect invariants from
the function that removes entries from the cselib hash table across
blocks, and made it recursive so that a VALUE equivalent to (plus
(value) (const_int)) will be retained, if the base value fits (maybe
recursively) the definition of invariant.

An earlier attempt to address this issue remained in cselib: using the
canonical value to build the reverse expression.  I believe it has a
potential of avoiding the creation of redundant reverse expressions, for
expressions involving equivalent but different VALUEs will evaluate to
different hashes.  I haven't observed effects WRT the given testcase,
before or after the change that actually fixed the problem, because we
now find the same base expression and thus reuse the reverse_op as well,
but I figured I'd keep it in for it is very cheap and possibly useful.

Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu.  Ok to install?

for gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/52001
	* cselib.c (invariant_p): Split out of...
	(preserve_only_constants): ... this.  Preserve plus expressions
	of invariant values and constants.
	* var-tracking.c (reverse_op): Don't drop equivs of constants.
	Use canonical value to build reverse operation.

Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c.orig	2012-02-12 06:13:40.676385499 -0200
+++ gcc/cselib.c	2012-02-12 09:07:00.653579375 -0200
@@ -383,22 +383,29 @@ cselib_clear_table (void)
   cselib_reset_table (1);
 }
 
-/* Remove from hash table all VALUEs except constants
-   and function invariants.  */
+/* Return TRUE if V is a constant or a function invariant, FALSE
+   otherwise.  */
 
-static int
-preserve_only_constants (void **x, void *info ATTRIBUTE_UNUSED)
+static bool
+invariant_p (cselib_val *v)
 {
-  cselib_val *v = (cselib_val *)*x;
   struct elt_loc_list *l;
 
+  if (v == cfa_base_preserved_val)
+    return true;
+
+  /* Keep VALUE equivalences around.  */
+  for (l = v->locs; l; l = l->next)
+    if (GET_CODE (l->loc) == VALUE)
+      return true;
+
   if (v->locs != NULL
       && v->locs->next == NULL)
     {
       if (CONSTANT_P (v->locs->loc)
 	  && (GET_CODE (v->locs->loc) != CONST
 	      || !references_value_p (v->locs->loc, 0)))
-	return 1;
+	return true;
       /* Although a debug expr may be bound to different expressions,
 	 we can preserve it as if it was constant, to get unification
 	 and proper merging within var-tracking.  */
@@ -406,24 +413,29 @@ preserve_only_constants (void **x, void 
 	  || GET_CODE (v->locs->loc) == DEBUG_IMPLICIT_PTR
 	  || GET_CODE (v->locs->loc) == ENTRY_VALUE
 	  || GET_CODE (v->locs->loc) == DEBUG_PARAMETER_REF)
-	return 1;
-      if (cfa_base_preserved_val)
-	{
-	  if (v == cfa_base_preserved_val)
-	    return 1;
-	  if (GET_CODE (v->locs->loc) == PLUS
-	      && CONST_INT_P (XEXP (v->locs->loc, 1))
-	      && XEXP (v->locs->loc, 0) == cfa_base_preserved_val->val_rtx)
-	    return 1;
-	}
+	return true;
+
+      /* (plus (value V) (const_int C)) is invariant iff V is invariant.  */
+      if (GET_CODE (v->locs->loc) == PLUS
+	  && CONST_INT_P (XEXP (v->locs->loc, 1))
+	  && GET_CODE (XEXP (v->locs->loc, 0)) == VALUE
+	  && invariant_p (CSELIB_VAL_PTR (XEXP (v->locs->loc, 0))))
+	return true;
     }
 
-  /* Keep VALUE equivalences around.  */
-  for (l = v->locs; l; l = l->next)
-    if (GET_CODE (l->loc) == VALUE)
-      return 1;
+  return false;
+}
+
+/* Remove from hash table all VALUEs except constants
+   and function invariants.  */
+
+static int
+preserve_only_constants (void **x, void *info ATTRIBUTE_UNUSED)
+{
+  cselib_val *v = (cselib_val *)*x;
 
-  htab_clear_slot (cselib_hash_table, x);
+  if (!invariant_p (v))
+    htab_clear_slot (cselib_hash_table, x);
   return 1;
 }
 
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-02-12 06:13:38.633412886 -0200
+++ gcc/var-tracking.c	2012-02-12 10:09:49.000000000 -0200
@@ -5298,7 +5298,6 @@ reverse_op (rtx val, const_rtx expr, rtx
 {
   rtx src, arg, ret;
   cselib_val *v;
-  struct elt_loc_list *l;
   enum rtx_code code;
 
   if (GET_CODE (expr) != SET)
@@ -5334,13 +5333,9 @@ reverse_op (rtx val, const_rtx expr, rtx
   if (!v || !cselib_preserved_value_p (v))
     return;
 
-  /* Adding a reverse op isn't useful if V already has an always valid
-     location.  Ignore ENTRY_VALUE, while it is always constant, we should
-     prefer non-ENTRY_VALUE locations whenever possible.  */
-  for (l = v->locs; l; l = l->next)
-    if (CONSTANT_P (l->loc)
-	&& (GET_CODE (l->loc) != CONST || !references_value_p (l->loc, 0)))
-      return;
+  /* Use canonical V to avoid creating multiple redundant expressions
+     for different VALUES equivalent to V.  */
+  v = canonical_cselib_val (v);
 
   switch (GET_CODE (src))
     {
-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

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