Add GCC-compatible flags -fprofile-generate and -fprofile-use

Justin Bogner mail at justinbogner.com
Mon Jun 29 14:09:11 PDT 2015


Diego Novillo <dnovillo at google.com> writes:
>     This patch adds support for specifying where the profile is emitted in a
>     way similar to GCC. These flags are used to specify directories instead
>     of filenames.
>
>     I've also added a couple of extensions: LLVM_PROFILE_FILE can still be
>     used to override the directory and file name to use and -fprofile-use
>     accepts both directories and filenames.
>
>     To simplify the combinations we need to handle, I've added code to
>     prevent users from using -fprofile-generate and -fprofile-instr-generate
>     simultaneously. Likewise for -fprofile-use and -fprofile-instr-use.

A few comments below (in addition to the update for the default filename
from a directory you already sent out).

>
> Diego.
>
> commit b57da8d24fe41456b1e71695c1aa10e6be1c1fa0
> Author: Diego Novillo <dnovillo at google.com>
> Date:   Thu Jun 25 18:46:32 2015 -0400
>
>     Add GCC-compatible flags -fprofile-generate and -fprofile-use.
>     
>     This patch adds support for specifying where the profile is emitted in a
>     way similar to GCC. These flags are used to specify directories instead
>     of filenames.
>     
>     I've also added a couple of extensions: LLVM_PROFILE_FILE can still be
>     used to override the directory and file name to use and -fprofile-use
>     accepts both directories and filenames.
>     
>     To simplify the combinations we need to handle, I've added code to
>     prevent users from using -fprofile-generate and -fprofile-instr-generate
>     simultaneously. Likewise for -fprofile-use and -fprofile-instr-use.
>
> diff --git a/docs/UsersManual.rst b/docs/UsersManual.rst
> index cd1b2b3..8e0ceb8 100644
> --- a/docs/UsersManual.rst
> +++ b/docs/UsersManual.rst
> @@ -1488,6 +1488,45 @@ instrumentation:
>     profile. As you make changes to your code, clang may no longer be able to
>     use the profile data. It will warn you when this happens.
>  
> +Profile generation and use can also be controlled by the GCC-compatible flags
> +``-fprofile-generate`` and ``-fprofile-use``. Although these flags are
> +semantically equivalent to their GCC counterparts, they *do not* handle
> +GCC-compatible profiles. They are only meant to implement GCC's semantics
> +with respect to profile creation and use.
> +
> +.. option:: -fprofile-generate[=<dirname>]
> +
> +  Without any other arguments, ``-fprofile-generate`` behaves identically to
> +  ``-fprofile-instr-generate``. When given a directory name, it generates the
> +  profile file ``default.profraw`` in the directory named ``dirname``. If
> +  ``dirname`` does not exist, it will be created. The environment variable

> +  ``LLVM_PROFILE_FILE`` can be used to alter the directory and filename for the
> +  profile file. For example,

I'd reword this like

  ``LLVM_PROFILE_FILE`` can be used to override the directory and filename
  for the profile file at runtime

> +
> +  .. code-block:: console
> +
> +    $ clang++ -O2 -fprofile-generate=yyy/zzz code.cc -o code
> +
> +  When ``code`` is executed, the profile will be written to the file
> +  ``yyy/zzz/default.profraw``. This can be altered at runtime via the
> +  ``LLVM_PROFILE_FILE`` environment variable:
> +
> +  .. code-block:: console
> +
> +    $ LLVM_PROFILE_FILE=/tmp/myprofile/code.profraw ./code
> +
> +  The above invocation will produce the profile file
> +  ``/tmp/myprofile/code.profraw`` instead of ``yyy/zzz/default.profraw``.
> +  Notice that ``LLVM_PROFILE_FILE`` overrides the directory *and* the file
> +  name for the profile file.
> +
> +.. option:: -fprofile-use[=<pathname>]
> +

> +  Without any other arguments, ``-fprofile-use`` behaves identically to
> +  ``-fprofile-instr-use``. Otherwise, if ``pathname`` is the full path to a
> +  profile file, it reads from that file. If ``pathname`` is a directory name,
> +  it reads from ``pathname/pgo-data``.

Apparently we already default to pgo-data, and I actually added that
code, but I'd rather default to "default.profdata" to mirror
"default.profraw". It seems the behaviour with the pgo-data name is
completely undocumented and untested, so it should be fine to switch.

