This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][LTO] Fix PR41576, hide gimple_types_compatible_p better
- From: Diego Novillo <dnovillo at google dot com>
- To: Richard Guenther <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 7 Oct 2009 09:34:27 -0400
- Subject: Re: [PATCH][LTO] Fix PR41576, hide gimple_types_compatible_p better
- References: <alpine.LNX.2.00.0910061731330.4520@zhemvz.fhfr.qr>
On Tue, Oct 6, 2009 at 11:39, Richard Guenther <rguenther@suse.de> wrote:
> --- 3187,3199 ----
> Â Âreturn false;
> Â}
>
> ! /* If STRUCTURALLY_P is false return 1 iff T1 and T2 are identical.
> ! Â ÂIf STRUCTURALLY_P is true return 1 iff T1 and T2 are structurally
> ! Â Âidentical. ÂOtherwise, return 0. ÂUse CACHE for caching queries. Â*/
There should be an explanation about what it means to be
'identical' vs 'structurally identical'. An example involving
qualifiers maybe. Perhaps rename 'structurally identical' to
'structurally equivalent'?
> + int
> + gimple_types_compatible_p (tree type1, tree type2)
> + {
> + Â return compare_and_maybe_complete_types (type1, type2, false, >c_visited);
> + }
>
> + int
> + gimple_types_structurally_equivalent_p (tree type1, tree type2)
> + {
> + Â return compare_and_maybe_complete_types (type1, type2, true, >se_visited);
> + }
>
These two need comments.
> Âvoid
> ! print_gimple_types_stats (void)
> Â{
> Â Âif (gimple_types)
> ! Â Â fprintf (stderr, "GIMPLE type table: size %ld, %ld elements, "
> ! Â Â Â Â Â Â"%ld searches, %ld collisions (ratio: %f)\n",
> ! Â Â Â Â Â Â(long) htab_size (gimple_types),
> ! Â Â Â Â Â Â(long) htab_elements (gimple_types),
> ! Â Â Â Â Â Â(long) gimple_types->searches,
> ! Â Â Â Â Â Â(long) gimple_types->collisions,
> ! Â Â Â Â Â Âhtab_collisions (gimple_types));
> ! Â else
> ! Â Â fprintf (stderr, "GIMPLE type table is empty\n");
> Â Âif (gtc_visited)
> ! Â Â fprintf (stderr, "GIMPLE type comparison table: size %ld, %ld elements, "
> ! Â Â Â Â Â Â"%ld searches, %ld collisions (ratio: %f)\n",
> ! Â Â Â Â Â Â(long) htab_size (gtc_visited),
> ! Â Â Â Â Â Â(long) htab_elements (gtc_visited),
> ! Â Â Â Â Â Â(long) gtc_visited->searches,
> ! Â Â Â Â Â Â(long) gtc_visited->collisions,
> ! Â Â Â Â Â Âhtab_collisions (gtc_visited));
> ! Â else
> ! Â Â fprintf (stderr, "GIMPLE type comparison table is empty\n");
> Â}
Please keep print_gimple_type_stats and make
free_gimple_type_tables call it. No point adding extraneous
side-effects to a reporting function and the print_* routines are
useful inside gdb sessions or -fdump-... flags.
> Index: trunk/gcc/lto-streamer.c
> ===================================================================
> *** trunk.orig/gcc/lto-streamer.c    2009-10-06 14:15:42.000000000 +0200
> --- trunk/gcc/lto-streamer.c  Â2009-10-06 14:19:25.000000000 +0200
> *************** print_lto_report (void)
> *** 196,202 ****
> Â Â Â Â Â lto_stats.num_function_bodies);
>
> Â Âfprintf (stderr, "[%s] ", s);
> - Â print_gimple_types_stats ();
Keep this. The call to print_lto_report should happen
before we free the hash tables now.
OK with those changes.
Diego.