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

[Vedant Kumar via llvm-commits]

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);
  return;
}   
```


https://reviews.llvm.org/D25687