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 <aol...@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
