silvas added a comment.
I agree, David's suggestion has simplified things. This LGTM (with a couple
nits) but I'll let David give the final approval.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:153
@@ +152,3 @@
+ if (CodeGenOpts.hasProfileClangUse()) {
+ assert(!CodeGenOpts.ProfileInstrumentUsePath.empty() &&
+ "Need to explicitly specify the profile name.");
----------------
This assertion is not needed anymore. Or at least should be reworded as
"hasProfileClangUse but no ProfileInstrumentPath" or similar to clearly
indicate the expected state that should be guaranteed by the rest of the
program.
================
Comment at: lib/Driver/Tools.cpp:3318
@@ -3316,1 +3317,3 @@
+ // The default is to use Clang instrumentation profile.
+ //CmdArgs.push_back("-fprofile-instrument-use=clang");
}
----------------
This is not needed anymore after David's suggestion.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:404
@@ +403,3 @@
+ const std::string ProfileName) {
+ assert(!ProfileName.empty());
+ auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
----------------
I think this assertion is not needed since we are already handling errors below
(by deferring to clang).
================
Comment at: lib/Frontend/CompilerInvocation.cpp:406
@@ +405,3 @@
+ auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
+ // In error, reutrn silently and let Clang PGO use report the error message.
+ if (ReaderOrErr.getError()) {
----------------
typo: reutrn
http://reviews.llvm.org/D17737
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
