[PATCH] D13308: Value profiling - remaining LLVM changes

Betul Buyukkurt via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 15 11:08:03 PDT 2015


> On Wed, Oct 14, 2015 at 5:49 PM, Betul Buyukkurt <betulb at codeaurora.org>
> wrote:
>>> davidxl added inline comments.
>>>
>>> ================
>>> Comment at: include/llvm/ProfileData/InstrProfReader.h:144
>>> @@ +143,3 @@
>>> +    const IntPtrT FunctionPointer;
>>> +    const uint32_t NumValueSites[IPVK_Last+1];
>>> +    const IntPtrT Values;
>>> ----------------
>>> Make NumValueSites[..] the last field of ProfileData.  The number of
>>> value
>>> profile kind also need to be recorded in the file header. This is to
>>> avoid
>>> format version bump each time a new value profile kind is added.
>>>
>>
>> I've added in my latest patch, a field to the raw header to record the
>> last element of the value kind enum at run-time. However, I think the
>> field ordering change will not help in avoiding a format version change.
>>
>
> Why not?  ProfileData now becomes a variable length record. The reader
> should be able to handle this without breaking format compatibility.

We may put the NumValueSites array as the last element but keeping it
variable length means use of flexible arrays and that's not supported in
C++. How do you want to achieve keeping the ProfileData pointer in the
RawInstrProfReader iterable (i.e. in readNextRecord)? Not possible as
it's. We'd have to be removing NumValueSites from the ProfileData struct,
and casting a uint8_t* Data pointer to ProfileData followed by to
NumValueSites[] is the solution. Is this how you'd like to proceed?

-Betul

> David
>
>
>> Justin, do you have any comments on the patch?
>>
>> -Betul
>>
>>> http://reviews.llvm.org/D13308
>>>
>>>
>>>
>>>
>>
>>
>




More information about the llvm-commits mailing list