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

Rong Xu via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 1 10:59:22 PST 2016


xur marked an inline comment as done.

================
Comment at: lib/CodeGen/CodeGenModule.cpp:154
@@ +153,3 @@
+    assert(!CodeGenOpts.ProfileInstrumentUsePath.empty() &&
+           "Need to explicitly specify the profile name.");
+    auto ReaderOrErr = llvm::IndexedInstrProfReader::create(
----------------
silvas wrote:
> Are we ensuring consistency in the driver? If not, we definitely want to do that.
I think in driver level, the change change is consistent with the old behavior: driver will always set option -fprofile-instrument-use-path= for the pgo use compilation,
i.e. when -fprofile-instru-use{=<path>} or -fprofile-use{=<path>} is in the command line, we will always have -fprofile-instrument-use-path=<path>,
It will be translated to CodeGenOpts.ProfileInstrumentUsePath in ParseCodeGenArgs(). 

In the cc1 level, the behavior changed: old behavior is:
-fprofile-instru-use=<path> will trigger the use compilation.
Now requires two cc1 opitons:
-fprofile-instrument=clang -fprofile-instrument-use-path=<path>.
Having -fprofile-instrument=clang only will trigger the assertion and
having -fprofile-instrument-use-path=<path> is vold op.
I did this because the old pgo use cc1 option requires the profile path.

This is contrast in the IR level instrument, where -fprofile-instrument-use-path=<path> is optional: we will use default.profdata in the absence.



================
Comment at: lib/Frontend/CompilerInvocation.cpp:381
@@ +380,3 @@
+                               OptSpecifier Id, DiagnosticsEngine &Diags) {
+  if (Id != OPT_fprofile_instrument_EQ && Id != OPT_fprofile_instrument_use_EQ)
+    return;
----------------
silvas wrote:
> I think it would be better to assert that it is one of the two options rather than silently doing nothing.
Yes. Assertion is better. Will do in the commit.


http://reviews.llvm.org/D17737





More information about the cfe-commits mailing list