[PATCH][LTO] Fix PR41576, hide gimple_types_compatible_p better

Diego Novillo dnovillo@google.com
Wed Oct 7 13:37:00 GMT 2009


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.



More information about the Gcc-patches mailing list