[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366645: [Clang] Replace cc1 options 
-mdisable-fp-elim and -momit-leaf-frame-pointer 
(authored by yuanfang, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56353?vs=210014=210975#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353

Files:
  cfe/trunk/include/clang/Basic/CodeGenOptions.def
  cfe/trunk/include/clang/Basic/CodeGenOptions.h
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/msp430-fp-elim.c
  cfe/trunk/test/CodeGen/x86_64-profiling-keep-fp.c
  cfe/trunk/test/CodeGen/xcore-abi.c
  cfe/trunk/test/CodeGen/xcore-abi.cpp
  cfe/trunk/test/CodeGenCXX/global-init.cpp
  cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm
  cfe/trunk/test/Driver/cl-options.c
  cfe/trunk/test/Driver/clang-translation.c
  cfe/trunk/test/Driver/frame-pointer-elim.c
  cfe/trunk/test/Driver/frame-pointer.c
  cfe/trunk/test/Driver/woa-fp.c
  cfe/trunk/test/Driver/xcore-opts.c

Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -1713,16 +1713,19 @@
 if (!CodeGenOpts.TrapFuncName.empty())
   FuncAttrs.addAttribute("trap-func-name", CodeGenOpts.TrapFuncName);
   } else {
-// Attributes that should go on the function, but not the call site.
-if (!CodeGenOpts.DisableFPElim) {
-  FuncAttrs.addAttribute("no-frame-pointer-elim", "false");
-} else if (CodeGenOpts.OmitLeafFramePointer) {
-  FuncAttrs.addAttribute("no-frame-pointer-elim", "false");
-  FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
-} else {
-  FuncAttrs.addAttribute("no-frame-pointer-elim", "true");
-  FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
+StringRef FpKind;
+switch (CodeGenOpts.getFramePointer()) {
+case CodeGenOptions::FramePointerKind::None:
+  FpKind = "none";
+  break;
+case CodeGenOptions::FramePointerKind::NonLeaf:
+  FpKind = "non-leaf";
+  break;
+case CodeGenOptions::FramePointerKind::All:
+  FpKind = "all";
+  break;
 }
+FuncAttrs.addAttribute("frame-pointer", FpKind);
 
 FuncAttrs.addAttribute("less-precise-fpmad",
llvm::toStringRef(CodeGenOpts.LessPreciseFPMAD));
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -22,6 +22,7 @@
 #include "InputInfo.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
@@ -501,8 +502,6 @@
   return codegenoptions::LimitedDebugInfo;
 }
 
-enum class FramePointerKind { None, NonLeaf, All };
-
 static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple ) {
   switch (Triple.getArch()){
   default:
@@ -579,8 +578,8 @@
   return true;
 }
 
-static FramePointerKind getFramePointerKind(const ArgList ,
-const llvm::Triple ) {
+static CodeGenOptions::FramePointerKind
+getFramePointerKind(const ArgList , const llvm::Triple ) {
   Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
options::OPT_fno_omit_frame_pointer);
   bool OmitFP = A && A->getOption().matches(options::OPT_fomit_frame_pointer);
@@ -591,10 +590,10 @@
 if (Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
  options::OPT_mno_omit_leaf_frame_pointer,
  Triple.isPS4CPU()))
-  return FramePointerKind::NonLeaf;
-return FramePointerKind::All;
+  return CodeGenOptions::FramePointerKind::NonLeaf;
+return CodeGenOptions::FramePointerKind::All;
   }
-  return FramePointerKind::None;
+  return CodeGenOptions::FramePointerKind::None;
 }
 
 /// Add a CC1 option to specify the debug compilation directory.
@@ -3946,12 +3945,23 @@
   if (Args.hasFlag(options::OPT_mrtd, options::OPT_mno_rtd, false))
 CmdArgs.push_back("-fdefault-calling-conv=stdcall");
 
