On Wed, Apr 27, 2011 at 8:54 AM, Jan Hubicka <[email protected]> wrote:
> Please don't forget about changelogs..
>> Index: tree.c
>> ===================================================================
>> --- tree.c (revision 172802)
>> +++ tree.c (working copy)
>> @@ -8513,14 +8513,12 @@ dump_tree_statistics (void)
>
> The crc bits was already reviewed, right?
Yes.
>> - else if (entry->checksum != checksum)
>> + else if (entry->lineno_checksum != lineno_checksum
>> + || entry->cfg_checksum != cfg_checksum)
>> {
>> error ("coverage mismatch for function %u while reading
>> execution counters",
>> fn_ident);
>> - error ("checksum is %x instead of %x", entry->checksum,
>> checksum);
>> + error ("checksum is (%x,%x) instead of (%x,%x)",
>> + entry->lineno_checksum, entry->cfg_checksum,
>> + lineno_checksum, cfg_checksum);
>
> Can't we give more informative message whether code changes or it seems to be
> optimization options disprepancy?
Good idea -- but to change the warning not the error here. For the
warning (which is promoted to error by default) currently it is:
error: coverage mismatch for function xxxx while reading counter yyy.
note: control flow checksum is aaa instead of bbb
Could be:
error: function xxx's control flow does match its profile data (counter yyy).
note: use -Wno-error=coverage-mismatch to tolerate the mismatch but
performance may drop if the function is hot.
>> +unsigned
>> +coverage_compute_cfg_checksum (void)
>> +{
>> + basic_block bb;
>> + unsigned chksum = n_basic_blocks;
>> +
>> + FOR_EACH_BB (bb)
>> + {
>> + edge e;
>> + edge_iterator ei;
> Perhaps also
> chksum = crc32_byte (chksum, bb->index)
> for cases where destination stays the same, but source of edge moves (i.e.
> with forwarder
> blocks)
Yes.
Thanks,
David
>
> Patch is OK, thanks
> Honza
>
