Disable frame pointer elimination when using -pg

[sisnkemp]

Summary

When using -pg in combination with optimizations (e.g. -O3), compiled programs crash in the profiling routine mcount().
This is because mcount (on x86_64 at least) relies on the frame pointer to be valid. See test case to reproduce.

This diff makes sure that the driver does not pass -fomit-frame-pointer or -momit-leaf-frame-pointer to the frontend when -pg is used. Currently, clang gives an error if -fomit-frame-pointer is used in combination with -pg, but -momit-leaf-frame-pointer was forgotten.
Also, disable frame pointer elimination in the frontend when -pg is set.

The behavior is consistent with gcc now: clang produces an error if -fomit-frame-pointer is used with -pg, and silently disables -momit-leaf-frame-pointer when -pg is used.

[sisnkemp]

Ping. Can somebody please comment on this?

[spatel]

Adding some PGO-active reviewers.

[davidxl]

Can you also add a test case with option -momit-leaf-frame-pointer?

[sisnkemp]

Added a test with -momit-leaf-frame-pointer as requested by davidxl.

[davidxl]

Looks good to me.

[RKSimon]

Committed at rL253886

lib/Driver/Tools.cpp

static bool shouldUseFramePointerForTarget(const ArgList &Args,
   return true;
 }
 
 static bool shouldUseFramePointer(const ArgList &Args,
                                   const llvm::Triple &Triple) {
   if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
                                options::OPT_fomit_frame_pointer))
     return A->getOption().matches(options::OPT_fno_omit_frame_pointer);
+  if (Args.hasArg(options::OPT_pg))
+    return true;
 
   return shouldUseFramePointerForTarget(Args, Triple);
 }
 
 static bool shouldUseLeafFramePointer(const ArgList &Args,
                                       const llvm::Triple &Triple) {
   if (Arg *A = Args.getLastArg(options::OPT_mno_omit_leaf_frame_pointer,
                                options::OPT_momit_leaf_frame_pointer))
     return A->getOption().matches(options::OPT_mno_omit_leaf_frame_pointer);
+  if (Args.hasArg(options::OPT_pg))
+    return true;
 
   if (Triple.isPS4CPU())
     return false;
 
   return shouldUseFramePointerForTarget(Args, Triple);
 }
 
 /// Add a CC1 option to specify the debug compilation directory.

lib/Frontend/CompilerInvocation.cpp

Opts.CoverageMapping =
       Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
   Opts.CXAAtExit = !Args.hasArg(OPT_fno_use_cxa_atexit);
   Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases);
   Opts.CodeModel = getCodeModel(Args, Diags);
   Opts.DebugPass = Args.getLastArgValue(OPT_mdebug_pass);
-  Opts.DisableFPElim = Args.hasArg(OPT_mdisable_fp_elim);
+  Opts.DisableFPElim =
+      (Args.hasArg(OPT_mdisable_fp_elim) || Args.hasArg(OPT_pg));
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
   Opts.FloatABI = Args.getLastArgValue(OPT_mfloat_abi);
   if (Arg *A = Args.getLastArg(OPT_meabi)) {
     StringRef Value = A->getValue();
     llvm::EABI EABIVersion = llvm::StringSwitch<llvm::EABI>(Value)
                                  .Case("default", llvm::EABI::Default)
                                  .Case("4", llvm::EABI::EABI4)

test/CodeGen/x86_64-profiling-keep-fp.c

+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -pg -S -o - %s | \
+// RUN:   FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -momit-leaf-frame-pointer -pg -S -o - %s | \
+// RUN:   FileCheck %s
+
+// Test that the frame pointer is kept when compiling with
+// profiling.
+
+//CHECK: pushq %rbp
+int main(void)
+{
+  return 0;
+}