-  FramePointerKind FPKeepKind = getFramePointerKind(Args, RawTriple);
-  if (FPKeepKind != FramePointerKind::None) {
-CmdArgs.push_back("-mdisable-fp-elim");
-if (FPKeepKind == FramePointerKind::NonLeaf)
-  CmdArgs.push_back("-momit-leaf-frame-pointer");
+  CodeGenOptions::FramePointerKind FPKeepKind =
+  getFramePointerKind(Args, 

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

@chandlerc ping?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 210014.
ychen marked an inline comment as done.
ychen added a comment.

- update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/msp430-fp-elim.c
  clang/test/CodeGen/x86_64-profiling-keep-fp.c
  clang/test/CodeGen/xcore-abi.c
  clang/test/CodeGen/xcore-abi.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenObjCXX/msabi-stret.mm
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-translation.c
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c
  clang/test/Driver/woa-fp.c
  clang/test/Driver/xcore-opts.c

Index: clang/test/Driver/xcore-opts.c
===
--- clang/test/Driver/xcore-opts.c
+++ clang/test/Driver/xcore-opts.c
@@ -3,9 +3,8 @@
 // RUN: %clang -target xcore -x c++ %s -fexceptions -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-EXCEP %s
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
+// CHECK: "-mframe-pointer=none"
 // CHECK: "-nostdsysteminc"
-// CHECK-NOT: "-mdisable-fp-elim"
-// CHECK-NOT: "-momit-leaf-frame-pointer"
 // CHECK: "-fno-signed-char"
 // CHECK: "-fno-use-cxa-atexit"
 // CHECK-NOT: "-fcxx-exceptions"
Index: clang/test/Driver/woa-fp.c
===
--- clang/test/Driver/woa-fp.c
+++ clang/test/Driver/woa-fp.c
@@ -34,7 +34,7 @@
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -O3 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -Os -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 
-// CHECK-DEFAULT: "-mdisable-fp-elim"
-// CHECK-FPO-NOT: "-mdisable-fp-elim"
-// CHECK-NO-FPO: "-mdisable-fp-elim"
+// CHECK-DEFAULT: "-mframe-pointer=all"
+// CHECK-FPO-NOT: "-mframe-pointer=all"
+// CHECK-NO-FPO: "-mframe-pointer=all"
 
Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,15 +57,15 @@
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -Os %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
-// CHECK0-32: -mdisable-fp-elim
-// CHECK1-32-NOT: -mdisable-fp-elim
-// CHECK2-32-NOT: -mdisable-fp-elim
-// CHECK3-32-NOT: -mdisable-fp-elim
-// CHECKs-32-NOT: -mdisable-fp-elim
+// CHECK0-32: -mframe-pointer=all
+// CHECK1-32-NOT: -mframe-pointer=all
+// CHECK2-32-NOT: -mframe-pointer=all
+// CHECK3-32-NOT: -mframe-pointer=all
+// CHECKs-32-NOT: -mframe-pointer=all
 
-// CHECK0-64: -mdisable-fp-elim
-// CHECK1-64-NOT: -mdisable-fp-elim
-// CHECK2-64-NOT: -mdisable-fp-elim
-// CHECK3-64-NOT: -mdisable-fp-elim
-// CHECKs-64-NOT: -mdisable-fp-elim
-// CHECK-MACHO-64: -mdisable-fp-elim
+// CHECK0-64: -mframe-pointer=all
+// CHECK1-64-NOT: -mframe-pointer=all
+// CHECK2-64-NOT: -mframe-pointer=all
+// CHECK3-64-NOT: -mframe-pointer=all
+// CHECKs-64-NOT: -mframe-pointer=all
+// CHECK-MACHO-64: -mframe-pointer=all
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -1,11 +1,6 @@
-// KEEP-ALL: "-mdisable-fp-elim"
-// KEEP-ALL-NOT: "-momit-leaf-frame-pointer"
-
-// KEEP-NON-LEAF: "-mdisable-fp-elim"
-// KEEP-NON-LEAF: "-momit-leaf-frame-pointer"
-
-// KEEP-NONE-NOT: "-mdisable-fp-elim"
-// KEEP-NONE-NOT: "-momit-leaf-frame-pointer"
+// KEEP-ALL:  "-mframe-pointer=all"
+// KEEP-NON-LEAF: "-mframe-pointer=non-leaf"
+// KEEP-NONE: "-mframe-pointer=none"
 
 // On Linux x86, omit frame pointer when optimization is enabled.
 // RUN: %clang -### -target i386-linux -S -fomit-frame-pointer %s 2>&1 | \
@@ -73,20 +68,17 @@
 // RUN: %clang -### -target armv7s-apple-ios -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7S %s
 // WARN-OMIT-7S: warning: optimization flag '-fomit-frame-pointer' is not supported for target 'armv7s'
-// WARN-OMIT-7S: "-mdisable-fp-elim"
-// WARN-OMIT-7S-NOT: "-momit-leaf-frame-pointer"
+// WARN-OMIT-7S: "-mframe-pointer=all"
 
 // RUN: %clang -### -target armv7k-apple-watchos -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7K %s
 // WARN-OMIT-7K: warning: optimization flag '-fomit-frame-pointer' is not supported for target 'armv7k'
-// 

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1728
 }
+assert(!FpKind.empty() && "unknown frame-pointer");
+FuncAttrs.addAttribute("frame-pointer", FpKind);

The `assert` here is not very necessary.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3955
+  const char *FPKeepKindStr = nullptr;
+  switch(FPKeepKind) {
+  case CodeGenOptions::FramePointerKind::None:

Add a space after `switch`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 209973.
ychen marked an inline comment as not done.
ychen edited the summary of this revision.
ychen added a comment.

- Make `FramePointerKind` enum class.
- Replace two if-else-if blocks to switch statements.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/msp430-fp-elim.c
  clang/test/CodeGen/x86_64-profiling-keep-fp.c
  clang/test/CodeGen/xcore-abi.c
  clang/test/CodeGen/xcore-abi.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenObjCXX/msabi-stret.mm
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-translation.c
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c
  clang/test/Driver/woa-fp.c
  clang/test/Driver/xcore-opts.c

Index: clang/test/Driver/xcore-opts.c
===
--- clang/test/Driver/xcore-opts.c
+++ clang/test/Driver/xcore-opts.c
@@ -3,9 +3,8 @@
 // RUN: %clang -target xcore -x c++ %s -fexceptions -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-EXCEP %s
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
+// CHECK: "-mframe-pointer=none"
 // CHECK: "-nostdsysteminc"
-// CHECK-NOT: "-mdisable-fp-elim"
-// CHECK-NOT: "-momit-leaf-frame-pointer"
 // CHECK: "-fno-signed-char"
 // CHECK: "-fno-use-cxa-atexit"
 // CHECK-NOT: "-fcxx-exceptions"
Index: clang/test/Driver/woa-fp.c
===
--- clang/test/Driver/woa-fp.c
+++ clang/test/Driver/woa-fp.c
@@ -34,7 +34,7 @@
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -O3 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -Os -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 
-// CHECK-DEFAULT: "-mdisable-fp-elim"
-// CHECK-FPO-NOT: "-mdisable-fp-elim"
-// CHECK-NO-FPO: "-mdisable-fp-elim"
+// CHECK-DEFAULT: "-mframe-pointer=all"
+// CHECK-FPO-NOT: "-mframe-pointer=all"
+// CHECK-NO-FPO: "-mframe-pointer=all"
 
Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,15 +57,15 @@
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -Os %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
-// CHECK0-32: -mdisable-fp-elim
-// CHECK1-32-NOT: -mdisable-fp-elim
-// CHECK2-32-NOT: -mdisable-fp-elim
-// CHECK3-32-NOT: -mdisable-fp-elim
-// CHECKs-32-NOT: -mdisable-fp-elim
+// CHECK0-32: -mframe-pointer=all
+// CHECK1-32-NOT: -mframe-pointer=all
+// CHECK2-32-NOT: -mframe-pointer=all
+// CHECK3-32-NOT: -mframe-pointer=all
+// CHECKs-32-NOT: -mframe-pointer=all
 
-// CHECK0-64: -mdisable-fp-elim
-// CHECK1-64-NOT: -mdisable-fp-elim
-// CHECK2-64-NOT: -mdisable-fp-elim
-// CHECK3-64-NOT: -mdisable-fp-elim
-// CHECKs-64-NOT: -mdisable-fp-elim
-// CHECK-MACHO-64: -mdisable-fp-elim
+// CHECK0-64: -mframe-pointer=all
+// CHECK1-64-NOT: -mframe-pointer=all
+// CHECK2-64-NOT: -mframe-pointer=all
+// CHECK3-64-NOT: -mframe-pointer=all
+// CHECKs-64-NOT: -mframe-pointer=all
+// CHECK-MACHO-64: -mframe-pointer=all
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -1,11 +1,6 @@
-// KEEP-ALL: "-mdisable-fp-elim"
-// KEEP-ALL-NOT: "-momit-leaf-frame-pointer"
-
-// KEEP-NON-LEAF: "-mdisable-fp-elim"
-// KEEP-NON-LEAF: "-momit-leaf-frame-pointer"
-
-// KEEP-NONE-NOT: "-mdisable-fp-elim"
-// KEEP-NONE-NOT: "-momit-leaf-frame-pointer"
+// KEEP-ALL:  "-mframe-pointer=all"
+// KEEP-NON-LEAF: "-mframe-pointer=non-leaf"
+// KEEP-NONE: "-mframe-pointer=none"
 
 // On Linux x86, omit frame pointer when optimization is enabled.
 // RUN: %clang -### -target i386-linux -S -fomit-frame-pointer %s 2>&1 | \
@@ -73,20 +68,17 @@
 // RUN: %clang -### -target armv7s-apple-ios -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7S %s
 // WARN-OMIT-7S: warning: optimization flag '-fomit-frame-pointer' is not supported for target 'armv7s'
-// WARN-OMIT-7S: "-mdisable-fp-elim"
-// WARN-OMIT-7S-NOT: "-momit-leaf-frame-pointer"
+// WARN-OMIT-7S: "-mframe-pointer=all"
 
 // RUN: %clang -### -target armv7k-apple-watchos -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck 

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Yuanfang Chen via Phabricator via cfe-commits]

ychen marked an inline comment as done and an inline comment as not done.
ychen added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:845
+  if (Args.hasArg(OPT_pg))
+Opts.setFramePointer(CodeGenOptions::FP_All);
+

MaskRay wrote:
> I guess this can be deleted now.
> 
> `-pg` + FramePointerKind::None is rejected by the driver.
> 
> ```
>   if (Arg *A = Args.getLastArg(options::OPT_pg))
> if (FPKeepKind == FramePointerKind::None)
>   D.Diag(diag::err_drv_argument_not_allowed_with) << 
> "-fomit-frame-pointer"
>   << A->getAsString(Args);
> ```
This is still needed for `clang/test/CodeGen/x86_64-profiling-keep-fp.c` to 
pass.

We could have another patch to address this and update the test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:120
 
+  enum FramePointerKind {
+FP_All, // Keep all frame pointers.

We can probably use `enum class FramePointerKind { None, NonLeaf, All };` 
here.. (`enum class ClassABI` is an example in this file)



Comment at: clang/lib/CodeGen/CGCall.cpp:1723
+else
+  llvm_unreachable("unknown frame-pointer");
 

`llvm_unreachable` can be deleted if a switch statement is used.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:49
 
+using FramePointerKind = clang::CodeGenOptions::FramePointerKind;
 using namespace clang::driver;

This is probably not necessary. The enum is only used in 2 places below.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3961
+  else
+llvm_unreachable("unknown FramePointerKind");
+

`llvm_unreachable("unknown FramePointerKind");` can be deleted if a switch 
statement is used.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:845
+  if (Args.hasArg(OPT_pg))
+Opts.setFramePointer(CodeGenOptions::FP_All);
+

I guess this can be deleted now.

`-pg` + FramePointerKind::None is rejected by the driver.

```
  if (Arg *A = Args.getLastArg(options::OPT_pg))
if (FPKeepKind == FramePointerKind::None)
  D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
  << A->getAsString(Args);
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 209550.
ychen added a comment.

- rebase after D64294 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/msp430-fp-elim.c
  clang/test/CodeGen/x86_64-profiling-keep-fp.c
  clang/test/CodeGen/xcore-abi.c
  clang/test/CodeGen/xcore-abi.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenObjCXX/msabi-stret.mm
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-translation.c
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c
  clang/test/Driver/woa-fp.c
  clang/test/Driver/xcore-opts.c

Index: clang/test/Driver/xcore-opts.c
===
--- clang/test/Driver/xcore-opts.c
+++ clang/test/Driver/xcore-opts.c
@@ -3,9 +3,8 @@
 // RUN: %clang -target xcore -x c++ %s -fexceptions -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-EXCEP %s
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
+// CHECK: "-mframe-pointer=none"
 // CHECK: "-nostdsysteminc"
-// CHECK-NOT: "-mdisable-fp-elim"
-// CHECK-NOT: "-momit-leaf-frame-pointer"
 // CHECK: "-fno-signed-char"
 // CHECK: "-fno-use-cxa-atexit"
 // CHECK-NOT: "-fcxx-exceptions"
Index: clang/test/Driver/woa-fp.c
===
--- clang/test/Driver/woa-fp.c
+++ clang/test/Driver/woa-fp.c
@@ -34,7 +34,7 @@
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -O3 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -Os -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 
-// CHECK-DEFAULT: "-mdisable-fp-elim"
-// CHECK-FPO-NOT: "-mdisable-fp-elim"
-// CHECK-NO-FPO: "-mdisable-fp-elim"
+// CHECK-DEFAULT: "-mframe-pointer=all"
+// CHECK-FPO-NOT: "-mframe-pointer=all"
+// CHECK-NO-FPO: "-mframe-pointer=all"
 
Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,15 +57,15 @@
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -Os %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
-// CHECK0-32: -mdisable-fp-elim
-// CHECK1-32-NOT: -mdisable-fp-elim
-// CHECK2-32-NOT: -mdisable-fp-elim
-// CHECK3-32-NOT: -mdisable-fp-elim
-// CHECKs-32-NOT: -mdisable-fp-elim
+// CHECK0-32: -mframe-pointer=all
+// CHECK1-32-NOT: -mframe-pointer=all
+// CHECK2-32-NOT: -mframe-pointer=all
+// CHECK3-32-NOT: -mframe-pointer=all
+// CHECKs-32-NOT: -mframe-pointer=all
 
-// CHECK0-64: -mdisable-fp-elim
-// CHECK1-64-NOT: -mdisable-fp-elim
-// CHECK2-64-NOT: -mdisable-fp-elim
-// CHECK3-64-NOT: -mdisable-fp-elim
-// CHECKs-64-NOT: -mdisable-fp-elim
-// CHECK-MACHO-64: -mdisable-fp-elim
+// CHECK0-64: -mframe-pointer=all
+// CHECK1-64-NOT: -mframe-pointer=all
+// CHECK2-64-NOT: -mframe-pointer=all
+// CHECK3-64-NOT: -mframe-pointer=all
+// CHECKs-64-NOT: -mframe-pointer=all
+// CHECK-MACHO-64: -mframe-pointer=all
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -1,11 +1,6 @@
-// KEEP-ALL: "-mdisable-fp-elim"
-// KEEP-ALL-NOT: "-momit-leaf-frame-pointer"
-
-// KEEP-NON-LEAF: "-mdisable-fp-elim"
-// KEEP-NON-LEAF: "-momit-leaf-frame-pointer"
-
-// KEEP-NONE-NOT: "-mdisable-fp-elim"
-// KEEP-NONE-NOT: "-momit-leaf-frame-pointer"
+// KEEP-ALL:  "-mframe-pointer=all"
+// KEEP-NON-LEAF: "-mframe-pointer=non-leaf"
+// KEEP-NONE: "-mframe-pointer=none"
 
 // On Linux x86, omit frame pointer when optimization is enabled.
 // RUN: %clang -### -target i386-linux -S -fomit-frame-pointer %s 2>&1 | \
@@ -73,20 +68,17 @@
 // RUN: %clang -### -target armv7s-apple-ios -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7S %s
 // WARN-OMIT-7S: warning: optimization flag '-fomit-frame-pointer' is not supported for target 'armv7s'
-// WARN-OMIT-7S: "-mdisable-fp-elim"
-// WARN-OMIT-7S-NOT: "-momit-leaf-frame-pointer"
+// WARN-OMIT-7S: "-mframe-pointer=all"
 
 // RUN: %clang -### -target armv7k-apple-watchos -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7K %s
 // WARN-OMIT-7K: warning: optimization flag '-fomit-frame-pointer' is not supported for target 

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This should be rebased after D64294 .


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit added a comment.
Herald added a project: clang.

ping


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit marked 2 inline comments as done.
tabloid.adroit added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:576-579
+  Arg *FP = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
+options::OPT_fomit_frame_pointer);
+  Arg *LeafFP = Args.getLastArg(options::OPT_mno_omit_leaf_frame_pointer,
+options::OPT_momit_leaf_frame_pointer);

chandlerc wrote:
> This still doesn't make sense to me...
> 
> If the user specifies `-fomit-frame-point` or `-fno-omit-frame-pointer` 
> *after* `-momit-leaf-frame-pointer` or `-mno-omit-leaf-frame-pointer`, then 
> that last flag should win...
> 
> I think you need to first get the "base" FP state by checking the main two 
> flags. Then you need to get the "last" FP state by checking *all four flags*. 
> When the last flag is a leaf flag, then the state is determined by the base + 
> the last. When the last flag is not one of the leaf flags, then the last flag 
> fully specifies the result.
> 
> I think you can also process these variables into something simpler to test 
> below, essentially handling all the matching logic in one place.
I see the point here. I didn't do it since it deviates from GCC/intel compiler 
behavior. Could you please confirm we really want that?

One example from here is : ` -momit-leaf-frame-pointer -fno-omit-frame-pointer`
(https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-momit-leaf-frame-pointer)

For GCC/intel, this is frame-pointer=non-leaf, for us, it will be 
frame-pointer=all


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit updated this revision to Diff 182213.
tabloid.adroit marked an inline comment as done.
tabloid.adroit added a comment.

Thanks for the review!

- address comments


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/x86_64-profiling-keep-fp.c
  test/CodeGenCXX/global-init.cpp
  test/CodeGenObjCXX/msabi-stret.mm
  test/Driver/cl-options.c
  test/Driver/clang-translation.c
  test/Driver/frame-pointer-elim.c
  test/Driver/frame-pointer.c
  test/Driver/woa-fp.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -3,9 +3,8 @@
 // RUN: %clang -target xcore -x c++ %s -fexceptions -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-EXCEP %s
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
+// CHECK: "-mframe-pointer=none"
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
-// CHECK-NOT: "-mdisable-fp-elim"
 // CHECK: "-fno-signed-char"
 // CHECK: "-fno-use-cxa-atexit"
 // CHECK-NOT: "-fcxx-exceptions"
Index: test/Driver/woa-fp.c
===
--- test/Driver/woa-fp.c
+++ test/Driver/woa-fp.c
@@ -34,7 +34,7 @@
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -O3 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -Os -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 
-// CHECK-DEFAULT: "-mdisable-fp-elim"
-// CHECK-FPO-NOT: "-mdisable-fp-elim"
-// CHECK-NO-FPO: "-mdisable-fp-elim"
+// CHECK-DEFAULT: "-mframe-pointer=all"
+// CHECK-FPO-NOT: "-mframe-pointer=all"
+// CHECK-NO-FPO: "-mframe-pointer=all"
 
Index: test/Driver/frame-pointer.c
===
--- test/Driver/frame-pointer.c
+++ test/Driver/frame-pointer.c
@@ -57,15 +57,15 @@
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -Os %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
-// CHECK0-32: -mdisable-fp-elim
-// CHECK1-32-NOT: -mdisable-fp-elim
-// CHECK2-32-NOT: -mdisable-fp-elim
-// CHECK3-32-NOT: -mdisable-fp-elim
-// CHECKs-32-NOT: -mdisable-fp-elim
+// CHECK0-32: -mframe-pointer=all
+// CHECK1-32-NOT: -mframe-pointer=all
+// CHECK2-32-NOT: -mframe-pointer=all
+// CHECK3-32-NOT: -mframe-pointer=all
+// CHECKs-32-NOT: -mframe-pointer=all
 
-// CHECK0-64: -mdisable-fp-elim
-// CHECK1-64-NOT: -mdisable-fp-elim
-// CHECK2-64-NOT: -mdisable-fp-elim
-// CHECK3-64-NOT: -mdisable-fp-elim
-// CHECKs-64-NOT: -mdisable-fp-elim
-// CHECK-MACHO-64: -mdisable-fp-elim
+// CHECK0-64: -mframe-pointer=all
+// CHECK1-64-NOT: -mframe-pointer=all
+// CHECK2-64-NOT: -mframe-pointer=all
+// CHECK3-64-NOT: -mframe-pointer=all
+// CHECKs-64-NOT: -mframe-pointer=all
+// CHECK-MACHO-64: -mframe-pointer=all
Index: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -2,65 +2,62 @@
 // pointer, for unoptimized we should have a leaf frame pointer.
 // RUN: %clang -### -target i386-pc-linux-gnu -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=LINUX-OPT %s
-// LINUX-OPT: "-momit-leaf-frame-pointer"
+// LINUX-OPT: "-mframe-pointer=none"
 
 // RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// LINUX: "-mframe-pointer=all"
 
 // CloudABI follows the same rules as Linux.
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=CLOUDABI-OPT %s
-// CLOUDABI-OPT: "-momit-leaf-frame-pointer"
+// CLOUDABI-OPT: "-mframe-pointer=none"
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// CLOUDABI: "-mframe-pointer=all"
 
 // NetBSD follows the same rules as Linux.
 // RUN: %clang -### -target x86_64-unknown-netbsd -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NETBSD-OPT %s
-// NETBSD-OPT: "-momit-leaf-frame-pointer"
+// NETBSD-OPT: "-mframe-pointer=none"
 
 // RUN: %clang -### -target x86_64-unknown-netbsd -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NETBSD %s
-// NETBSD-NOT: "-momit-leaf-frame-pointer"
+// NETBSD: "-mframe-pointer=all"
 
 // Darwin disables omitting the leaf frame 

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

Wow, thanks for the cleanups. This is much easier to read as a consequence. And 
sorry it took a while to get you another round of review. Comments below.




Comment at: include/clang/Driver/CC1Options.td:270
+def mframe_pointer_EQ : Joined<["-"], "mframe-pointer=">,
+  HelpText<"Specify effect of frame pointer elimination optimization 
(all,non-leaf,none)">, Values<"all,non-leaf,none">;
 def mdisable_tail_calls : Flag<["-"], "mdisable-tail-calls">,

Should say more `Specify which frame pointers to retain (all, non-leaf, none)`.



Comment at: lib/Driver/ToolChains/Clang.cpp:574
 
-static bool shouldUseFramePointer(const ArgList ,
-  const llvm::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) ||
-   mustUseNonLeafFramePointerForTarget(Triple);
-
-  if (Args.hasArg(options::OPT_pg))
-return true;
-
-  return useFramePointerForTargetByDefault(Args, Triple);
-}
+static StringRef DetermineFramePointer(const ArgList ,
+   const llvm::Triple ) {

Keep LLVM coding convention naming pattern please.



Comment at: lib/Driver/ToolChains/Clang.cpp:576-579
+  Arg *FP = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
+options::OPT_fomit_frame_pointer);
+  Arg *LeafFP = Args.getLastArg(options::OPT_mno_omit_leaf_frame_pointer,
+options::OPT_momit_leaf_frame_pointer);

This still doesn't make sense to me...

If the user specifies `-fomit-frame-point` or `-fno-omit-frame-pointer` *after* 
`-momit-leaf-frame-pointer` or `-mno-omit-leaf-frame-pointer`, then that last 
flag should win...

I think you need to first get the "base" FP state by checking the main two 
flags. Then you need to get the "last" FP state by checking *all four flags*. 
When the last flag is a leaf flag, then the state is determined by the base + 
the last. When the last flag is not one of the leaf flags, then the last flag 
fully specifies the result.

I think you can also process these variables into something simpler to test 
below, essentially handling all the matching logic in one place.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit added a comment.

gentle ping


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56353/new/

https://reviews.llvm.org/D56353



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit created this revision.
tabloid.adroit added reviewers: chandlerc, rnk, t.p.northover.
Herald added a subscriber: cfe-commits.

set cc1 '-mframe-pointer=' according to driver options 
m(no-)omit-leaf-frame-pointer
and f(no-)omit-frame-pointer

This makes -fomit-frame-pointer imply -momit-leaf-frame-pointer.
This matches GCC's behavior. PR9825

following up
https://reviews.llvm.org/D56351

related:
https://reviews.llvm.org/D55915


Repository:
  rC Clang

https://reviews.llvm.org/D56353

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/x86_64-profiling-keep-fp.c
  test/CodeGenCXX/global-init.cpp
  test/CodeGenObjCXX/msabi-stret.mm
  test/Driver/cl-options.c
  test/Driver/clang-translation.c
  test/Driver/frame-pointer-elim.c
  test/Driver/frame-pointer.c
  test/Driver/woa-fp.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -3,9 +3,8 @@
 // RUN: %clang -target xcore -x c++ %s -fexceptions -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-EXCEP %s
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
+// CHECK: "-mframe-pointer=none"
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
-// CHECK-NOT: "-mdisable-fp-elim"
 // CHECK: "-fno-signed-char"
 // CHECK: "-fno-use-cxa-atexit"
 // CHECK-NOT: "-fcxx-exceptions"
Index: test/Driver/woa-fp.c
===
--- test/Driver/woa-fp.c
+++ test/Driver/woa-fp.c
@@ -34,7 +34,7 @@
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -O3 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 // RUN: %clang -target armv7-windows-itanium -fno-omit-frame-pointer -### -S %s -Os -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-NO-FPO
 
-// CHECK-DEFAULT: "-mdisable-fp-elim"
-// CHECK-FPO-NOT: "-mdisable-fp-elim"
-// CHECK-NO-FPO: "-mdisable-fp-elim"
+// CHECK-DEFAULT: "-mframe-pointer=all"
+// CHECK-FPO-NOT: "-mframe-pointer=all"
+// CHECK-NO-FPO: "-mframe-pointer=all"
 
Index: test/Driver/frame-pointer.c
===
--- test/Driver/frame-pointer.c
+++ test/Driver/frame-pointer.c
@@ -57,15 +57,15 @@
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -### -S -Os %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
-// CHECK0-32: -mdisable-fp-elim
-// CHECK1-32-NOT: -mdisable-fp-elim
-// CHECK2-32-NOT: -mdisable-fp-elim
-// CHECK3-32-NOT: -mdisable-fp-elim
-// CHECKs-32-NOT: -mdisable-fp-elim
+// CHECK0-32: -mframe-pointer=all
+// CHECK1-32-NOT: -mframe-pointer=all
+// CHECK2-32-NOT: -mframe-pointer=all
+// CHECK3-32-NOT: -mframe-pointer=all
+// CHECKs-32-NOT: -mframe-pointer=all
 
-// CHECK0-64: -mdisable-fp-elim
-// CHECK1-64-NOT: -mdisable-fp-elim
-// CHECK2-64-NOT: -mdisable-fp-elim
-// CHECK3-64-NOT: -mdisable-fp-elim
-// CHECKs-64-NOT: -mdisable-fp-elim
-// CHECK-MACHO-64: -mdisable-fp-elim
+// CHECK0-64: -mframe-pointer=all
+// CHECK1-64-NOT: -mframe-pointer=all
+// CHECK2-64-NOT: -mframe-pointer=all
+// CHECK3-64-NOT: -mframe-pointer=all
+// CHECKs-64-NOT: -mframe-pointer=all
+// CHECK-MACHO-64: -mframe-pointer=all
Index: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -2,65 +2,62 @@
 // pointer, for unoptimized we should have a leaf frame pointer.
 // RUN: %clang -### -target i386-pc-linux-gnu -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=LINUX-OPT %s
-// LINUX-OPT: "-momit-leaf-frame-pointer"
+// LINUX-OPT: "-mframe-pointer=none"
 
 // RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// LINUX: "-mframe-pointer=all"
 
 // CloudABI follows the same rules as Linux.
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=CLOUDABI-OPT %s
-// CLOUDABI-OPT: "-momit-leaf-frame-pointer"
+// CLOUDABI-OPT: "-mframe-pointer=none"
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// CLOUDABI: "-mframe-pointer=all"
 
 // NetBSD follows the same rules as Linux.
 // RUN: %clang -### -target x86_64-unknown-netbsd -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NETBSD-OPT %s
-// NETBSD-OPT: "-momit-leaf-frame-pointer"
+// NETBSD-OPT: "-mframe-pointer=none"
 
 // RUN: %clang -### -target