[PATCH] D16730: [PGO] cc1 option names change for profile instrumentation (NFC)

[Sean Silva via llvm-commits]

silvas added inline comments.

================
Comment at: include/clang/Frontend/CodeGenOptions.def:106
@@ -105,3 +105,3 @@
 
-CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate
-                                        ///< execution counts to use with PGO.
+/// \brief Choose PGO instrumenation kind.
+ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNoInstr)
----------------
This is not accurate. It is not just for PGO.

================
Comment at: lib/CodeGen/BackendUtil.cpp:432
@@ -431,2 +431,3 @@
 
-  if (CodeGenOpts.ProfileInstrGenerate) {
+  switch (CodeGenOpts.getProfileInstr()) {
+  case CodeGenOptions::ProfileClangInstr: {
----------------
Keep this as an `if`.

================
Comment at: lib/CodeGen/CGStmt.cpp:1207
@@ -1205,2 +1206,3 @@
       SwitchWeights->push_back(getProfileCount(NextCase));
-    if (CGM.getCodeGenOpts().ProfileInstrGenerate) {
+    if (CGM.getCodeGenOpts().getProfileInstr() ==
+        CodeGenOptions::ProfileClangInstr) {
----------------
Is there a way to shorten this comparison? Can we add a helper like `hasProfileClangInstr()` or something instead of always having to do `getProfileInstr() == CodeGenOptions::ProfileClangInstr`? Otherwise, many of these comparisons read strange (such as the one in incrementProfileCounter, which is logically checking "is clang instr on or off", but the code as written makes it seem like it asking what kind of instr is being done).

================
Comment at: lib/Frontend/CompilerInvocation.cpp:485
@@ +484,3 @@
+    PGOInstrumentor Method = llvm::StringSwitch<PGOInstrumentor>(Value)
+                                 .Case("Clang", Clang)
+                                 .Default(Unknown);
----------------
Please use lower case ("clang") for the option value, for consistency with other options.


http://reviews.llvm.org/D16730