[PATCH] Added summary info to GCDAProfiling.

[Yuchen Wu]

>> + 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?

In this case, the counters the comment is referring to are the programs (which technically never changes) and runs (which does change).

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

Good catch, also introduced a const int now.

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

They were noted earlier in the read section, but I'll add them here too for clarity.

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

Done.

I've attached the new patch with your suggestions added. 		 	   		  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-summary-info-to-GCDAProfiling.patch
Type: application/octet-stream
Size: 2572 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131104/3d80b14f/attachment.obj>