silvas added a comment.
Thanks, this is looking good. I have some naming suggestions and nits, but the
overall content of the patch LGTM.
================
Comment at: include/clang/Driver/CC1Options.td:286
@@ +285,3 @@
+def fprofile_instrument_usepath_EQ :
+ Joined<["-"], "fprofile-instrument-usepath=">,
+ HelpText<"Specify the profile path in PGO use compilation">;
----------------
Small bikeshed: `usepath` reads weird to me. Can we make it `use-path`?
================
Comment at: include/clang/Frontend/CodeGenOptions.h:237
@@ +236,3 @@
+ /// \brief Check if Clang profile use is on.
+ bool hasProfileClangUse () const {
+ return getProfileUse() == ProfileClangInstr;
----------------
No space before the parens. Here and elsewhere.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:378
@@ -377,1 +377,3 @@
+enum PGOInstrumentor { ProfileUnknown, ProfileNone, ProfileClang, ProfileLLVM
};
+static PGOInstrumentor getPGOInstrumentor(StringRef S) {
----------------
Can we reuse the enum used for CodeGenOpts? Please add a comment if not.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:533
@@ +532,3 @@
+ Args.getLastArgValue(OPT_fprofile_instrument_path_EQ);
+ Opts.InstrProfileInput =
+ Args.getLastArgValue(OPT_fprofile_instrument_usepath_EQ);
----------------
Maybe consider renaming these variables in a separate patch. The current naming
is pretty confusing since it makes it seems like InstrProfileOutput mirrors
InstrProfileInput, but they are totally unrelated. Maybe
`InstrProfileDefaultProfrawPath` and `InstrProfileUsePath`, respectively.
Maybe in this patch rename `InstrProfileInput` to `ProfileInstrumentUsePath` to
match the option name?
http://reviews.llvm.org/D17737
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
