public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Sebastian Huber <sebastian.huber@embedded-brains.de>,
	gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] gcov: Add __gcov_info_to_gdca()
Date: Mon, 23 Nov 2020 15:30:06 +0100	[thread overview]
Message-ID: <4806f0f4-2fee-867e-87ad-6831aedd9be7@suse.cz> (raw)
In-Reply-To: <20201117095741.3143-1-sebastian.huber@embedded-brains.de>

On 11/17/20 10:57 AM, Sebastian Huber wrote:
> This is a proposal to get the gcda data for a gcda info in a free-standing
> environment.  It is intended to be used with the -fprofile-info-section option.
> A crude test program which doesn't use a linker script is:
> 
>    #include <gcov.h>
>    #include <stdio.h>
> 
>    extern const struct gcov_info *my_info;
> 
>    static void
>    filename(const char *f, void *arg)
>    {
>      printf("filename: %s\n", f);
>    }
> 
>    static void
>    dump(const void *d, unsigned n, void *arg)
>    {
>      const unsigned char *c;
>      unsigned i;
> 
>      c = d;
> 
>      for (i = 0; i < n; ++i) {
> 	printf("%02x", c[i]);
>      }
>    }
> 
>    int main()
>    {
>      __asm__ volatile (".set my_info, .LPBX2");
>      __gcov_info_to_gcda(my_info, filename, dump, NULL);
>      return 0;
>    }
> 
> gcc/
> 
> 	* doc/invoke.texi (fprofile-info-section): Mention
> 	__gcov_info_to_gdca().
> 
> libgcc/
> 
> 	Makefile.in (LIBGCOV_DRIVER): Add _gcov_info_to_gcda.
> 	gcov.h (gcov_info): Declare.
> 	(__gcov_info_to_gdca): Likewise.
> 	libgcov-driver.c (gcov_are_all_counters_zero): New.
> 	(write_one_data): Use gcov_are_all_counters_zero().
> 	(gcov_fn_info_to_gcda): New.
> 	(__gcov_info_to_gcda): Likewise.
> ---
>   gcc/doc/invoke.texi     |  73 ++++++++++++++++++++----
>   libgcc/Makefile.in      |   2 +-
>   libgcc/gcov.h           |  15 +++++
>   libgcc/libgcov-driver.c | 120 ++++++++++++++++++++++++++++++++++++----
>   4 files changed, 188 insertions(+), 22 deletions(-)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 3510a54c6c4..09cb4922f5e 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14248,17 +14248,17 @@ To optimize the program based on the collected profile information, use
>   Register the profile information in the specified section instead of using a
>   constructor/destructor.  The section name is @var{name} if it is specified,
>   otherwise the section name defaults to @code{.gcov_info}.  A pointer to the
> -profile information generated by @option{-fprofile-arcs} or
> -@option{-ftest-coverage} is placed in the specified section for each
> -translation unit.  This option disables the profile information registration
> -through a constructor and it disables the profile information processing
> -through a destructor.  This option is not intended to be used in hosted
> -environments such as GNU/Linux.  It targets systems with limited resources
> -which do not support constructors and destructors.  The linker could collect
> -the input sections in a continuous memory block and define start and end
> -symbols.  The runtime support could dump the profiling information registered
> -in this linker set during program termination to a serial line for example.  A
> -GNU linker script example which defines a linker output section follows:
> +profile information generated by @option{-fprofile-arcs} is placed in the
> +specified section for each translation unit.  This option disables the profile
> +information registration through a constructor and it disables the profile
> +information processing through a destructor.  This option is not intended to be
> +used in hosted environments such as GNU/Linux.  It targets free-standing
> +environments (for example embedded systems) with limited resources which do not
> +support constructors/destructors or the C library file I/O.
> +
> +The linker could collect the input sections in a continuous memory block and
> +define start and end symbols.  A GNU linker script example which defines a
> +linker output section follows:
>   
>   @smallexample
>     .gcov_info      :
> @@ -14269,6 +14269,57 @@ GNU linker script example which defines a linker output section follows:
>     @}
>   @end smallexample
>   
> +The program could dump the profiling information registered in this linker set
> +for example like this:
> +
> +@smallexample
> +#include <gcov.h>
> +#include <stdio.h>
> +
> +extern const struct gcov_info *__gcov_info_start[];
> +extern const struct gcov_info *__gcov_info_end[];
> +
> +static void
> +filename (const char *f, void *arg)
> +@{
> +  puts (f);
> +@}
> +
> +static void
> +dump (const void *d, unsigned n, void *arg)
> +@{
> +  const unsigned char *c = d;
> +
> +  for (unsigned i = 0; i < n; ++i)
> +    printf ("%02x", c[i]);
> +@}
> +
> +static void
> +dump_gcov_info (void)
> +@{
> +  const struct gcov_info **info = __gcov_info_start;
> +  const struct gcov_info **end = __gcov_info_end;
> +
> +  /* Obfuscate variable to prevent compiler optimizations.  */
> +  __asm__ ("" : "+r" (end));
> +
> +  while (info != end)
> +  @{
> +    void *arg = NULL;
> +    __gcov_info_to_gcda (*info, filename, dump, arg);
> +    putchar ('\n');
> +    ++info;
> +  @}
> +@}
> +
> +int
> +main()
> +@{
> +  dump_gcov_info();
> +  return 0;
> +@}
> +@end smallexample
> +
>   @item -fprofile-note=@var{path}
>   @opindex fprofile-note
>   
> diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
> index d6075d32bd4..c22413d768c 100644
> --- a/libgcc/Makefile.in
> +++ b/libgcc/Makefile.in
> @@ -908,7 +908,7 @@ LIBGCOV_INTERFACE = _gcov_dump _gcov_fork				\
>   	_gcov_execl _gcov_execlp					\
>   	_gcov_execle _gcov_execv _gcov_execvp _gcov_execve _gcov_reset  \
>   	_gcov_lock_unlock
> -LIBGCOV_DRIVER = _gcov
> +LIBGCOV_DRIVER = _gcov _gcov_info_to_gcda
>   
>   libgcov-merge-objects = $(patsubst %,%$(objext),$(LIBGCOV_MERGE))
>   libgcov-profiler-objects = $(patsubst %,%$(objext),$(LIBGCOV_PROFILER))
> diff --git a/libgcc/gcov.h b/libgcc/gcov.h
> index 0e3eed31032..371ee6feb11 100644
> --- a/libgcc/gcov.h
> +++ b/libgcc/gcov.h
> @@ -25,6 +25,8 @@
>   #ifndef GCC_GCOV_H
>   #define GCC_GCOV_H
>   
> +struct gcov_info;
> +
>   /* Set all counters to zero.  */
>   
>   extern void __gcov_reset (void);
> @@ -33,4 +35,17 @@ extern void __gcov_reset (void);
>   
>   extern void __gcov_dump (void);
>   
> +/* Convert the gcov information to a gcda data stream.  The first callback is
> +   called exactly once with the filename associated with the gcov information.
> +   The filename may be NULL.  Afterwards, the second callback is subsequently
> +   called with chunks (the begin and length of the chunk are passed as the
> +   first two arguments) of the gcda data stream.  The fourth parameter is a
> +   user-provided argument passed as the last argument to the callback
> +   functions.  */
> +
> +extern void __gcov_info_to_gcda (const struct gcov_info *,
> +				 void (*) (const char *, void *),
> +				 void (*) (const void *, unsigned, void *),
> +				 void *);
> +
>   #endif /* GCC_GCOV_H */
> diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
> index e53e4dc392a..4191b89a6f7 100644
> --- a/libgcc/libgcov-driver.c
> +++ b/libgcc/libgcov-driver.c
> @@ -26,6 +26,18 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>   #include "libgcov.h"
>   #include "gcov-io.h"
>   
> +/* Return 1, if all counter values are zero, otherwise 0. */
> +
> +static inline int
> +gcov_are_all_counters_zero (const struct gcov_ctr_info *ci_ptr)
> +{
> +  for (unsigned i = 0; i < ci_ptr->num; i++)
> +    if (ci_ptr->values[i] != 0)
> +	return 0;
> +
> +  return 1;
> +}
> +
>   #if defined(inhibit_libc)
>   /* If libc and its header files are not available, provide dummy functions.  */
>   
> @@ -428,16 +440,8 @@ write_one_data (const struct gcov_info *gi_ptr,
>   	    write_top_counters (ci_ptr, t_ix, n_counts);
>   	  else
>   	    {
> -	      /* Do not stream when all counters are zero.  */
> -	      int all_zeros = 1;
> -	      for (unsigned i = 0; i < n_counts; i++)
> -		if (ci_ptr->values[i] != 0)
> -		  {
> -		    all_zeros = 0;
> -		    break;
> -		  }
> -
> -	      if (all_zeros)
> +	      if (gcov_are_all_counters_zero (ci_ptr))
> +		/* Do not stream when all counters are zero.  */
>   		gcov_write_tag_length (GCOV_TAG_FOR_COUNTER (t_ix),
>   				       GCOV_TAG_COUNTER_LENGTH (-n_counts));
>   	      else
> @@ -637,3 +641,99 @@ __gcov_init (struct gcov_info *info)
>   #endif /* !IN_GCOV_TOOL */
>   #endif /* L_gcov */
>   #endif /* inhibit_libc */
> +
> +#if !IN_GCOV_TOOL
> +#ifdef L_gcov_info_to_gcda
> +static void
> +gcov_fn_info_to_gcda (const struct gcov_info *gi_ptr,
> +		      const struct gcov_fn_info *gfi_ptr,
> +		      void (*dump) (const void *, unsigned, void *),
> +		      void *arg)
> +{
> +  const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
> +
> +  for (unsigned t_ix = 0; t_ix < GCOV_COUNTERS; t_ix++)
> +    {
> +      if (!gi_ptr->merge[t_ix])
> +	continue;
> +
> +      if (t_ix != GCOV_COUNTER_V_TOPN && t_ix != GCOV_COUNTER_V_INDIR)
> +	{
> +	  gcov_unsigned_t word = GCOV_TAG_FOR_COUNTER (t_ix);
> +	  (*dump) (&word, sizeof (word), arg);
> +	  gcov_position_t n_counts = ci_ptr->num;
> +
> +	  if (gcov_are_all_counters_zero (ci_ptr))
> +	    {
> +	      /* Do not stream when all counters are zero.  */
> +	      word = GCOV_TAG_COUNTER_LENGTH (-n_counts);
> +	      (*dump) (&word, sizeof (word), arg);
> +	    }
> +	  else
> +	    {
> +	      word = GCOV_TAG_COUNTER_LENGTH (n_counts);
> +	      (*dump) (&word, sizeof (word), arg);
> +
> +	      for (unsigned i = 0; i < n_counts; i++)
> +		{
> +		  gcov_type value = ci_ptr->values[i];
> +		  word = (gcov_unsigned_t) value;
> +		  (*dump) (&word, sizeof (word), arg);
> +
> +		  if (sizeof (value) > sizeof (word))
> +		    word = (gcov_unsigned_t) (value >> 32);
> +		  else
> +		    word = 0;
> +
> +		  (*dump) (&word, sizeof (word), arg);
> +		}
> +	    }
> +	}
> +
> +      ci_ptr++;
> +    }
> +}
> +
> +/* Convert the gcov info to a gcda data stream.  This function does not support
> +   whole program statistics and top counters.  It is intended for free-standing
> +   environments which do not support the C library file I/O.  For the data
> +   format, see also write_one_data().  */
> +
> +void
> +__gcov_info_to_gcda (const struct gcov_info *gi_ptr,
> +		     void (*filename) (const char *, void *),
> +		     void (*dump) (const void *, unsigned, void *),
> +		     void *arg)

Hello.

I would prefer a better names for the hooks. What about something like
open_filename_hook and write_data_hook?

> +{
> +  (*filename) (gi_ptr->filename, arg);
> +  gcov_unsigned_t word = GCOV_DATA_MAGIC;
> +  (*dump) (&word, sizeof (word), arg);

And I would add a new macro like
#define GCOV_WRITE_DATA(data) (*write_data_hook) (&DATA, sizeof (DATA), arg

What do you think?

Note that we already entered a code freeze before the patch was sent to the mailing list.
That means we can install it in the next stage1.

Martin

> +  (*dump) (&gi_ptr->version, sizeof (gi_ptr->version), arg);
> +  (*dump) (&gi_ptr->stamp, sizeof (gi_ptr->stamp), arg);
> +
> +  for (unsigned f_ix = 0; f_ix != gi_ptr->n_functions; f_ix++)
> +    {
> +      word = GCOV_TAG_FUNCTION;
> +      (*dump) (&word, sizeof (word), arg);
> +
> +      const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
> +      if (gfi_ptr && gfi_ptr->key == gi_ptr)
> +	word = GCOV_TAG_FUNCTION_LENGTH;
> +      else
> +	word = 0;
> +      (*dump) (&word, sizeof (word), arg);
> +      if (!word)
> +	continue;
> +
> +      (*dump) (&gfi_ptr->ident, sizeof (gfi_ptr->ident), arg);
> +      (*dump) (&gfi_ptr->lineno_checksum,
> +	       sizeof (gfi_ptr->lineno_checksum), arg);
> +      (*dump) (&gfi_ptr->cfg_checksum, sizeof (gfi_ptr->cfg_checksum), arg);
> +      gcov_fn_info_to_gcda (gi_ptr, gfi_ptr, dump, arg);
> +    }
> +
> +  word = 0;
> +  (*dump) (&word, sizeof (word), arg);
> +}
> +#endif /* L_gcov_info_to_gcda */
> +#endif /* !IN_GCOV_TOOL */
> 


  parent reply	other threads:[~2020-11-23 14:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-17  9:57 Sebastian Huber
2020-11-20  8:37 ` Martin Liška
2020-11-20  9:25   ` Sebastian Huber
2020-11-20  9:49     ` Martin Liška
2020-11-20 10:11       ` Sebastian Huber
2020-11-20 15:25         ` Martin Liška
2020-11-20 16:14           ` Sebastian Huber
2020-11-23 12:25             ` Sebastian Huber
2020-11-23 14:24               ` Martin Liška
2020-11-23 14:30 ` Martin Liška [this message]
2020-11-23 14:35   ` Sebastian Huber
2020-11-23 14:49     ` Martin Liška
2020-11-23 14:50       ` Sebastian Huber
2020-11-23 14:55         ` Martin Liška
2021-07-13 13:03 Sebastian Huber
2021-07-13 13:11 ` Sebastian Huber

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4806f0f4-2fee-867e-87ad-6831aedd9be7@suse.cz \
    --to=mliska@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=sebastian.huber@embedded-brains.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).