[PATCH] InstrProf: Calculate a better function hash

[Chandler Carruth]

Still looking at the patch but:

    Using hash values with the most significant bit set exposed a bug where
    unsigned values were read using strtol and strtoll.  Changed to strtoul
    and strtoull to keep the tests passing.

This bit is clearly correct, and I'm happy for it to get exercised once the
final bits land. I would split this out into its own patch and go ahead and
commit it. The hash change is better as a separate patch IMO.


On Thu, Apr 10, 2014 at 11:45 AM, Chandler Carruth <chandlerc at google.com>wrote:

> Sorry for delays, took vacation around EuroLLVM... I'll try to catch up on
> some of these, but will probably only hit full steam again next week.
>
> On Wed, Mar 26, 2014 at 11:27 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>> The hash here (Working) is just like any other number in IRGen.  Its byte-
>> swapping requirements are no different from, e.g.,
>> CodeGenPGO::NumRegionCounters.
>>
>> Here’s the flow (with the current file formats):
>>
>>   - IRGen emits an i64 as part of the __llvm_profile_data_foo aggregate.
>>
>>   - CodeGen emits it in the right section, byte swapping if host!=target.
>>
>>   - The profile runtime dumps it in the raw binary format.  This code is
>> in
>>     compiler-rt:lib/profile/InstrProfilingFile.c and
>>     compiler-rt:lib/profile/InstrProfilingBuffer.c.
>>
>>   - llvm-profdata reads it in, byte swapping if host!=target.  This code
>> is
>>     in llvm:lib/ProfileData/InstrProfReader.cpp.
>>
>
> Ahh, ok. The part I was missing here was that the raw format is
> target-endian rather than endian neutral. That makes a lot of sense (for
> the same reason I originally pushed for having the raw format be different
> from the merged final format), but it just didn't click initially that this
> is why everything works.
>
> I'll take another look at the patch.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140410/94069eb2/attachment.html>