Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

[Rong Xu via cfe-commits]

xur updated this revision to Diff 45708.
xur added a comment.

This new patches integrates Sean review comments:
(1) make -fprofile-ir-instr a cc1 option instead of driver option.
(2) add one cc1 option test, and test the pass is indeed invoked.
(3) remove the driver test (runtime library).
(4) fix a comment.

Thanks,

-Rong


http://reviews.llvm.org/D15829

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/pgo-instrumentation.c


Index: test/CodeGen/pgo-instrumentation.c
===================================================================
--- /dev/null
+++ test/CodeGen/pgo-instrumentation.c
@@ -0,0 +1,9 @@
+// Test PGO instrumentation.
+//
+// Ensure Pass PGOInstrumentationGenPass is invoked.
+// RUN: %clang -O2 -c -Xclang -fprofile-ir-instr -fprofile-generate %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// CHECK-PASS: PGOInstrumentationGenPass
+//
+// Ensure cc1 option -fprofile-ir-instr is null operation without existing pgo options.
+// RUN: %clang -O2 -c -Xclang -fprofile-ir-instr %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PASS2
+// CHECK-PASS2-NOT: PGOInstrumentationGenPass
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -477,8 +477,16 @@
   Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as);
   Opts.Autolink = !Args.hasArg(OPT_fno_autolink);
   Opts.SampleProfileFile = Args.getLastArgValue(OPT_fprofile_sample_use_EQ);
-  Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) ||
-      Args.hasArg(OPT_fprofile_instr_generate_EQ);
+  // Only set ProfileIRInstr when there are profile generate or use options.
+  Opts.ProfileIRInstr = Args.hasArg(OPT_fprofile_ir_instr) &&
+                        (Args.hasArg(OPT_fprofile_instr_generate) ||
+                         Args.hasArg(OPT_fprofile_instr_generate_EQ) ||
+                         Args.hasArg(OPT_fprofile_instr_use_EQ));
+  // Set Opts.ProfileInstrGenerate when Opts.ProfileIRInstr is false.
+  // Opts.ProfileInstrGenerate will be used for Clang instrumentation only.
+  if (!Opts.ProfileIRInstr)
+    Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) ||
+                                Args.hasArg(OPT_fprofile_instr_generate_EQ);
   Opts.InstrProfileOutput = Args.getLastArgValue(OPT_fprofile_instr_generate_EQ);
   Opts.InstrProfileInput = Args.getLastArgValue(OPT_fprofile_instr_use_EQ);
   Opts.CoverageMapping =
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3275,6 +3275,8 @@
       CmdArgs.push_back(Args.MakeArgString(CoverageFilename));
     }
   }
+
+  Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr);
 }
 
 static void addPS4ProfileRTArgs(const ToolChain &TC, const ArgList &Args,
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -147,7 +147,7 @@
   if (C.getLangOpts().ObjC1)
     ObjCData = new ObjCEntrypoints();
 
-  if (!CodeGenOpts.InstrProfileInput.empty()) {
+  if (!CodeGenOpts.ProfileIRInstr && !CodeGenOpts.InstrProfileInput.empty()) {
     auto ReaderOrErr =
         llvm::IndexedInstrProfReader::create(CodeGenOpts.InstrProfileInput);
     if (std::error_code EC = ReaderOrErr.getError()) {
Index: lib/CodeGen/BackendUtil.cpp
===================================================================
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -436,6 +436,18 @@
     MPM->add(createInstrProfilingPass(Options));
   }
 
+  if (CodeGenOpts.ProfileIRInstr) {
+    // Should not have ProfileInstrGenerate set -- it is for clang
+    // instrumentation only.
+    assert (!CodeGenOpts.ProfileInstrGenerate);
+    if (!CodeGenOpts.InstrProfileInput.empty())
+      PMBuilder.PGOInstrUse = CodeGenOpts.InstrProfileInput;
+    else if (!CodeGenOpts.InstrProfileOutput.empty())
+      PMBuilder.PGOInstrGen = CodeGenOpts.InstrProfileOutput;
+    else
+      PMBuilder.PGOInstrGen = "default.profraw";
+  }
+
   if (!CodeGenOpts.SampleProfileFile.empty())
     MPM->add(createSampleProfileLoaderPass(CodeGenOpts.SampleProfileFile));
 
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -105,6 +105,7 @@
 
 CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate
                                         ///< execution counts to use with PGO.
+CODEGENOPT(ProfileIRInstr, 1, 0) ///< IR level PGO instrumentation and use.
 CODEGENOPT(CoverageMapping , 1, 0) ///< Generate coverage mapping regions to
                                    ///< enable code coverage analysis.
 CODEGENOPT(DumpCoverageMapping , 1, 0) ///< Dump the generated coverage mapping
Index: include/clang/Driver/CC1Options.td
===================================================================
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -268,6 +268,9 @@
 def fsanitize_coverage_8bit_counters
     : Flag<["-"], "fsanitize-coverage-8bit-counters">,
       HelpText<"Enable frequency counters in sanitizer coverage">;
+def fprofile_ir_instr : Flag<["-"], "fprofile-ir-instr">,
+    HelpText<"Use IR level instrumentation rather the FE implementation">;
+
 
 //===----------------------------------------------------------------------===//
 // Dependency Output Options

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits