[PATCH] D17737: [PGO] change profile use cc1 option

[Sean Silva via cfe-commits]

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