[PATCH] D25687: [PGO] fix bogus warning for merging empty llvm profile file

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 18 11:14:36 PDT 2016

vsk added a comment.

Thanks for splitting the patch up. Please upload diffs with more context in the future (e.g git diff -U1000).

Comment at: include/llvm/ProfileData/InstrProfReader.h:108
+  /// instrprof file. Nullptr will be returned for an empty profile file.
   static Expected<std::unique_ptr<InstrProfReader>> create(const Twine &Path);
This breaks CoverageMapping::load(), and creates an inconsistency between the API's for InstrProfReader::create and IndexedInstrProfReader::create (one can successfully return nullptr, the other can't).

I'd prefer for this functionality to be implemented via a new instrprof_error. Clients can opt into handling this as a 'soft'/recoverable error, or fail with an appropriate diagnostic packaged up by the Error.

Comment at: tools/llvm-profdata/llvm-profdata.cpp:149
+  if (!Reader)
+    return;
   bool IsIRProfile = Reader->isIRLevelProfile();
In llvm-profdata's loadInput, you could write:

if ((Error E = ReaderOrErr.takeError())) {
  if (E != instrprof_error::empty_profile)
    WC->Err = std::move(E);


More information about the llvm-commits mailing list