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 Wed, 7 Oct 2009, Diego Novillo wrote:

> 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.

I've split the patch and installed the following first piece.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2009-10-08  Richard Guenther  <rguenther@suse.de>

	* gimple.c (free_gimple_type_tables): New function.
	* gimple.h (free_gimple_type_tables): Declare.

	lto/
	* lto.c (read_cgraph_and_symbols): Free the gimple type merging
	hash tables.

Index: trunk/gcc/gimple.c
===================================================================
*** trunk.orig/gcc/gimple.c	2009-10-07 17:13:22.000000000 +0200
--- trunk/gcc/gimple.c	2009-10-08 15:33:39.000000000 +0200
*************** print_gimple_types_stats (void)
*** 3899,3906 ****
    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,
--- 3899,3906 ----
    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,
*************** print_gimple_types_stats (void)
*** 3910,3915 ****
--- 3910,3941 ----
      fprintf (stderr, "GIMPLE type comparison table is empty\n");
  }
  
+ /* Free the gimple type hashtables used for LTO type merging.  */
+ 
+ void
+ free_gimple_type_tables (void)
+ {
+   /* Last chance to print stats for the tables.  */
+   if (flag_lto_report)
+     print_gimple_types_stats ();
+ 
+   if (gimple_types)
+     {
+       htab_delete (gimple_types);
+       gimple_types = NULL;
+     }
+   if (type_hash_cache)
+     {
+       pointer_map_destroy (type_hash_cache);
+       type_hash_cache = NULL;
+     }
+   if (gtc_visited)
+     {
+       htab_delete (gtc_visited);
+       gtc_visited = NULL;
+     }
+ }
+ 
  
  /* Return a type the same as TYPE except unsigned or
     signed according to UNSIGNEDP.  */
Index: trunk/gcc/gimple.h
===================================================================
*** trunk.orig/gcc/gimple.h	2009-10-07 17:13:22.000000000 +0200
--- trunk/gcc/gimple.h	2009-10-08 15:32:18.000000000 +0200
*************** extern void gimple_force_type_merge (tre
*** 919,924 ****
--- 919,925 ----
  extern int gimple_types_compatible_p (tree, tree);
  extern tree gimple_register_type (tree);
  extern void print_gimple_types_stats (void);
+ extern void free_gimple_type_tables (void);
  extern tree gimple_unsigned_type (tree);
  extern tree gimple_signed_type (tree);
  extern alias_set_type gimple_get_alias_set (tree);
Index: trunk/gcc/lto/lto.c
===================================================================
*** trunk.orig/gcc/lto/lto.c	2009-10-08 10:42:29.000000000 +0200
--- trunk/gcc/lto/lto.c	2009-10-08 15:31:39.000000000 +0200
*************** read_cgraph_and_symbols (unsigned nfiles
*** 1844,1849 ****
--- 1844,1852 ----
    /* Fixup all decls and types.  */
    lto_fixup_decls (all_file_decl_data);
  
+   /* Free the type hash tables.  */
+   free_gimple_type_tables ();
+ 
    /* FIXME lto. This loop needs to be changed to use the pass manager to
       call the ipa passes directly.  */
    if (!errorcount)

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