[llvm] r204496 - InstrProf: Read raw binary profile in llvm-profdata
Duncan P. N. Exon Smith
dexonsmith at apple.com
Fri Mar 21 13:51:33 PDT 2014
On Mar 21, 2014, at 12:07 PM, Justin Bogner <mail at justinbogner.com> wrote:
> "Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
>> InstrProf: Read raw binary profile in llvm-profdata
>>
>> Read a raw binary profile that corresponds to a memory dump from the
>> runtime profile.
>>
>> The test is a binary file generated from
>> cfe/trunk/test/Profile/c-general.c with the new compiler-rt runtime and
>> the matching text version of the input. It includes instructions on how
>> to regenerate.
> ...
>> error_code InstrProfReader::create(std::string Path,
>> std::unique_ptr<InstrProfReader> &Result) {
>> std::unique_ptr<MemoryBuffer> Buffer;
>> @@ -30,10 +41,19 @@ error_code InstrProfReader::create(std::
>> if (Buffer->getBufferSize() > std::numeric_limits<unsigned>::max())
>> return instrprof_error::too_large;
>>
>> - // FIXME: This needs to determine which format the file is and construct the
>> - // correct subclass.
>> - Result.reset(new TextInstrProfReader(Buffer));
>> + if (Buffer->getBufferSize() < sizeof(uint64_t)) {
>> + Result.reset(new TextInstrProfReader(Buffer));
>> + Result->readHeader();
>> + return instrprof_error::success;
>> + }
>>
>> + uint64_t Magic = *(uint64_t *)Buffer->getBufferStart();
>> + uint64_t SwappedMagic = sys::SwapByteOrder(Magic);
>> + if (Magic == getRawMagic() || SwappedMagic == getRawMagic())
>> + Result.reset(new RawInstrProfReader(Buffer));
>> + else
>> + Result.reset(new TextInstrProfReader(Buffer));
>> + Result->readHeader();
>> return instrprof_error::success;
>> }
>
> This needs to check the return code from readHeader.
r204510
> Also, it'd read
> better to abstract the format detection into the classes themselves, IE:
>
> InstrProfReader *Reader;
> if (RawInstrProfReader::hasFormat(Buffer.get()))
> Reader = new RawInstrProfReader(Buffer);
> else
> Reader = new TextInstrProfReader(Buffer);
>
> if (error_code EC = Reader->readHeader())
> return EC;
> return instrprof_error::success();
>
> The hasFormat can check if the buffer is long enough to contain the
> magic number, which avoids some duplication too.
r204511
>
>> Added: llvm/trunk/test/tools/llvm-profdata/binary.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-profdata/binary.test?rev=204496&view=auto==============================================================================
>> --- llvm/trunk/test/tools/llvm-profdata/binary.test (added)
>> +++ llvm/trunk/test/tools/llvm-profdata/binary.test Fri Mar 21 13:26:05 2014
>> @@ -0,0 +1,15 @@
>> +REGENERATE: You need a checkout of clang with compiler-rt to generate the
>> +REGENERATE: binary file here. These shell commands can be used to regenerate
>> +REGENERATE: it.
>> +REGENERATE:
>> +REGENERATE: $ SRC=path/to/llvm
>> +REGENERATE: $ CFE=$SRC/tools/clang
>> +REGENERATE: $ TESTDIR=$SRC/test/tools/llvm-profdata
>> +REGENERATE: $ CFE_TESTDIR=$CFE/test/Profile
>> +REGENERATE: $ clang -o a.out -fprofile-instr-generate $CFE_TESTDIR/test/Profile/c-general.c
>> +REGENERATE: $ LLVM_PROFILE_FILE=$TESTDIR/Inputs/binary.profdata ./a.out
>> +REGENERATE: $ cp $CFE_TESTDIR/Inputs/c-general.profdata $TESTDIR/Inputs/binary-compare.profdata
>> +
>> +RUN: llvm-profdata show %p/Inputs/binary.profdata -o %t1
>> +RUN: llvm-profdata show %p/Inputs/binary-compare.profdata -o %t2
>> +RUN: diff -up %t1 %t2
>
> Why include binary-compare.profdata at all? Use show on the
> binary.profdata and pipe into filecheck, that way you (1) don't need to
> include the text formatted version, and (2) get better error messages.
Good point. I’ll fix this soon.
More information about the llvm-commits
mailing list