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]

Re: remove illegal if_marked usage in type_hash_marked_p


On Fri, Jul 9, 2010 at 1:13 PM, Tom de Vries <tjvries@xs4all.nl> wrote:
> The hash table type_hash_table has as if_marked function type_hash_marked_p.
> ...
> static GTY ((if_marked ("type_hash_marked_p"), param_is (struct type_hash)))
> ? ? htab_t type_hash_table;
> ...
>
> The 'TYPE_SYMTAB_POINTER (type)' clause in type_hash_marked_p has 2
> problems:
> 1. it is incorrect usage of the if_marked construct. Only testing of
> ggc_marked_p is allowed in a if_marked function, as stated in
> http://gcc.gnu.org/ml/gcc/2010-06/msg00693.html.
> 2. it has no positive effect from 4.4 onwards. From 4.4 onwards,
> -funit-at-a-time is more or less hardcoded, with as effect that if a type is
> used in any function in the unit, the type will be marked. This way, it is
> already ensured that different functions will share types and therefore
> debug info. The only effect the clause has from 4.4 onwards, is to keep
> types not used in the unit, from being garbage collected.
>
> The problematic clause was found via a debug bootstrap comparison failure
> for 4.3, as described in bug 31230.
>
> The patch is for trunk, and removes the problematic clause.
>
> The testcase is not very good, since it also passes without the patch for
> trunk, but I don't know how to make a better one.
>
> Bootstrapped and regression tested (gcc, objc, gfortran, g++, libgomp,
> libstdc++, libjava, libmudflap, libffi) on x86_64-unknown-linux-gnu with and
> without patch, with similar results.
> Same thing for a debug bootstrap.
>
> Ok for trunk? FYI I don't have copyright assignment or write access.

Ok.  I will take care of applyign it.

Thanks,
Richard.

> ? ? ? ?* tree.c (type_hash_if_marked_p): illegal if_marked usage,
> ? ? ? ?removed non-ggc_marked_p clause.
>
> Index: src/gcc/tree.c
> ===================================================================
> --- src/gcc/tree.c ? ? ?(revision 161295)
> +++ src/gcc/tree.c ? ? ?(working copy)
> @@ -5976,16 +5976,14 @@ type_hash_canon (unsigned int hashcode,
>
> ?/* See if the data pointed to by the type hash table is marked. ?We
> consider
> ? ?it marked if the type is marked or if a debug type number or symbol
> - ? table entry has been made for the type. ?This reduces the amount of
> - ? debugging output and eliminates that dependency of the debug output on
> - ? the number of garbage collections. ?*/
> + ? table entry has been made for the type. ?*/
>
> ?static int
> ?type_hash_marked_p (const void *p)
> ?{
> ? const_tree const type = ((const struct type_hash *) p)->type;
>
> - ?return ggc_marked_p (type) || TYPE_SYMTAB_POINTER (type);
> + ?return ggc_marked_p (type);
> ?}
>
> ?static void
> Index: src/gcc/testsuite/gcc.dg/debug/dwarf2/pr31230.c
> ===================================================================
> --- src/gcc/testsuite/gcc.dg/debug/dwarf2/pr31230.c ? ? (revision 0)
> +++ src/gcc/testsuite/gcc.dg/debug/dwarf2/pr31230.c ? ? (revision 0)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA --param ggc-min-expand=0 --param
> ggc-min-heapsize=0" } */
> +/* { dg-final { scan-assembler-times "DIE.*DW_TAG_array_type" 1 ?} } */
> +/* { dg-final { scan-assembler-times "DIE.*DW_TAG_subrange_type" 1 ?} } */
> +
> +void f1 (void)
> +{
> + ?char buffer1[100];
> +}
> +
> +int f2 (void)
> +{
> + ?return 0;
> +}
> +
> +void f3 (void)
> +{
> + ?char buffer2[100];
> +}
>
>


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