[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.


More information about the cfe-commits mailing list