[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