[PATCH] InstrProf: Calculate a better function hash

Chandler Carruth chandlerc at google.com
Thu Apr 10 03:56:03 PDT 2014


Another set of diffs that really should be pulled out into a separate
commit:

     /// Assign a counter to track entry to the function body.
-    void VisitFunctionDecl(const FunctionDecl *S) {
-      CounterMap[S->getBody()] = NextCounter++;
-      Visit(S->getBody());
+    void VisitFunctionDecl(const FunctionDecl *D) {
+      assignCounter(D);
+      Visit(D->getBody());
     }

All of the simple parameter renames are fine, just commit those. No need to
clutter up the functional change.


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

> 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/89ba55f0/attachment.html>


More information about the cfe-commits mailing list