* [PR debug/45130] fix MEM_REF -fcompare-debug regression
@ 2010-09-05 8:36 Alexandre Oliva
2010-09-05 9:45 ` Richard Guenther
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2010-09-05 8:36 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]
MEM_REFs introduced some -fcompare-debug regressions. There are two
main issues, both related with caching of MEM attributes.
One is that top-level qualifiers in the types of the two MEM_REF
operands, although irrelevant, are regarded as conversions by the tree
dumpers. When e.g. copyprop replaces a non-const-qualified variable to
a MEM_REF that was built out of a const-qualified temporary, we end up
with a type mismatch, dumped as an explicit type conversion in the
MEM_REF operand.
This, compounded with the second issue, that types are disregarded by
the hash and compare functions used to build the MEM attribute cache,
means that the presence of debug expressions with or without the
mismatches may cause a different-typed expression to be cached. Then,
non-debug expressions that share the same cached MEM attrs may be dumped
one way or another, failing -fcompare-debug.
As it turns out, disregarding top-level qualifiers is not enough to fix
the problem, because other unrelated types happen to share the same
cache entry, particularly when expressions that dereference NULL are
involved.
To avoid this inappropriate sharing, I've added the OEP_SAME_TYPE flag
to operand_equal_p, as well as to iterative_hash_expr. This would
render useless the first hunk, to disregard top-level qualifiers in
MEM_REF dumping, but since such mismatches are just noise anyway, I
decided to leave it in.
This patch enabled bootstrap with -fcompare-debug to succeed for all of
stage3, including target libraries, with otherwise standard flags, on
x86_64-linux-gnu and i686-pc-linux-gnu, and for stage3 similar bootstrap
to succeed with -O1 and -O3 all the way to bootstrap-compare, on both
platforms as well. Regression testing also passed on both platforms.
Ok to install?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mem-ref-same-type.patch --]
[-- Type: text/x-diff, Size: 9064 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/45419
PR debug/45408
* tree-pretty-print.c (dump_generic_node): Disregard top-level
qualifiers in otherwise equal types.
* emit-rtl.c (mem_attrs_htab_hash): Hash with OEP_SAME_TYPE.
(mem_attrs_htab_eq): Compare with OEP_SAME_TYPE.
* fold-const.c (operand_equal_p): Handle OEP_SAME_TYPE.
* tree.c (iterative_hash_expr_1): Support OEP_SAME_TYPE. Renamed
from...
(iterative_hash_expr): This. New wrapper.
(iterative_hash_expr_flags): New.
* tree.h (OEP_SAME_TYPE): New enumerator in operand_equal_flag.
(iterative_hash_expr_flags): Declare.
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c.orig 2010-09-04 05:55:51.000000000 -0300
+++ gcc/tree-pretty-print.c 2010-09-04 05:56:06.000000000 -0300
@@ -807,8 +807,6 @@ dump_generic_node (pretty_printer *buffe
== TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
&& (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
- && (TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 0)))
- == TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 1))))
/* Same value types ignoring qualifiers. */
&& (TYPE_MAIN_VARIANT (TREE_TYPE (node))
== TYPE_MAIN_VARIANT
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c.orig 2010-09-04 05:54:18.881551941 -0300
+++ gcc/emit-rtl.c 2010-09-04 05:56:07.000000000 -0300
@@ -260,7 +260,7 @@ mem_attrs_htab_hash (const void *x)
^ (p->addrspace * 4000)
^ ((p->offset ? INTVAL (p->offset) : 0) * 50000)
^ ((p->size ? INTVAL (p->size) : 0) * 2500000)
- ^ (size_t) iterative_hash_expr (p->expr, 0));
+ ^ (size_t) iterative_hash_expr_flags (p->expr, 0, OEP_SAME_TYPE));
}
/* Returns nonzero if the value represented by X (which is really a
@@ -278,7 +278,7 @@ mem_attrs_htab_eq (const void *x, const
&& p->addrspace == q->addrspace
&& (p->expr == q->expr
|| (p->expr != NULL_TREE && q->expr != NULL_TREE
- && operand_equal_p (p->expr, q->expr, 0))));
+ && operand_equal_p (p->expr, q->expr, OEP_SAME_TYPE))));
}
/* Allocate a new mem_attrs structure and insert it into the hash table if
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c.orig 2010-09-04 05:54:18.000000000 -0300
+++ gcc/fold-const.c 2010-09-04 05:56:07.000000000 -0300
@@ -2388,7 +2388,9 @@ combine_comparisons (location_t loc,
If OEP_PURE_SAME is set, then pure functions with identical arguments
are considered the same. It is used when the caller has other ways
- to ensure that global memory is unchanged in between. */
+ to ensure that global memory is unchanged in between.
+
+ If OEP_SAME_TYPE is set, then mismatched types */
int
operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
@@ -2404,6 +2406,9 @@ operand_equal_p (const_tree arg0, const_
if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
return 0;
+ if ((flags & OEP_SAME_TYPE) && TREE_TYPE (arg0) != TREE_TYPE (arg1))
+ return 0;
+
/* Check equality of integer constants before bailing out due to
precision differences. */
if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
@@ -2527,7 +2532,7 @@ operand_equal_p (const_tree arg0, const_
case ADDR_EXPR:
return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0),
- 0);
+ (flags & OEP_SAME_TYPE));
default:
break;
}
Index: gcc/tree.c
===================================================================
--- gcc/tree.c.orig 2010-09-04 05:55:51.881551941 -0300
+++ gcc/tree.c 2010-09-04 12:20:57.000000000 -0300
@@ -6669,11 +6669,12 @@ commutative_ternary_tree_code (enum tree
/* Generate a hash value for an expression. This can be used iteratively
by passing a previous result as the VAL argument.
- This function is intended to produce the same hash for expressions which
- would compare equal using operand_equal_p. */
+ This function is intended to produce the same hash for expressions
+ which would compare equal using operand_equal_p, called with the
+ same FLAGS. */
-hashval_t
-iterative_hash_expr (const_tree t, hashval_t val)
+static hashval_t
+iterative_hash_expr_1 (const_tree t, hashval_t val, unsigned int flags)
{
int i;
enum tree_code code;
@@ -6682,6 +6683,9 @@ iterative_hash_expr (const_tree t, hashv
if (t == NULL_TREE)
return iterative_hash_hashval_t (0, val);
+ if (TREE_TYPE (t) && (flags & OEP_SAME_TYPE))
+ val = iterative_hash_hashval_t (TYPE_UID (TREE_TYPE (t)), val);
+
code = TREE_CODE (t);
switch (code)
@@ -6707,10 +6711,10 @@ iterative_hash_expr (const_tree t, hashv
return iterative_hash (TREE_STRING_POINTER (t),
TREE_STRING_LENGTH (t), val);
case COMPLEX_CST:
- val = iterative_hash_expr (TREE_REALPART (t), val);
- return iterative_hash_expr (TREE_IMAGPART (t), val);
+ val = iterative_hash_expr_1 (TREE_REALPART (t), val, flags);
+ return iterative_hash_expr_1 (TREE_IMAGPART (t), val, flags);
case VECTOR_CST:
- return iterative_hash_expr (TREE_VECTOR_CST_ELTS (t), val);
+ return iterative_hash_expr_1 (TREE_VECTOR_CST_ELTS (t), val, flags);
case SSA_NAME:
/* We can just compare by pointer. */
return iterative_hash_host_wide_int (SSA_NAME_VERSION (t), val);
@@ -6721,7 +6725,7 @@ iterative_hash_expr (const_tree t, hashv
/* A list of expressions, for a CALL_EXPR or as the elements of a
VECTOR_CST. */
for (; t; t = TREE_CHAIN (t))
- val = iterative_hash_expr (TREE_VALUE (t), val);
+ val = iterative_hash_expr_1 (TREE_VALUE (t), val, flags);
return val;
case CONSTRUCTOR:
{
@@ -6729,8 +6733,8 @@ iterative_hash_expr (const_tree t, hashv
tree field, value;
FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (t), idx, field, value)
{
- val = iterative_hash_expr (field, val);
- val = iterative_hash_expr (value, val);
+ val = iterative_hash_expr_1 (field, val, flags);
+ val = iterative_hash_expr_1 (value, val, flags);
}
return val;
}
@@ -6769,7 +6773,7 @@ iterative_hash_expr (const_tree t, hashv
{
/* Make sure to include signness in the hash computation. */
val += TYPE_UNSIGNED (TREE_TYPE (t));
- val = iterative_hash_expr (TREE_OPERAND (t, 0), val);
+ val = iterative_hash_expr_1 (TREE_OPERAND (t, 0), val, flags);
}
else if (commutative_tree_code (code))
@@ -6778,8 +6782,10 @@ iterative_hash_expr (const_tree t, hashv
however it appears. We do this by first hashing both operands
and then rehashing based on the order of their independent
hashes. */
- hashval_t one = iterative_hash_expr (TREE_OPERAND (t, 0), 0);
- hashval_t two = iterative_hash_expr (TREE_OPERAND (t, 1), 0);
+ hashval_t one = iterative_hash_expr_1 (TREE_OPERAND (t, 0),
+ 0, flags);
+ hashval_t two = iterative_hash_expr_1 (TREE_OPERAND (t, 1),
+ 0, flags);
hashval_t t;
if (one > two)
@@ -6790,13 +6796,30 @@ iterative_hash_expr (const_tree t, hashv
}
else
for (i = TREE_OPERAND_LENGTH (t) - 1; i >= 0; --i)
- val = iterative_hash_expr (TREE_OPERAND (t, i), val);
+ val = iterative_hash_expr_1 (TREE_OPERAND (t, i), val, flags);
}
return val;
break;
}
}
+/* Wrapper for iterative_hash_expr_1, expected to be
+ inlined/specialized with flags == 0. */
+
+hashval_t
+iterative_hash_expr (const_tree t, hashval_t val)
+{
+ return iterative_hash_expr_1 (t, val, 0);
+}
+
+/* Publicly-visible wrapper for iterative_hash_expr_1. */
+
+hashval_t
+iterative_hash_expr_flags (const_tree t, hashval_t val, unsigned int flags)
+{
+ return iterative_hash_expr_1 (t, val, flags);
+}
+
/* Generate a hash value for a pair of expressions. This can be used
iteratively by passing a previous result as the VAL argument.
Index: gcc/tree.h
===================================================================
--- gcc/tree.h.orig 2010-09-04 05:55:51.868554928 -0300
+++ gcc/tree.h 2010-09-04 05:56:07.000000000 -0300
@@ -4952,7 +4952,8 @@ extern bool fold_deferring_overflow_warn
enum operand_equal_flag
{
OEP_ONLY_CONST = 1,
- OEP_PURE_SAME = 2
+ OEP_PURE_SAME = 2,
+ OEP_SAME_TYPE = 4
};
extern int operand_equal_p (const_tree, const_tree, unsigned int);
@@ -5085,6 +5086,8 @@ extern int tree_log2 (const_tree);
extern int tree_floor_log2 (const_tree);
extern int simple_cst_equal (const_tree, const_tree);
extern hashval_t iterative_hash_expr (const_tree, hashval_t);
+extern hashval_t iterative_hash_expr_flags (const_tree, hashval_t,
+ unsigned int);
extern hashval_t iterative_hash_exprs_commutative (const_tree,
const_tree, hashval_t);
extern hashval_t iterative_hash_host_wide_int (HOST_WIDE_INT, hashval_t);
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-05 8:36 [PR debug/45130] fix MEM_REF -fcompare-debug regression Alexandre Oliva
@ 2010-09-05 9:45 ` Richard Guenther
2010-09-06 18:18 ` Alexandre Oliva
0 siblings, 1 reply; 8+ messages in thread
From: Richard Guenther @ 2010-09-05 9:45 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
On Sun, Sep 5, 2010 at 9:21 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> MEM_REFs introduced some -fcompare-debug regressions. There are two
> main issues, both related with caching of MEM attributes.
>
> One is that top-level qualifiers in the types of the two MEM_REF
> operands, although irrelevant, are regarded as conversions by the tree
> dumpers. When e.g. copyprop replaces a non-const-qualified variable to
> a MEM_REF that was built out of a const-qualified temporary, we end up
> with a type mismatch, dumped as an explicit type conversion in the
> MEM_REF operand.
>
> This, compounded with the second issue, that types are disregarded by
> the hash and compare functions used to build the MEM attribute cache,
> means that the presence of debug expressions with or without the
> mismatches may cause a different-typed expression to be cached. Then,
> non-debug expressions that share the same cached MEM attrs may be dumped
> one way or another, failing -fcompare-debug.
>
> As it turns out, disregarding top-level qualifiers is not enough to fix
> the problem, because other unrelated types happen to share the same
> cache entry, particularly when expressions that dereference NULL are
> involved.
>
> To avoid this inappropriate sharing, I've added the OEP_SAME_TYPE flag
> to operand_equal_p, as well as to iterative_hash_expr. This would
> render useless the first hunk, to disregard top-level qualifiers in
> MEM_REF dumping, but since such mismatches are just noise anyway, I
> decided to leave it in.
>
> This patch enabled bootstrap with -fcompare-debug to succeed for all of
> stage3, including target libraries, with otherwise standard flags, on
> x86_64-linux-gnu and i686-pc-linux-gnu, and for stage3 similar bootstrap
> to succeed with -O1 and -O3 all the way to bootstrap-compare, on both
> platforms as well. Regression testing also passed on both platforms.
>
> Ok to install?
+ If OEP_SAME_TYPE is set, then mismatched types */
there seems to be missing sth.
You want to change how types of constants are treated, right? But you
now apply the type check everywhere.
The issue seems also to be a correctness one to me, as we'd happily
share MEM[(char *)ptr] and MEM[(int *)ptr] which would break
TBAA.
Thus, simply add a type equality check for operand 1 in operand_equal_p here:
case MEM_REF:
/* Require equal access sizes. We can have incomplete types
for array references of variable-sized arrays from the
Fortran frontent though. */
return ((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1))
|| (TYPE_SIZE (TREE_TYPE (arg0))
&& TYPE_SIZE (TREE_TYPE (arg1))
&& operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
TYPE_SIZE (TREE_TYPE
(arg1)), flags)))
&& OP_SAME (0) && OP_SAME (1));
Richard.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-05 9:45 ` Richard Guenther
@ 2010-09-06 18:18 ` Alexandre Oliva
2010-09-07 9:52 ` Richard Guenther
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2010-09-06 18:18 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 437 bytes --]
On Sep 5, 2010, Richard Guenther <richard.guenther@gmail.com> wrote:
> + If OEP_SAME_TYPE is set, then mismatched types */
> there seems to be missing sth.
Ugh. Sorry.
> Thus, simply add a type equality check for operand 1 in operand_equal_p here:
Thanks. I'm going ahead and hashing the main type variant of operand 1,
and discarding qualifiers and checking types when dumping trees. How
does this look? (pending regstrap)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mem-ref-same-type.patch --]
[-- Type: text/x-diff, Size: 3770 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/45419
PR debug/45408
* tree-pretty-print.c (dump_generic_node): Disregard top-level
qualifiers in otherwise equal MEM_REF pointer types.
* fold-const.c (operand_equal_p): Compare pointer type of MEM_REFs.
* tree.c (iterative_hash_expr): Hash the pointer type of MEM_REFs.
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c.orig 2010-09-05 09:17:52.542299276 -0300
+++ gcc/tree-pretty-print.c 2010-09-06 14:50:07.000000000 -0300
@@ -807,8 +807,8 @@ dump_generic_node (pretty_printer *buffe
== TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
&& (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
- && (TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 0)))
- == TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 1))))
+ && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 0)))
+ == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1))))
/* Same value types ignoring qualifiers. */
&& (TYPE_MAIN_VARIANT (TREE_TYPE (node))
== TYPE_MAIN_VARIANT
@@ -827,9 +827,12 @@ dump_generic_node (pretty_printer *buffe
}
else
{
+ tree ptype;
+
pp_string (buffer, "MEM[");
pp_string (buffer, "(");
- dump_generic_node (buffer, TREE_TYPE (TREE_OPERAND (node, 1)),
+ ptype = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1)));
+ dump_generic_node (buffer, ptype,
spc, flags | TDF_SLIM, false);
pp_string (buffer, ")");
dump_generic_node (buffer, TREE_OPERAND (node, 0),
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c.orig 2010-09-05 09:17:52.620425283 -0300
+++ gcc/fold-const.c 2010-09-06 14:32:03.000000000 -0300
@@ -2593,14 +2593,17 @@ operand_equal_p (const_tree arg0, const_
return OP_SAME (0);
case MEM_REF:
- /* Require equal access sizes. We can have incomplete types
- for array references of variable-sized arrays from the
- Fortran frontent though. */
+ /* Require equal access sizes, and similar pointer types.
+ We can have incomplete types for array references of
+ variable-sized arrays from the Fortran frontent
+ though. */
return ((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1))
|| (TYPE_SIZE (TREE_TYPE (arg0))
&& TYPE_SIZE (TREE_TYPE (arg1))
&& operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
TYPE_SIZE (TREE_TYPE (arg1)), flags)))
+ && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg0, 1)))
+ == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg1, 1))))
&& OP_SAME (0) && OP_SAME (1));
case ARRAY_REF:
Index: gcc/tree.c
===================================================================
--- gcc/tree.c.orig 2010-09-05 09:17:52.677304476 -0300
+++ gcc/tree.c 2010-09-06 14:46:57.000000000 -0300
@@ -6734,6 +6734,21 @@ iterative_hash_expr (const_tree t, hashv
}
return val;
}
+ case MEM_REF:
+ {
+ tree type = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (t, 1)));
+
+ /* The type of the second operand is irrelevant, except for
+ its top-level qualifiers. */
+ val = iterative_hash_object (TYPE_HASH (type), val);
+
+ /* We could use the standard hash computation from this point
+ on. */
+ val = iterative_hash_object (code, val);
+ val = iterative_hash_expr (TREE_OPERAND (t, 1), val);
+ val = iterative_hash_expr (TREE_OPERAND (t, 0), val);
+ return val;
+ }
case FUNCTION_DECL:
/* When referring to a built-in FUNCTION_DECL, use the __builtin__ form.
Otherwise nodes that compare equal according to operand_equal_p might
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-06 18:18 ` Alexandre Oliva
@ 2010-09-07 9:52 ` Richard Guenther
2010-09-08 22:27 ` Alexandre Oliva
0 siblings, 1 reply; 8+ messages in thread
From: Richard Guenther @ 2010-09-07 9:52 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
On Mon, Sep 6, 2010 at 7:57 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Sep 5, 2010, Richard Guenther <richard.guenther@gmail.com> wrote:
>
>> + If OEP_SAME_TYPE is set, then mismatched types */
>
>> there seems to be missing sth.
>
> Ugh. Sorry.
>
>> Thus, simply add a type equality check for operand 1 in operand_equal_p here:
>
> Thanks. I'm going ahead and hashing the main type variant of operand 1,
> and discarding qualifiers and checking types when dumping trees. How
> does this look? (pending regstrap)
+ /* The type of the second operand is irrelevant, except for
+ its top-level qualifiers. */
+ val = iterative_hash_object (TYPE_HASH (type), val);
I suppose this was to be "The type of the second operand is relevant, ..."
Ok with that change.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-07 9:52 ` Richard Guenther
@ 2010-09-08 22:27 ` Alexandre Oliva
2010-09-09 1:24 ` H.J. Lu
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2010-09-08 22:27 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 521 bytes --]
On Sep 7, 2010, Richard Guenther <richard.guenther@gmail.com> wrote:
> + /* The type of the second operand is irrelevant, except for
> + its top-level qualifiers. */
> + val = iterative_hash_object (TYPE_HASH (type), val);
> I suppose this was to be "The type of the second operand is relevant, ..."
> Ok with that change.
Thanks. I had to apply a similar fix to the code that dumps MEM_REFs
within COMPONENT_REFs, that I'm going ahead and checking in as obvious.
Here's the complete patch.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mem-ref-same-type.patch --]
[-- Type: text/x-diff, Size: 4401 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/45419
PR debug/45408
* tree-pretty-print.c (dump_generic_node): Disregard top-level
qualifiers in otherwise equal MEM_REF pointer types.
* fold-const.c (operand_equal_p): Compare pointer type of MEM_REFs.
* tree.c (iterative_hash_expr): Hash the pointer type of MEM_REFs.
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c.orig 2010-09-06 14:50:54.291300126 -0300
+++ gcc/tree-pretty-print.c 2010-09-07 17:06:00.000000000 -0300
@@ -807,8 +807,8 @@ dump_generic_node (pretty_printer *buffe
== TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
&& (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
- && (TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 0)))
- == TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 1))))
+ && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 0)))
+ == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1))))
/* Same value types ignoring qualifiers. */
&& (TYPE_MAIN_VARIANT (TREE_TYPE (node))
== TYPE_MAIN_VARIANT
@@ -827,9 +827,12 @@ dump_generic_node (pretty_printer *buffe
}
else
{
+ tree ptype;
+
pp_string (buffer, "MEM[");
pp_string (buffer, "(");
- dump_generic_node (buffer, TREE_TYPE (TREE_OPERAND (node, 1)),
+ ptype = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1)));
+ dump_generic_node (buffer, ptype,
spc, flags | TDF_SLIM, false);
pp_string (buffer, ")");
dump_generic_node (buffer, TREE_OPERAND (node, 0),
@@ -1161,8 +1164,8 @@ dump_generic_node (pretty_printer *buffe
== TYPE_MODE (TREE_TYPE (TREE_OPERAND (op0, 1))))
&& (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (op0, 0)))
== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (op0, 1))))
- && (TYPE_QUALS (TREE_TYPE (TREE_OPERAND (op0, 0)))
- == TYPE_QUALS (TREE_TYPE (TREE_OPERAND (op0, 1))))
+ && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (op0, 1))))
/* Same value types ignoring qualifiers. */
&& (TYPE_MAIN_VARIANT (TREE_TYPE (op0))
== TYPE_MAIN_VARIANT
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c.orig 2010-09-06 14:50:54.313302817 -0300
+++ gcc/fold-const.c 2010-09-06 14:50:58.000000000 -0300
@@ -2593,14 +2593,17 @@ operand_equal_p (const_tree arg0, const_
return OP_SAME (0);
case MEM_REF:
- /* Require equal access sizes. We can have incomplete types
- for array references of variable-sized arrays from the
- Fortran frontent though. */
+ /* Require equal access sizes, and similar pointer types.
+ We can have incomplete types for array references of
+ variable-sized arrays from the Fortran frontent
+ though. */
return ((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1))
|| (TYPE_SIZE (TREE_TYPE (arg0))
&& TYPE_SIZE (TREE_TYPE (arg1))
&& operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
TYPE_SIZE (TREE_TYPE (arg1)), flags)))
+ && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg0, 1)))
+ == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg1, 1))))
&& OP_SAME (0) && OP_SAME (1));
case ARRAY_REF:
Index: gcc/tree.c
===================================================================
--- gcc/tree.c.orig 2010-09-06 14:50:54.291300126 -0300
+++ gcc/tree.c 2010-09-07 17:08:24.000000000 -0300
@@ -6734,6 +6734,21 @@ iterative_hash_expr (const_tree t, hashv
}
return val;
}
+ case MEM_REF:
+ {
+ /* The type of the second operand is relevant, except for
+ its top-level qualifiers. */
+ tree type = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (t, 1)));
+
+ val = iterative_hash_object (TYPE_HASH (type), val);
+
+ /* We could use the standard hash computation from this point
+ on. */
+ val = iterative_hash_object (code, val);
+ val = iterative_hash_expr (TREE_OPERAND (t, 1), val);
+ val = iterative_hash_expr (TREE_OPERAND (t, 0), val);
+ return val;
+ }
case FUNCTION_DECL:
/* When referring to a built-in FUNCTION_DECL, use the __builtin__ form.
Otherwise nodes that compare equal according to operand_equal_p might
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-08 22:27 ` Alexandre Oliva
@ 2010-09-09 1:24 ` H.J. Lu
2010-09-11 0:22 ` Alexandre Oliva
0 siblings, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2010-09-09 1:24 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: Richard Guenther, gcc-patches
On Wed, Sep 8, 2010 at 2:51 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Sep 7, 2010, Richard Guenther <richard.guenther@gmail.com> wrote:
>
>> + /* The type of the second operand is irrelevant, except for
>> + its top-level qualifiers. */
>> + val = iterative_hash_object (TYPE_HASH (type), val);
>
>> I suppose this was to be "The type of the second operand is relevant, ..."
>
>> Ok with that change.
>
> Thanks. I had to apply a similar fix to the code that dumps MEM_REFs
> within COMPONENT_REFs, that I'm going ahead and checking in as obvious.
>
> Here's the complete patch.
>
>
This may have caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45604
--
H.J.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-09 1:24 ` H.J. Lu
@ 2010-09-11 0:22 ` Alexandre Oliva
2010-09-11 12:29 ` Richard Guenther
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Oliva @ 2010-09-11 0:22 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Guenther, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]
On Sep 8, 2010, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> This may have caused:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45604
Thanks. Indeed, it did. Sorry that I didn't catch this, I seem to have
used an already-broken baseline for the regression testing.
In spite of the comments above, I missed that we're not to issue a type
cast between pointer-to-T and reference-to-T, or vice-versa.
I implemented a test to cover that exception, but then I realized that,
once we know we have pointer-to-T or reference-to-T, it doesn't matter,
for purposes of tree dumping, whether the actual pointer types are even
related, does it?
Indeed, I checked that, once these cases were covered, we'd never get a
hit. Turns out I caught one case in Ada and two in the C++ part of
libjava.
I only investigated the Ada one, and in that case we had two types, each
its own main type variant, that were AFAICT identical pointer types to
the same type. The tree dump for both types was bitwise identical.
I took this as confirmation that comparing the top-level type of the two
operands of MEM_REFs, after comparing the pointed-to-types, buys us
nothing. Richi, does this look right?
If so, this patch fixes the testsuite regressions, and cleans up other
unnecessary tree-dump changes that my previous patch introduced. Ok to
install? Regstrapped on x86_64-linux-gnu and i686-linux-gnu.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mem-ref-same-type-fallout.patch --]
[-- Type: text/x-diff, Size: 1475 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/45604
PR debug/45419
PR debug/45408
* tree-pretty-print.c (dump_generic_node): Disregard top-level
types of MEM_REF pointer types to the same type.
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c.orig 2010-09-09 14:46:57.193300054 -0300
+++ gcc/tree-pretty-print.c 2010-09-09 23:25:34.000000000 -0300
@@ -809,8 +809,6 @@ dump_generic_node (pretty_printer *buffe
== TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
&& (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
- && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 0)))
- == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1))))
/* Same value types ignoring qualifiers. */
&& (TYPE_MAIN_VARIANT (TREE_TYPE (node))
== TYPE_MAIN_VARIANT
@@ -1173,8 +1171,6 @@ dump_generic_node (pretty_printer *buffe
== TYPE_MODE (TREE_TYPE (TREE_OPERAND (op0, 1))))
&& (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (op0, 0)))
== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (op0, 1))))
- && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (op0, 0)))
- == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (op0, 1))))
/* Same value types ignoring qualifiers. */
&& (TYPE_MAIN_VARIANT (TREE_TYPE (op0))
== TYPE_MAIN_VARIANT
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PR debug/45130] fix MEM_REF -fcompare-debug regression
2010-09-11 0:22 ` Alexandre Oliva
@ 2010-09-11 12:29 ` Richard Guenther
0 siblings, 0 replies; 8+ messages in thread
From: Richard Guenther @ 2010-09-11 12:29 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: H.J. Lu, gcc-patches
On Sat, Sep 11, 2010 at 1:01 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Sep 8, 2010, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> This may have caused:
>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45604
>
> Thanks. Indeed, it did. Sorry that I didn't catch this, I seem to have
> used an already-broken baseline for the regression testing.
>
> In spite of the comments above, I missed that we're not to issue a type
> cast between pointer-to-T and reference-to-T, or vice-versa.
>
> I implemented a test to cover that exception, but then I realized that,
> once we know we have pointer-to-T or reference-to-T, it doesn't matter,
> for purposes of tree dumping, whether the actual pointer types are even
> related, does it?
>
> Indeed, I checked that, once these cases were covered, we'd never get a
> hit. Turns out I caught one case in Ada and two in the C++ part of
> libjava.
>
> I only investigated the Ada one, and in that case we had two types, each
> its own main type variant, that were AFAICT identical pointer types to
> the same type. The tree dump for both types was bitwise identical.
>
> I took this as confirmation that comparing the top-level type of the two
> operands of MEM_REFs, after comparing the pointed-to-types, buys us
> nothing. Richi, does this look right?
>
> If so, this patch fixes the testsuite regressions, and cleans up other
> unnecessary tree-dump changes that my previous patch introduced. Ok to
> install? Regstrapped on x86_64-linux-gnu and i686-linux-gnu.
Yes, this makes sense.
Thanks,
Richard.
>
>
> --
> 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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-09-11 10:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-05 8:36 [PR debug/45130] fix MEM_REF -fcompare-debug regression Alexandre Oliva
2010-09-05 9:45 ` Richard Guenther
2010-09-06 18:18 ` Alexandre Oliva
2010-09-07 9:52 ` Richard Guenther
2010-09-08 22:27 ` Alexandre Oliva
2010-09-09 1:24 ` H.J. Lu
2010-09-11 0:22 ` Alexandre Oliva
2010-09-11 12:29 ` Richard Guenther
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).