[Driver][PGO] Driver support for de-Optimizing cold functions (PATCH 2/2)

[myhsu]

Summary

Following up https://reviews.llvm.org/D87337 . This patch add the driver support with the flag -fprofile-omit-cold-opt=<percentage> and a shorthand -fprofile-omit-cold-opt that is equal to "-fprofile-omit-cold-opt=0"

authors: myhsu, probinson, and edd

[myhsu]

Fix a bug in driver that accidentally made this feature turn on by default

[myhsu]

Unified driver options from "-fprofile-deopt-cold-*" into "-fprofile-omit-cold-opt" and "-fprofile-omit-cold-opt=<percentage>". Where the former one is just a shorthand of "-fprofile-omit-cold-opt=0"

[rengolin]

clang/docs/ClangCommandLineReference.rst

2021

Following discussions in the mailing list, I'm wondering if we want to join the optsize and optnone implementations into one flag.

[myhsu]

just realized I haven't updated this patch for years. I'll come up with an up-to-date version when I have time.

clang/docs/ClangCommandLineReference.rst

 Enable sample-based profile guided optimizations
 
 .. program:: clang1
 .. option:: -fprofile-use=<pathname>
 .. program:: clang
 
 Use instrumentation data for profile-guided optimization. If pathname is a directory, it reads from <pathname>/default.profdata. Otherwise, it reads from file <pathname>.
 
+.. option:: -fprofile-omit-cold-opt, -fno-profile-omit-cold-opt
+
+Adds ``optnone`` attributes to functions whose instrumentation-based PGO profiling counts are equal to zero (i.e. "cold").
+
+.. program:: clang1
+.. option:: -fprofile-omit-cold-opt=<percentage>
+.. program:: clang
+
+This allows you to choose the lower <percentage>% of functions, sorted by their profiling counts, as cold functions that will not be optimized.
+
 .. option:: -freciprocal-math, -fno-reciprocal-math
 
 Allow division operations to be reassociated
 
 .. option:: -freg-struct-return
 
 Override the default ABI to return small structs in registers
 

clang/include/clang/Driver/Options.td

 def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
     Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
     HelpText<"The compilation directory to embed in the debug info.">;
 def fdebug_compilation_dir_EQ : Joined<["-"], "fdebug-compilation-dir=">,
     Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
     Alias<fdebug_compilation_dir>;
 defm debug_info_for_profiling : OptInFFlag<"debug-info-for-profiling",
   "Emit extra debug info to make sample profile more accurate">;
+
+// Options to omit optimizations on cold code.
+def fprofile_omit_cold_opt : Flag<["-"], "fprofile-omit-cold-opt">,
+    Group<f_Group>, Flags<[CoreOption]>,
+    HelpText<"Do not optimize functions that profiling indicates are cold">;
+def fno_profile_omit_cold_opt : Flag<["-"], "fno-profile-omit-cold-opt">,
+    Group<f_Group>, Flags<[CoreOption]>;
+def fprofile_omit_cold_opt_EQ : Joined<["-"], "fprofile-omit-cold-opt=">,
+    Group<f_Group>, Flags<[CoreOption]>, MetaVarName<"<percentage>">,
+    HelpText<"Do not optimize functions whose profiling count is lower than this percentage">;
+
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
     Group<f_Group>, Flags<[CoreOption]>,
     HelpText<"Generate instrumented code to collect execution counts into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_generate_EQ : Joined<["-"], "fprofile-instr-generate=">,
     Group<f_Group>, Flags<[CoreOption]>, MetaVarName<"<file>">,
     HelpText<"Generate instrumented code to collect execution counts into <file> (overridden by LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_use : Flag<["-"], "fprofile-instr-use">, Group<f_Group>,
     Flags<[CoreOption]>;

clang/lib/Driver/ToolChains/Clang.cpp

