[PATCH] D9009: Value profiling compiler-rt changes

[Xinliang David Li via llvm-commits]

32bit case does not seem to work well.
1) a subtle problem is that __llvm_profile_data struct is not declared to
match the instrumentation in terms of alignment, sizeof operator returns
smaller value than that is actually allocated -- the result is that some
profile-data may get silently dropped/truncated during dump.
2) 32 bit reader also assumes 4 byte alignment while the actual data is 8
byte aligned.

The fix is simple, just use LLVM_ALIGNAS macro when declaring prof data
structure (in both runtime and InstrProf.h). In runtime, we will have to
duplicate the def of LLVM_ALIGNAS macro for now.  With this change, cross
platform reading are also fine.

This is actually an old bug exposed by this change. The old layout of
ProfileData happens to work (multiples of 8 bytes).

David

On Sun, Nov 15, 2015 at 1:25 PM, Xinliang David Li <davidxl at google.com>
wrote:

> Let me reexamine the code a little more to make sure reading cross
> platform profiling reading (32 --> 64, 64->64) will be ok (I suppose there
> are test coverage on that).
>
> David
>
> On Sun, Nov 15, 2015 at 1:11 PM, Betul Buyukkurt <betulb at codeaurora.org>
> wrote:
>
>>
>> Hi David,
>>
>> Alignment is set to be 8 bytes for the ProfileData struct elements in
>> lib/Transforms/Instrumentation/InstrProfiling.cpp through the line:
>>
>>  Data->setAlignment(8);
>>
>> I also checked the size of the ProfileData struct during raw test file
>> updates. It came up to be 40 bytes i.e. assigning 6 bytes of padding to
>> the 34 byte actual ProfileData struct size on a 32 byte architectures.
>>
>> I just got my commit rights. I'll merge the patches once I have an LGTM on
>> them.
>>
>> -Betul
>>
>> > This may not be true for 32 bit target.
>> >
>> > David
>> >
>> > On Sun, Nov 15, 2015 at 6:23 AM, Betul Buyukkurt <betulb at codeaurora.org
>> > <mailto:betulb at codeaurora.org> > wrote:
>> >
>> >
>> > betulb updated this revision to Diff 40236.
>> > betulb added a comment.
>> >
>> > In this revision:
>> >
>> > - Removed padding computation after the Data section. It's not needed as
>> > the
>> > ProfileData struct is 8 byte aligned.
>> >
>> >
>> >
>> > http://reviews.llvm.org/D9009
>> >
>> > Files:
>> >   lib/profile/InstrProfiling.c
>> >   lib/profile/InstrProfiling.h
>> >   lib/profile/InstrProfilingBuffer.c
>> >   lib/profile/InstrProfilingFile.c
>> >
>> >
>> >
>> >
>> >
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151115/d97193d1/attachment.html>