[PATCH] Added summary info to GCDAProfiling.

[Justin Bogner]

Yuchen Wu <yuchenericwu at hotmail.com> writes:
> Added summary info to GCDAProfiling.
>
> This function will be called by GCOVProfiling to write and update object
> and program summaries to be read in by llvm-cov.
>
> From cc52684a2ae7a71fffa9e86dbd2523f9f1f08dc0 Mon Sep 17 00:00:00 2001
> From: Yuchen Wu <yuchen_wu at apple.com>
> Date: Tue, 29 Oct 2013 17:35:07 -0700
> Subject: [PATCH] Added summary info to GCDAProfiling.
>
> This function will be called by GCOVProfiling to write and update object
> and program summaries to be read in by llvm-cov.
> ---
>  lib/profile/GCDAProfiling.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/lib/profile/GCDAProfiling.c b/lib/profile/GCDAProfiling.c
> index dcd7662..e398b57 100644
> --- a/lib/profile/GCDAProfiling.c
> +++ b/lib/profile/GCDAProfiling.c
> @@ -366,7 +366,7 @@ void llvm_gcda_emit_arcs(uint32_t num_counters, uint64_t *counters) {
>    if (val != (uint32_t)-1) {
>      /* There are counters present in the file. Merge them. */
>      if (val != 0x01a10000) {
> -      fprintf(stderr, "profiling:invalid magic number (0x%08x)\n", val);
> +      fprintf(stderr, "profiling:invalid arc tag (0x%08x)\n", val);
>        return;
>      }
>  
> @@ -400,6 +400,53 @@ void llvm_gcda_emit_arcs(uint32_t num_counters, uint64_t *counters) {
>  #endif
>  }
>  
> +void llvm_gcda_summary_info() {
> +  uint32_t i;
> +  uint32_t runs = 1;
> +  uint32_t val = 0;
> +  uint64_t save_cur_pos = cur_pos;
> +
> +  if (!output_file) return;
> +
> +  val = read_32bit_value();
> +
> +  if (val != (uint32_t)-1) {
> +    /* There are counters present in the file. Merge them. */

I'm pretty sure this comment makes no sense here - copy pasted from the
other function?

> +    if (val != 0xa1000000) {
> +      fprintf(stderr, "profiling:invalid object tag (0x%08x)\n", val);
> +      return;
> +    }
> +
> +    val = read_32bit_value(); // length
> +    if (val == (uint32_t)-1 || val != 9) {

This seems a little redundant. Also, maybe this 9 should have a name,
since we need it a few times.

> +      fprintf(stderr, "profiling:invalid object length (%d)\n", val); // length
> +      return;
> +    }
> +
> +    read_32bit_value(); // checksum, unused
> +    read_32bit_value(); // num, unused
> +    runs += read_32bit_value(); // add previous run count to new counter
> +  }
> +
> +  cur_pos = save_cur_pos;
> +
> +  /* Object summary tag */
> +  write_bytes("\0\0\0\xa1", 4);
> +  write_32bit_value(9); // hard-coded to 9 to be gcov-compatible

The constant should make this comment unnecessary.

> +  write_32bit_value(0);
> +  write_32bit_value(0);

It'd be nice to note what these fields are.

> +  write_32bit_value(runs);
> +  for (i = 0; i < 6; ++i) write_32bit_value(0);

It's clearer what's going on if we say 3..9 here, since we can then use
our constant. Also, I think it's more readable with a newline before the
loop body.

> +
> +  /* Program summary tag */
> +  write_bytes("\0\0\0\xa3", 4); // tag indicates 1 program
> +  write_32bit_value(0); // 0 length
> +
> +#ifdef DEBUG_GCDAPROFILING
> +  fprintf(stderr, "llvmgcda:   %u runs\n", runs);
> +#endif
> +}
> +
>  void llvm_gcda_end_file() {
>    /* Write out EOF record. */
>    if (output_file) {