else if ((ProfileUseArg->getOption().matches(
                   options::OPT_fprofile_instr_use))) {
       SmallString<128> Path(
           ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
       if (Path.empty() || llvm::sys::fs::is_directory(Path))
         llvm::sys::path::append(Path, "default.profdata");
       CmdArgs.push_back(
           Args.MakeArgString(Twine("-fprofile-instrument-use-path=") + Path));
     }
+    // Tell profile passes to avoid optimizing cold functions.
+    if (Args.hasFlag(options::OPT_fprofile_omit_cold_opt,
+                     options::OPT_fprofile_omit_cold_opt_EQ,
+                     options::OPT_fno_profile_omit_cold_opt, false)) {
+      CmdArgs.push_back("-mllvm");
+      CmdArgs.push_back("-profile-omit-cold-func-opt=true");
+
+      if (Arg *A = Args.getLastArg(options::OPT_fprofile_omit_cold_opt_EQ)) {
+        CmdArgs.push_back("-mllvm");
+        CmdArgs.push_back(Args.MakeArgString(
+            Twine("-profile-omit-cold-func-opt-percent=") + A->getValue()));
+      }
+    }
   }
 
   bool EmitCovNotes = Args.hasFlag(options::OPT_ftest_coverage,
                                    options::OPT_fno_test_coverage, false) ||
                       Args.hasArg(options::OPT_coverage);
   bool EmitCovData = TC.needsGCovInstrumentation(Args);
   if (EmitCovNotes)
     CmdArgs.push_back("-femit-coverage-notes");

clang/lib/Driver/ToolChains/CommonArgs.cpp

if (CSPGOGenerateArg->getOption().matches(
           Args.MakeArgString("-plugin-opt=cs-profile-path=default_%m.profraw"));
   } else if (ProfileUseArg) {
     SmallString<128> Path(
         ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
     if (Path.empty() || llvm::sys::fs::is_directory(Path))
       llvm::sys::path::append(Path, "default.profdata");
     CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
                                          Path));
+    // Propagate -profile-omit-cold-func-opt options
+    if (Args.hasFlag(options::OPT_fprofile_omit_cold_opt,
+                     options::OPT_fprofile_omit_cold_opt_EQ,
+                     options::OPT_fno_profile_omit_cold_opt, false)) {
+      CmdArgs.push_back("-plugin-opt=-profile-omit-cold-func-opt=true");
+      Arg *A = Args.getLastArg(options::OPT_fprofile_omit_cold_opt_EQ);
+      if (A) {
+        CmdArgs.push_back(Args.MakeArgString(
+            Twine("-plugin-opt=-profile-omit-cold-func-opt-percent=") +
+            A->getValue()));
+      }
+    }
   }
 
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
                    options::OPT_fno_experimental_new_pass_manager,
                    /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER)) {
     CmdArgs.push_back("-plugin-opt=new-pass-manager");
   }

clang/test/Driver/gold-lto-omit-cold-opt.c

+// RUN: touch %t.o
+//
+// RUN: %clang -### -fprofile-instr-use -fprofile-omit-cold-opt %t.o -flto 2>&1 \
+// RUN:     | FileCheck --check-prefix=CHECK-BASE %s
+
+// RUN: %clang -### -fprofile-instr-use -fprofile-omit-cold-opt=87 \
+// RUN:     %t.o -flto 2>&1 \
+// RUN:     | FileCheck --check-prefixes=CHECK-BASE,CHECK-PERCENT %s
+
+// CHECK-BASE: "-plugin-opt=-profile-omit-cold-func-opt=true"
+// CHECK-PERCENT: "-plugin-opt=-profile-omit-cold-func-opt-percent=87"

clang/test/Driver/omit-cold-opt.c

+// RUN: %clang -### -fprofile-instr-use -fprofile-omit-cold-opt -c %s 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-BASE %s
+
+// RUN: %clang -### -fprofile-instr-use \
+// RUN:             -fprofile-omit-cold-opt=87 \
+// RUN:             -c %s 2>&1 \
+// RUN: | FileCheck --check-prefixes=CHECK-BASE,CHECK-PERCENT %s
+
+// RUN: %clang -### -fprofile-instr-use \
+// RUN:             -fprofile-omit-cold-opt -fno-profile-omit-cold-opt \
+// RUN:             -c %s 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-NEG %s
+
+// Shouldn't do anything if it's not using PGO profile
+// RUN: %clang -### -fprofile-omit-cold-opt -c %s 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-NEG %s
+
+// CHECK-BASE: "-mllvm" "-profile-omit-cold-func-opt=true"
+// CHECK-PERCENT: "-mllvm" "-profile-omit-cold-func-opt-percent=87"
+// CHECK-NEG-NOT: "-profile-omit-cold-func-opt=true"