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] Add support for profile merging hooks


Zdenek Dvorak wrote:
here is the new version, including your and Honza's suggestions.
has this undergone a bootstrap w & w/o --enable-checking?


+ /* Creates a type corresponding to merger_function type.  */
+ static tree
+ build_merger_function_type ()
+ {
this doesn't need to be a separate function. Just create the fn
type in build_ctr_info_type.

*************** build_ctr_info_value (counter, type)
*** 614,619 ****
--- 636,655 ----
}
else
value = tree_cons (fields, null_pointer_node, value);
+ fields = TREE_CHAIN (fields);
+ + fn = build_decl (FUNCTION_DECL,
+ get_identifier (ctr_mergers[counter]),
+ build_merger_function_type ());
and then use TREE_TYPE (TREE_TYPE (fields)), to extract it here



  /* In lib gcov we want function linkage to be static, so we do not
     polute the global namespace. In the compiler we want it extern, so
     that they can be accessed from elsewhere.  */
this comment is now wrong.
! #if IN_LIBGCOV

! ! #else /*IN_LIBGCOV */
! ! #if IN_GCOV
could you not use #elif?


+ /* Type of function used to merge counters. */ + typedef void (*merger_function) (gcov_type *, unsigned);
this should be called gcov_merge_fn, if you need a typedef at all, that
is -- I don't think it necessary. Please have it return a gcov_type *

+ /* Information about counters. */
struct gcov_ctr_info
{
unsigned num; /* number of counters. */
gcov_type *values; /* their values. */
+ merger_function merger; /* The function used to merge them. */
and this field 'merge' (it is a command)

+ /* The default merger function.  */
+ extern void __gcov_default_merger (gcov_type *, unsigned);
and this would be better named __gcov_merge_add, because that's what it
does.

/* Chain of per-object gcov structures. */
*************** gcov_exit (void)
*** 228,233 ****
--- 238,244 ----

--- 247,256 ----
  			|| fi_ptr->n_ctrs[c_ix] * 8 != length)
  		      goto read_mismatch;
  		    c_ptr = values[c_ix];
! 		    n_counts = fi_ptr->n_ctrs[c_ix];
! 		    merger = gi_ptr->counts[c_ix].merger;
! 		    (*merger) (c_ptr, n_counts);
! 		    c_ptr += n_counts;
  		    values[c_ix] = c_ptr;
  		    c_ix++;
  		}
the merger temporary is not needed, and if the merge fn
returns a pointer to the next counter, you can remove some
other variables too
	values[c_ix] = gi_ptr->counts[c_ix].merge (values[c_ix], fi_ptr->n_ctrs[c_ix]);
(the (*ptr) () idiom is KnR, we can use C89 now.)

+ void
+ __gcov_default_merger (counters, n_counters)
+      gcov_type *counters;
+      unsigned n_counters;
needs a comment
+ {
+   for (; n_counters; counters++, n_counters--)
+     *counters += gcov_read_counter ();
return counters;
+ }
+ #endif /* L_gcov_default_merger */
#endif /* inhibit_libc */


--
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
         The voices in my head said this was stupid too
nathan at codesourcery dot com : http://www.cs.bris.ac.uk/~nathan/ : nathan at acm dot org



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