> +
>  
>  Controlling Size of Debug Information
>  -------------------------------------
> diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td
> index 4eb5647..ff6731d 100644
> --- a/include/clang/Driver/Options.td
> +++ b/include/clang/Driver/Options.td
> @@ -429,6 +429,16 @@ def fprofile_instr_use_EQ : Joined<["-"], "fprofile-instr-use=">,
>  def fcoverage_mapping : Flag<["-"], "fcoverage-mapping">,
>      Group<f_Group>, Flags<[CC1Option]>,
>      HelpText<"Generate coverage mapping to enable code coverage analysis">;
> +def fprofile_generate : Flag<["-"], "fprofile-generate">,
> +    Alias<fprofile_instr_generate>;
> +def fprofile_generate_EQ : Joined<["-"], "fprofile-generate=">,
> +    Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<directory>">,
> +    HelpText<"Generate instrumented code to collect execution counts into <directory>/default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
> +def fprofile_use : Flag<["-"], "fprofile-use">, Group<f_Group>,
> +    Alias<fprofile_instr_use>;
> +def fprofile_use_EQ : Joined<["-"], "fprofile-use=">,
> +    Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<pathname>">,
> +    HelpText<"Use instrumentation data for profile-guided optimization. If pathname is a directory, it reads from <pathname>/default.profile. Otherwise, it reads from file <pathname>.">;
>  
>  def fblocks : Flag<["-"], "fblocks">, Group<f_Group>, Flags<[CC1Option]>,
>    HelpText<"Enable the 'blocks' language feature">;
> @@ -904,7 +914,6 @@ def fpie : Flag<["-"], "fpie">, Group<f_Group>;
>  def fno_pie : Flag<["-"], "fno-pie">, Group<f_Group>;
>  def fprofile_arcs : Flag<["-"], "fprofile-arcs">, Group<f_Group>;
>  def fno_profile_arcs : Flag<["-"], "fno-profile-arcs">, Group<f_Group>;
> -def fprofile_generate : Flag<["-"], "fprofile-generate">, Group<f_Group>;
>  def framework : Separate<["-"], "framework">, Flags<[LinkerInput]>;
>  def frandom_seed_EQ : Joined<["-"], "frandom-seed=">, Group<clang_ignored_f_Group>;
>  def freg_struct_return : Flag<["-"], "freg-struct-return">, Group<f_Group>, Flags<[CC1Option]>,
> @@ -1787,8 +1796,6 @@ defm : BooleanFFlag<"keep-inline-functions">, Group<clang_ignored_gcc_optimizati
>  
>  def fprofile_dir : Joined<["-"], "fprofile-dir=">, Group<clang_ignored_gcc_optimization_f_Group>;
>  
> -defm profile_use : BooleanFFlag<"profile-use">, Group<clang_ignored_gcc_optimization_f_Group>;
> -def fprofile_use_EQ : Joined<["-"], "fprofile-use=">, Group<clang_ignored_gcc_optimization_f_Group>;
>  def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, Group<f_Group>;
>  
>  defm align_functions : BooleanFFlag<"align-functions">, Group<clang_ignored_gcc_optimization_f_Group>;
> diff --git a/include/clang/Frontend/CodeGenOptions.h b/include/clang/Frontend/CodeGenOptions.h
> index 66597bd..8c199d2 100644
> --- a/include/clang/Frontend/CodeGenOptions.h
> +++ b/include/clang/Frontend/CodeGenOptions.h
> @@ -154,7 +154,12 @@ public:
>    /// A list of dependent libraries.
>    std::vector<std::string> DependentLibraries;
>  
> +  /// Name of the directory where to save the profile file specified in
> +  /// -fprofile-generate.
> +  std::string InstrProfileDir;
> +
>    /// Name of the profile file to use as output for -fprofile-instr-generate
> +  /// and -fprofile-generate.
>    std::string InstrProfileOutput;
>  
>    /// Name of the profile file to use with -fprofile-sample-use.
> diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp
> index 801b49f..663a38a 100644
> --- a/lib/CodeGen/BackendUtil.cpp
> +++ b/lib/CodeGen/BackendUtil.cpp
> @@ -415,6 +415,7 @@ void EmitAssemblyHelper::CreatePasses() {
>    if (CodeGenOpts.ProfileInstrGenerate) {
>      InstrProfOptions Options;
>      Options.NoRedZone = CodeGenOpts.DisableRedZone;
> +    Options.InstrProfileDir = CodeGenOpts.InstrProfileDir;
>      Options.InstrProfileOutput = CodeGenOpts.InstrProfileOutput;
>      MPM->add(createInstrProfilingPass(Options));
>    }
> diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
> index 3371861..74e43bc 100644
> --- a/lib/Driver/ToolChains.cpp
> +++ b/lib/Driver/ToolChains.cpp
> @@ -309,6 +309,7 @@ void Darwin::addProfileRTLibs(const ArgList &Args,
>    if (!(Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
>                       false) ||
>          Args.hasArg(options::OPT_fprofile_generate) ||
> +        Args.hasArg(options::OPT_fprofile_generate_EQ) ||
>          Args.hasArg(options::OPT_fprofile_instr_generate) ||
>          Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
>          Args.hasArg(options::OPT_fcreate_profile) ||
> diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
> index c6b3bb8..44bf2cf 100644
> --- a/lib/Driver/Tools.cpp
> +++ b/lib/Driver/Tools.cpp
> @@ -2292,6 +2292,7 @@ static void addProfileRT(const ToolChain &TC, const ArgList &Args,
>    if (!(Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
>                       false) ||
>          Args.hasArg(options::OPT_fprofile_generate) ||
> +        Args.hasArg(options::OPT_fprofile_generate_EQ) ||
>          Args.hasArg(options::OPT_fprofile_instr_generate) ||
>          Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
>          Args.hasArg(options::OPT_fcreate_profile) ||
> @@ -3536,20 +3537,45 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
>    Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
>  
>    if ((Args.hasArg(options::OPT_fprofile_instr_generate) ||
> -       Args.hasArg(options::OPT_fprofile_instr_generate_EQ)) &&
> +       Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
> +       Args.hasArg(options::OPT_fprofile_generate) ||
> +       Args.hasArg(options::OPT_fprofile_generate_EQ)) &&
>        (Args.hasArg(options::OPT_fprofile_instr_use) ||
> -       Args.hasArg(options::OPT_fprofile_instr_use_EQ)))
> +       Args.hasArg(options::OPT_fprofile_instr_use_EQ) ||
> +       Args.hasArg(options::OPT_fprofile_use) ||
> +       Args.hasArg(options::OPT_fprofile_use_EQ)))
>      D.Diag(diag::err_drv_argument_not_allowed_with)
>        << "-fprofile-instr-generate" << "-fprofile-instr-use";
>  
> +  if ((Args.hasArg(options::OPT_fprofile_instr_generate) ||
> +       Args.hasArg(options::OPT_fprofile_instr_generate_EQ)) &&
> +      (Args.hasArg(options::OPT_fprofile_generate) ||
> +       Args.hasArg(options::OPT_fprofile_generate_EQ)))
> +    D.Diag(diag::err_drv_argument_not_allowed_with)
> +      << "-fprofile-instr-generate" << "-fprofile-generate";
> +
> +  if ((Args.hasArg(options::OPT_fprofile_instr_use) ||
> +       Args.hasArg(options::OPT_fprofile_instr_use_EQ)) &&
> +      (Args.hasArg(options::OPT_fprofile_use) ||
> +       Args.hasArg(options::OPT_fprofile_use_EQ)))
> +    D.Diag(diag::err_drv_argument_not_allowed_with)
> +      << "-fprofile-instr-use" << "-fprofile-use";
> +
>    if (Arg *A = Args.getLastArg(options::OPT_fprofile_instr_generate_EQ))
>      A->render(Args, CmdArgs);
> -  else
> +  else if (Arg *A = Args.getLastArg(options::OPT_fprofile_generate_EQ))
> +    A->render(Args, CmdArgs);
> +  else if (Args.hasArg(options::OPT_fprofile_instr_generate))
>      Args.AddAllArgs(CmdArgs, options::OPT_fprofile_instr_generate);
> +  else if (Args.hasArg(options::OPT_fprofile_generate))
> +    Args.AddAllArgs(CmdArgs, options::OPT_fprofile_generate);

Hm, these (even before your change) behave kind of oddly when multiple
are specified. If someone specifies multiple forms it would probably
make more sense to accept the last one, rather than have certain forms
"win".

We should use the getLastArg form that takes multiple arguments here and
consolidate these down to a single canonical form for -cc1.

>  
>    if (Arg *A = Args.getLastArg(options::OPT_fprofile_instr_use_EQ))
>      A->render(Args, CmdArgs);
> -  else if (Args.hasArg(options::OPT_fprofile_instr_use))
> +  else if (Arg *A = Args.getLastArg(options::OPT_fprofile_use_EQ))
> +    A->render(Args, CmdArgs);
> +  else if (Args.hasArg(options::OPT_fprofile_instr_use) ||
> +           Args.hasArg(options::OPT_fprofile_use))
>      CmdArgs.push_back("-fprofile-instr-use=pgo-data");
>  
>    if (Args.hasArg(options::OPT_ftest_coverage) ||
> @@ -3562,7 +3588,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
>  
>    if (Args.hasArg(options::OPT_fcoverage_mapping) &&
>        !(Args.hasArg(options::OPT_fprofile_instr_generate) ||
> -        Args.hasArg(options::OPT_fprofile_instr_generate_EQ)))
> +        Args.hasArg(options::OPT_fprofile_generate) ||
> +        Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
> +        Args.hasArg(options::OPT_fprofile_generate_EQ)))
>      D.Diag(diag::err_drv_argument_only_allowed_with)
>        << "-fcoverage-mapping" << "-fprofile-instr-generate";
>  
> diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
> index dd664ca..d212be8 100644
> --- a/lib/Frontend/CompilerInvocation.cpp
> +++ b/lib/Frontend/CompilerInvocation.cpp
> @@ -448,9 +448,20 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
>    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);
> +                              Args.hasArg(OPT_fprofile_instr_generate_EQ) ||
> +                              Args.hasArg(OPT_fprofile_generate) ||
> +                              Args.hasArg(OPT_fprofile_generate_EQ);
>    Opts.InstrProfileOutput = Args.getLastArgValue(OPT_fprofile_instr_generate_EQ);
> +  Opts.InstrProfileDir = Args.getLastArgValue(OPT_fprofile_generate_EQ);
>    Opts.InstrProfileInput = Args.getLastArgValue(OPT_fprofile_instr_use_EQ);
> +  if (Opts.InstrProfileInput.empty()) {
> +    SmallString<128> Filename(Args.getLastArgValue(OPT_fprofile_use_EQ));
> +    if (!Filename.empty()) {
> +      if (llvm::sys::fs::is_directory(Filename))
> +        llvm::sys::path::append(Filename, "pgo-data");
> +      Opts.InstrProfileInput = Filename.str();
> +    }
> +  }
>    Opts.CoverageMapping = Args.hasArg(OPT_fcoverage_mapping);
>    Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
>    Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose);
> diff --git a/test/Driver/clang_f_opts.c b/test/Driver/clang_f_opts.c
> index 68890a7..a911ea6 100644
> --- a/test/Driver/clang_f_opts.c
> +++ b/test/Driver/clang_f_opts.c
> @@ -162,7 +162,7 @@
>  // RUN:     -fprefetch-loop-arrays -fno-prefetch-loop-arrays                  \
>  // RUN:     -fprofile-correction -fno-profile-correction                      \
>  // RUN:     -fprofile-dir=bar                                                 \
> -// RUN:     -fprofile-use -fprofile-use=zed -fno-profile-use                  \
> +// RUN:     -fprofile-use -fprofile-use=zed                                   \
>  // RUN:     -fprofile-values -fno-profile-values                              \
>  // RUN:     -frounding-math -fno-rounding-math                                \
>  // RUN:     -fsee -fno-see                                                    \
> @@ -242,8 +242,6 @@
>  // RUN: -fno-keep-inline-functions                                            \
>  // RUN: -freorder-blocks                                                      \
>  // RUN: -fprofile-dir=/rand/dir                                               \
> -// RUN: -fprofile-use                                                         \
> -// RUN: -fprofile-use=/rand/dir                                               \
>  // RUN: -falign-functions                                                     \
>  // RUN: -falign-functions=1                                                   \
>  // RUN: -ffloat-store                                                         \
> @@ -312,8 +310,6 @@
>  // CHECK-WARNING-DAG: optimization flag '-fno-keep-inline-functions' is not supported
>  // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
>  // CHECK-WARNING-DAG: optimization flag '-fprofile-dir=/rand/dir' is not supported
> -// CHECK-WARNING-DAG: optimization flag '-fprofile-use' is not supported
> -// CHECK-WARNING-DAG: optimization flag '-fprofile-use=/rand/dir' is not supported
>  // CHECK-WARNING-DAG: optimization flag '-falign-functions' is not supported
>  // CHECK-WARNING-DAG: optimization flag '-falign-functions=1' is not supported
>  // CHECK-WARNING-DAG: optimization flag '-ffloat-store' is not supported
> diff --git a/test/Profile/Inputs/gcc-flag-compatibility.proftext b/test/Profile/Inputs/gcc-flag-compatibility.proftext
> new file mode 100644
> index 0000000..99d41bb
> --- /dev/null
> +++ b/test/Profile/Inputs/gcc-flag-compatibility.proftext
> @@ -0,0 +1,5 @@
> +main
> +4
> +2
> +1
> +100
> diff --git a/test/Profile/gcc-flag-compatibility.c b/test/Profile/gcc-flag-compatibility.c
> new file mode 100644
> index 0000000..af64a96
> --- /dev/null
> +++ b/test/Profile/gcc-flag-compatibility.c
> @@ -0,0 +1,48 @@
> +// Tests for -fprofile-generate and -fprofile-use flag compatibility. These two
> +// flags behave similarly to their GCC counterparts:
> +//
> +// -fprofile-generate         Generates the profile file ./default.profraw
> +// -fprofile-generate=<dir>   Generates the profile file <dir>/default.profraw
> +// -fprofile-use              Uses the profile file ./pgo-data
> +// -fprofile-use=<dir>        Uses the profile file <dir>/pgo-data
> +// -fprofile-use=<dir>/file   Uses the profile file <dir>/file
> +
> +// Check that -fprofile-generate uses the runtime default profile file.
> +// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate | FileCheck -check-prefix=PROFILE-GEN %s
> +// PROFILE-GEN: @__llvm_profile_runtime = external global i32
> +// PROFILE-GEN-NOT: call void @__llvm_profile_override_default_filename
> +// PROFILE-GEN-NOT: declare void @__llvm_profile_override_default_filename(i8*)
> +
> +// Check that -fprofile-generate=/path/to generates /path/to/default.profraw
> +// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to | FileCheck -check-prefix=PROFILE-GEN-EQ %s
> +// PROFILE-GEN-EQ: private constant [25 x i8] c"/path/to/default.profraw\00"
> +// PROFILE-GEN-EQ: call void @__llvm_profile_override_default_filename(i8* getelementptr inbounds ([25 x i8], [25 x i8]* @0, i32 0, i32 0))
> +// PROFILE-GEN-EQ: declare void @__llvm_profile_override_default_filename(i8*)
> +
> +// Check that -fprofile-use reads pgo-data
> +// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o pgo-data
> +// RUN: %clang %s -o - -mllvm -disable-llvm-optzns -emit-llvm -S -fprofile-use | FileCheck -check-prefix=PROFILE-USE-1 %s
> +// PROFILE-USE-1: = !{!"branch_weights", i32 101, i32 2}
> +
> +// Check that -fprofile-use=some/path reads some/path/pgo-data
> +// RUN: rm -rf %t.dir
> +// RUN: mkdir -p %t.dir/some/path
> +// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o %t.dir/some/path/pgo-data
> +// RUN: %clang %s -o - -mllvm -disable-llvm-optzns -emit-llvm -S -fprofile-use=%t.dir/some/path | FileCheck -check-prefix=PROFILE-USE-2 %s
> +// PROFILE-USE-2: = !{!"branch_weights", i32 101, i32 2}
> +
> +// Check that -fprofile-use=some/path/file.prof reads some/path/file.prof
> +// RUN: rm -rf %t.dir
> +// RUN: mkdir -p %t.dir/some/path
> +// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o %t.dir/some/path/file.prof
> +// RUN: %clang %s -o - -mllvm -disable-llvm-optzns -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof | FileCheck -check-prefix=PROFILE-USE-3 %s
> +// PROFILE-USE-3: = !{!"branch_weights", i32 101, i32 2}
> +
> +int X = 0;
> +
> +int main() {
> +  int i;
> +  for (i = 0; i < 100; i++)
> +    X += i;
> +  return 0;
> +}



More information about the cfe-commits mailing list