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: [PATCH][LTO] Fix PR41576, hide gimple_types_compatible_p better


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, &gtc_visited);
> + }
>
> + int
> + gimple_types_structurally_equivalent_p (tree type1, tree type2)
> + {
> + Â return compare_and_maybe_complete_types (type1, type2, true, &gtse_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.


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