[PATCH] Fix up PCH with DECL_VALUE_EXPR/DECL_DEBUG_EXPR
NightStrike
nightstrike@gmail.com
Mon Jun 7 19:29:00 GMT 2010
On Wed, Jun 2, 2010 at 3:40 PM, Richard Guenther <rguenther@suse.de> wrote:
> On Wed, 2 Jun 2010, Jakub Jelinek wrote:
>
>> Hi!
>>
>> During regtesting PR44367 patch on i686-linux I saw weird PCH errors on
>> libstdc++.
>> The problem seems to be that DECL_VALUE_EXPR/DECL_DEBUG_EXPR hash tables
>> hash using htab_hash_pointer (and even store that hash value into the
>> tree_map structure). When writing PCH, the addresses of the decls in the
>> hash table change, but the hash table isn't rehashed, nor ->hash fields
>> updated, which results in DECL_HAS_VALUE_EXPR_P being set after PCH read,
>> but DECL_VALUE_EXPR for it returning NULL (the right decl is in the hash
>> table, but not where the hash lookup will look for it).
>>
>> In both these cases (and for shadowed_var_for_decl too) only decls
>> are used as from trees, so this patch fixes it by using DECL_UID as hash
>> instead of htab_hash_pointer. Furthermore, I think doing DECL_UID
>> on the from tree is cheap enough that it isn't worth wasting 8 bytes per
>> decl with DECL_VALUE_EXPR for the cached hash value.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Ok.
>
> Thanks,
> Richard.
>
>> 2010-06-02 Jakub Jelinek <jakub@redhat.com>
>>
>> * tree.h (struct tree_decl_map): New type.
>> (tree_decl_map_eq, tree_decl_map_marked_p): Define.
>> (tree_decl_map_hash): New prototype.
>> (debug_expr_for_decl, value_expr_for_decl): Change into
>> tree_decl_map hashtab from tree_map.
>> (init_ttree): Adjust initialization.
>> (tree_decl_map_hash): New function.
>> (decl_debug_expr_lookup, decl_debug_expr_insert,
>> decl_value_expr_lookup, decl_value_expr_insert): Adjust.
>> cp/
>> * cp-objcp-common.c (shadowed_var_for_decl): Change into
>> tree_decl_map hashtab from tree_map.
>> (decl_shadowed_for_var_lookup, decl_shadowed_for_var_insert): Adjust.
>> (init_shadowed_var_for_decl): Adjust initialization.
>>
>> --- gcc/tree.h.jj 2010-06-01 12:27:48.000000000 +0200
>> +++ gcc/tree.h 2010-06-02 19:10:10.000000000 +0200
>> @@ -5382,6 +5382,17 @@ struct GTY(()) tree_map {
>> extern unsigned int tree_map_hash (const void *);
>> #define tree_map_marked_p tree_map_base_marked_p
>>
>> +/* Map from a decl tree to another tree. */
>> +
>> +struct GTY(()) tree_decl_map {
>> + struct tree_map_base base;
>> + tree to;
>> +};
>> +
>> +#define tree_decl_map_eq tree_map_base_eq
>> +extern unsigned int tree_decl_map_hash (const void *);
>> +#define tree_decl_map_marked_p tree_map_base_marked_p
>> +
>> /* Map from a tree to an int. */
>>
>> struct GTY(()) tree_int_map {
>> --- gcc/tree.c.jj 2010-05-28 14:36:04.000000000 +0200
>> +++ gcc/tree.c 2010-06-02 19:16:19.000000000 +0200
>> @@ -196,10 +196,10 @@ static GTY ((if_marked ("ggc_marked_p"),
>> /* General tree->tree mapping structure for use in hash tables. */
>>
>>
>> -static GTY ((if_marked ("tree_map_marked_p"), param_is (struct tree_map)))
>> +static GTY ((if_marked ("tree_decl_map_marked_p"), param_is (struct tree_decl_map)))
>> htab_t debug_expr_for_decl;
>>
>> -static GTY ((if_marked ("tree_map_marked_p"), param_is (struct tree_map)))
>> +static GTY ((if_marked ("tree_decl_map_marked_p"), param_is (struct tree_decl_map)))
>> htab_t value_expr_for_decl;
>>
>> static GTY ((if_marked ("tree_priority_map_marked_p"),
>> @@ -533,11 +533,11 @@ init_ttree (void)
>> type_hash_table = htab_create_ggc (TYPE_HASH_INITIAL_SIZE, type_hash_hash,
>> type_hash_eq, 0);
>>
>> - debug_expr_for_decl = htab_create_ggc (512, tree_map_hash,
>> - tree_map_eq, 0);
>> + debug_expr_for_decl = htab_create_ggc (512, tree_decl_map_hash,
>> + tree_decl_map_eq, 0);
>>
>> - value_expr_for_decl = htab_create_ggc (512, tree_map_hash,
>> - tree_map_eq, 0);
>> + value_expr_for_decl = htab_create_ggc (512, tree_decl_map_hash,
>> + tree_decl_map_eq, 0);
>> init_priority_for_decl = htab_create_ggc (512, tree_priority_map_hash,
>> tree_priority_map_eq, 0);
>>
>> @@ -5577,7 +5577,7 @@ tree_map_base_eq (const void *va, const
>> return (a->from == b->from);
>> }
>>
>> -/* Hash a from tree in a tree_map. */
>> +/* Hash a from tree in a tree_base_map. */
>>
>> unsigned int
>> tree_map_base_hash (const void *item)
>> @@ -5595,12 +5595,22 @@ tree_map_base_marked_p (const void *p)
>> return ggc_marked_p (((const struct tree_map_base *) p)->from);
>> }
>>
>> +/* Hash a from tree in a tree_map. */
>> +
>> unsigned int
>> tree_map_hash (const void *item)
>> {
>> return (((const struct tree_map *) item)->hash);
>> }
>>
>> +/* Hash a from tree in a tree_decl_map. */
>> +
>> +unsigned int
>> +tree_decl_map_hash (const void *item)
>> +{
>> + return DECL_UID (((const struct tree_decl_map *) item)->base.from);
>> +}
>> +
>> /* Return the initialization priority for DECL. */
>>
>> priority_type
>> @@ -5706,11 +5716,11 @@ print_value_expr_statistics (void)
>> tree
>> decl_debug_expr_lookup (tree from)
>> {
>> - struct tree_map *h, in;
>> + struct tree_decl_map *h, in;
>> in.base.from = from;
>>
>> - h = (struct tree_map *) htab_find_with_hash (debug_expr_for_decl, &in,
>> - htab_hash_pointer (from));
>> + h = (struct tree_decl_map *)
>> + htab_find_with_hash (debug_expr_for_decl, &in, DECL_UID (from));
>> if (h)
>> return h->to;
>> return NULL_TREE;
>> @@ -5721,15 +5731,15 @@ decl_debug_expr_lookup (tree from)
>> void
>> decl_debug_expr_insert (tree from, tree to)
>> {
>> - struct tree_map *h;
>> + struct tree_decl_map *h;
>> void **loc;
>>
>> - h = GGC_NEW (struct tree_map);
>> - h->hash = htab_hash_pointer (from);
>> + h = GGC_NEW (struct tree_decl_map);
>> h->base.from = from;
>> h->to = to;
>> - loc = htab_find_slot_with_hash (debug_expr_for_decl, h, h->hash, INSERT);
>> - *(struct tree_map **) loc = h;
>> + loc = htab_find_slot_with_hash (debug_expr_for_decl, h, DECL_UID (from),
>> + INSERT);
>> + *(struct tree_decl_map **) loc = h;
>> }
>>
>> /* Lookup a value expression for FROM, and return it if we find one. */
>> @@ -5737,11 +5747,11 @@ decl_debug_expr_insert (tree from, tree
>> tree
>> decl_value_expr_lookup (tree from)
>> {
>> - struct tree_map *h, in;
>> + struct tree_decl_map *h, in;
>> in.base.from = from;
>>
>> - h = (struct tree_map *) htab_find_with_hash (value_expr_for_decl, &in,
>> - htab_hash_pointer (from));
>> + h = (struct tree_decl_map *)
>> + htab_find_with_hash (value_expr_for_decl, &in, DECL_UID (from));
>> if (h)
>> return h->to;
>> return NULL_TREE;
>> @@ -5752,15 +5762,15 @@ decl_value_expr_lookup (tree from)
>> void
>> decl_value_expr_insert (tree from, tree to)
>> {
>> - struct tree_map *h;
>> + struct tree_decl_map *h;
>> void **loc;
>>
>> - h = GGC_NEW (struct tree_map);
>> - h->hash = htab_hash_pointer (from);
>> + h = GGC_NEW (struct tree_decl_map);
>> h->base.from = from;
>> h->to = to;
>> - loc = htab_find_slot_with_hash (value_expr_for_decl, h, h->hash, INSERT);
>> - *(struct tree_map **) loc = h;
>> + loc = htab_find_slot_with_hash (value_expr_for_decl, h, DECL_UID (from),
>> + INSERT);
>> + *(struct tree_decl_map **) loc = h;
>> }
>>
>> /* Hashing of types so that we don't make duplicates.
>> --- gcc/cp/cp-objcp-common.c.jj 2010-04-16 13:41:33.000000000 +0200
>> +++ gcc/cp/cp-objcp-common.c 2010-06-02 19:20:55.000000000 +0200
>> @@ -177,7 +177,7 @@ has_c_linkage (const_tree decl)
>> return DECL_EXTERN_C_P (decl);
>> }
>>
>> -static GTY ((if_marked ("tree_map_marked_p"), param_is (struct tree_map)))
>> +static GTY ((if_marked ("tree_decl_map_marked_p"), param_is (struct tree_decl_map)))
>> htab_t shadowed_var_for_decl;
>>
>> /* Lookup a shadowed var for FROM, and return it if we find one. */
>> @@ -185,11 +185,11 @@ static GTY ((if_marked ("tree_map_marked
>> tree
>> decl_shadowed_for_var_lookup (tree from)
>> {
>> - struct tree_map *h, in;
>> + struct tree_decl_map *h, in;
>> in.base.from = from;
>>
>> - h = (struct tree_map *) htab_find_with_hash (shadowed_var_for_decl, &in,
>> - htab_hash_pointer (from));
>> + h = (struct tree_decl_map *)
>> + htab_find_with_hash (shadowed_var_for_decl, &in, DECL_UID (from));
>> if (h)
>> return h->to;
>> return NULL_TREE;
>> @@ -200,22 +200,22 @@ decl_shadowed_for_var_lookup (tree from)
>> void
>> decl_shadowed_for_var_insert (tree from, tree to)
>> {
>> - struct tree_map *h;
>> + struct tree_decl_map *h;
>> void **loc;
>>
>> - h = GGC_NEW (struct tree_map);
>> - h->hash = htab_hash_pointer (from);
>> + h = GGC_NEW (struct tree_decl_map);
>> h->base.from = from;
>> h->to = to;
>> - loc = htab_find_slot_with_hash (shadowed_var_for_decl, h, h->hash, INSERT);
>> - *(struct tree_map **) loc = h;
>> + loc = htab_find_slot_with_hash (shadowed_var_for_decl, h, DECL_UID (from),
>> + INSERT);
>> + *(struct tree_decl_map **) loc = h;
>> }
>>
>> void
>> init_shadowed_var_for_decl (void)
>> {
>> - shadowed_var_for_decl = htab_create_ggc (512, tree_map_hash,
>> - tree_map_eq, 0);
>> + shadowed_var_for_decl = htab_create_ggc (512, tree_decl_map_hash,
>> + tree_decl_map_eq, 0);
>> }
>>
>>
>>
>> Jakub
>>
>>
>
> --
> Richard Guenther <rguenther@suse.de>
> Novell / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
>
Ping: committed?
More information about the Gcc-patches
mailing list