[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Yuanfang Chen via Phabricator via cfe-commits]

ychen accepted this revision.
ychen added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked 5 inline comments as done.
MaskRay added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:579
 
-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 bool shouldUseLeafFramePointer(const ArgList ,
-  const llvm::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 useFramePointerForTargetByDefault(Args, Triple);
+static FramePointerKind getFramePointerKind(const ArgList ,
+const llvm::Triple ) {

ychen wrote:
> `getFramePointerKind` -> `decideFramePointerKind` / 
> `determineFramePointerKind` ? 
This is a pure function. I think it is fine to use `get`. This is also 
consistent with several other `get*` in this file.



Comment at: lib/Driver/ToolChains/Clang.cpp:585
+  (A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ||
+  (!(A && A->getOption().matches(options::OPT_fomit_frame_pointer)) &&
+   (Args.hasArg(options::OPT_pg) ||

ychen wrote:
> MaskRay wrote:
> > ychen wrote:
> > > It looks better if  `frame_pointer` is represented using tri-state. 
> > > Something like this?
> > > 
> > > It would be great to have comments for conditions that are not obvious 
> > > such as the overriding rules.
> > > 
> > > ```
> > >   // There are three states for frame_pointer.
> > >   enum class FpFlag {true, false, none};
> > >   FpFlag FPF = FpFlag::none;
> > >   if (Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
> > >options::OPT_fno_omit_frame_pointer))
> > > FPF = A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ?
> > >  FpFlag::true : FpFlag::false;
> > > 
> > >   if (!mustUseNonLeaf && FPF == FpFlag::false)
> > > return FramePointerKind::None;
> > > 
> > >   if (mustUseNonLeaf || FPF == FpFlag::true || 
> > > Args.hasArg(options::OPT_pg) ||
> > >   useFramePointerForTargetByDefault(Args, Triple)) {
> > > 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 FramePointerKind::None;
> > > ```
> > I actually think the current version is clearer.. The local `enum class 
> > FpFlag {true, false, none};` doesn't improve readability in my opinion.
> > 
> > 
> > I can define separate variables for:
> > 
> > * A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)
> > * A && A->getOption().matches(options::OPT_fomit_frame_pointer)
> > 
> > If reviewers think that makes the code easier to read.
> I think local enum may be optional.
> 
> Say 
>   - `Fp  = A && 
> A->getOption().matches(options::OPT_fno_omit_frame_pointer)`
>   - `NoFp = A && A->getOption().matches(options::OPT_fomit_frame_pointer)`
> 
> The `!(A && A->getOption().matches(options::OPT_fomit_frame_pointer))` in the 
> current revision could be `!A`. The implicit logic is `NoFp`  could only be 
> overriden by `mustUseNonLeaf`.
> 
> This block helps to make the implicit logic explicit and simplify the rest of 
> the code.
> 
> ```
> if (!mustUseNonLeaf && NoFp)
>   return FramePointerKind::None;
> }
> ```
> 
> 
I refactored the code a bit.

I still prefer the current approach to return 3 possible values. Too many 
`return` statements just clutter the code I think.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 209112.
MaskRay added a comment.

Improve readability


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/cl-options.c
  test/Driver/clang_f_opts.c
  test/Driver/frame-pointer-elim.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -4,8 +4,8 @@
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
 // 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: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -1,48 +1,74 @@
-// For these next two tests when optimized we should omit the leaf frame
-// 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"
-
-// RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// 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"
+
+// On Linux x86, omit frame pointer when optimization is enabled.
+// RUN: %clang -### -target i386-linux -S -fomit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target i386-linux -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// -fno-omit-frame-pointer or -pg disables frame pointer omission.
+// RUN: %clang -### -target i386-linux -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target i386-linux -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target i386-linux -S -O1 -pg %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
+// -momit-leaf-frame-pointer omits leaf frame pointer.
+// -fno-omit-frame-pointer loses out to -momit-leaf-frame-pointer.
+// RUN: %clang -### -target i386 -S -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target i386-linux -S -O1 -fno-omit-frame-pointer -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target i386-linux -S -O1 -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// Explicit or default -fomit-frame-pointer wins over -mno-omit-leaf-frame-pointer.
+// RUN: %clang -### -target i386 -S %s -fomit-frame-pointer -mno-omit-leaf-frame-pointer 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target i386-linux -S %s -O1 -mno-omit-leaf-frame-pointer 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// -pg -fomit-frame-pointer => error.
+// RUN: %clang -### -S -fomit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-OMIT-FP-PG %s
+// RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
+// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
+// CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg'
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-netbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=NETBSD %s
-// NETBSD-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang -### -target i386-apple-darwin -S %s 2>&1 | \

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:585
+  (A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ||
+  (!(A && A->getOption().matches(options::OPT_fomit_frame_pointer)) &&
+   (Args.hasArg(options::OPT_pg) ||

MaskRay wrote:
> ychen wrote:
> > It looks better if  `frame_pointer` is represented using tri-state. 
> > Something like this?
> > 
> > It would be great to have comments for conditions that are not obvious such 
> > as the overriding rules.
> > 
> > ```
> >   // There are three states for frame_pointer.
> >   enum class FpFlag {true, false, none};
> >   FpFlag FPF = FpFlag::none;
> >   if (Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
> >options::OPT_fno_omit_frame_pointer))
> > FPF = A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ?
> >  FpFlag::true : FpFlag::false;
> > 
> >   if (!mustUseNonLeaf && FPF == FpFlag::false)
> > return FramePointerKind::None;
> > 
> >   if (mustUseNonLeaf || FPF == FpFlag::true || Args.hasArg(options::OPT_pg) 
> > ||
> >   useFramePointerForTargetByDefault(Args, Triple)) {
> > 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 FramePointerKind::None;
> > ```
> I actually think the current version is clearer.. The local `enum class 
> FpFlag {true, false, none};` doesn't improve readability in my opinion.
> 
> 
> I can define separate variables for:
> 
> * A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)
> * A && A->getOption().matches(options::OPT_fomit_frame_pointer)
> 
> If reviewers think that makes the code easier to read.
I think local enum may be optional.

Say 
  - `Fp  = A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)`
  - `NoFp = A && A->getOption().matches(options::OPT_fomit_frame_pointer)`

The `!(A && A->getOption().matches(options::OPT_fomit_frame_pointer))` in the 
current revision could be `!A`. The implicit logic is `NoFp`  could only be 
overriden by `mustUseNonLeaf`.

This block helps to make the implicit logic explicit and simplify the rest of 
the code.

```
if (!mustUseNonLeaf && NoFp)
  return FramePointerKind::None;
}
```




Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:585
+  (A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ||
+  (!(A && A->getOption().matches(options::OPT_fomit_frame_pointer)) &&
+   (Args.hasArg(options::OPT_pg) ||

ychen wrote:
> It looks better if  `frame_pointer` is represented using tri-state. Something 
> like this?
> 
> It would be great to have comments for conditions that are not obvious such 
> as the overriding rules.
> 
> ```
>   // There are three states for frame_pointer.
>   enum class FpFlag {true, false, none};
>   FpFlag FPF = FpFlag::none;
>   if (Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
>options::OPT_fno_omit_frame_pointer))
> FPF = A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ?
>  FpFlag::true : FpFlag::false;
> 
>   if (!mustUseNonLeaf && FPF == FpFlag::false)
> return FramePointerKind::None;
> 
>   if (mustUseNonLeaf || FPF == FpFlag::true || Args.hasArg(options::OPT_pg) ||
>   useFramePointerForTargetByDefault(Args, Triple)) {
> 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 FramePointerKind::None;
> ```
I actually think the current version is clearer.. The local `enum class FpFlag 
{true, false, none};` doesn't improve readability in my opinion.


I can define separate variables for:

* A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)
* A && A->getOption().matches(options::OPT_fomit_frame_pointer)

If reviewers think that makes the code easier to read.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:579
 
-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 bool shouldUseLeafFramePointer(const ArgList ,
-  const llvm::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 useFramePointerForTargetByDefault(Args, Triple);
+static FramePointerKind getFramePointerKind(const ArgList ,
+const llvm::Triple ) {

`getFramePointerKind` -> `decideFramePointerKind` / `determineFramePointerKind` 
? 



Comment at: lib/Driver/ToolChains/Clang.cpp:585
+  (A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ||
+  (!(A && A->getOption().matches(options::OPT_fomit_frame_pointer)) &&
+   (Args.hasArg(options::OPT_pg) ||

It looks better if  `frame_pointer` is represented using tri-state. Something 
like this?

It would be great to have comments for conditions that are not obvious such as 
the overriding rules.

```
  // There are three states for frame_pointer.
  enum class FpFlag {true, false, none};
  FpFlag FPF = FpFlag::none;
  if (Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
   options::OPT_fno_omit_frame_pointer))
FPF = A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ?
 FpFlag::true : FpFlag::false;

  if (!mustUseNonLeaf && FPF == FpFlag::false)
return FramePointerKind::None;

  if (mustUseNonLeaf || FPF == FpFlag::true || Args.hasArg(options::OPT_pg) ||
  useFramePointerForTargetByDefault(Args, Triple)) {
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 FramePointerKind::None;
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@ychen The GCC behavior ("omit" wins over "no-omit") makes sense because: in 
the 4 states:

- 00) leaf retained, non-leaf retained
- 01) leaf retained, non-leaf omitted
- 10) leaf omitted, non-leaf retained
- 11) leaf omitted, non-leaf omitted

State 10) doesn't make sense. I think `-momit-leaf-frame-pointer` was designed 
to omit leaf frame pointer when frame pointer is enabled 
(`-fno-omit-frame-pointer` is in action). To make the other 3 states 
representable, letting "omit" wins over "no-omit" (the current GCC behavior) is 
the only sensible choice.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208616.
MaskRay added a comment.

Simplify and add another test


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/cl-options.c
  test/Driver/clang_f_opts.c
  test/Driver/frame-pointer-elim.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -4,8 +4,8 @@
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
 // 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: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -1,48 +1,74 @@
-// For these next two tests when optimized we should omit the leaf frame
-// 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"
-
-// RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// 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"
+
+// On Linux x86, omit frame pointer when optimization is enabled.
+// RUN: %clang -### -target i386-linux -S -fomit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target i386-linux -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// -fno-omit-frame-pointer or -pg disables frame pointer omission.
+// RUN: %clang -### -target i386-linux -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target i386-linux -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target i386-linux -S -O1 -pg %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
+// -momit-leaf-frame-pointer omits leaf frame pointer.
+// -fno-omit-frame-pointer loses out to -momit-leaf-frame-pointer.
+// RUN: %clang -### -target i386 -S -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target i386-linux -S -O1 -fno-omit-frame-pointer -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target i386-linux -S -O1 -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// Explicit or default -fomit-frame-pointer wins over -mno-omit-leaf-frame-pointer.
+// RUN: %clang -### -target i386 -S %s -fomit-frame-pointer -mno-omit-leaf-frame-pointer 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target i386-linux -S %s -O1 -mno-omit-leaf-frame-pointer 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// -pg -fomit-frame-pointer => error.
+// RUN: %clang -### -S -fomit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-OMIT-FP-PG %s
+// RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
+// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
+// CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg'
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-netbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=NETBSD %s
-// NETBSD-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang -### -target i386-apple-darwin -S %s 

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208615.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Implement the GCC behavior:

  -fomit-frame-pointer wins over -mno-omit-leaf-frame-pointer
  -fno-omit-frame-pointer loses out to -momit-leaf-frame-pointer


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/cl-options.c
  test/Driver/clang_f_opts.c
  test/Driver/frame-pointer-elim.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -4,8 +4,8 @@
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
 // 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: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -1,48 +1,72 @@
-// For these next two tests when optimized we should omit the leaf frame
-// 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"
-
-// RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// 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"
+
+// On Linux x86, omit frame pointer when optimization is enabled.
+// RUN: %clang -### -target i386-linux -S -fomit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target i386-linux -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// -fno-omit-frame-pointer or -pg disables frame pointer omission.
+// RUN: %clang -### -target i386-linux -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target i386-linux -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target i386-linux -S -O1 -pg %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
+// -momit-leaf-frame-pointer omits leaf frame pointer.
+// -fno-omit-frame-pointer loses out to -momit-leaf-frame-pointer.
+// RUN: %clang -### -target i386 -S -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target i386-linux -S -O1 -fno-omit-frame-pointer -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+
+// Explicit or default -fomit-frame-pointer wins over -mno-omit-leaf-frame-pointer.
+// RUN: %clang -### -target i386 -S %s -fomit-frame-pointer -mno-omit-leaf-frame-pointer 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target i386-linux -S %s -O1 -mno-omit-leaf-frame-pointer 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// -pg -fomit-frame-pointer => error.
+// RUN: %clang -### -S -fomit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-OMIT-FP-PG %s
+// RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
+// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
+// CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg'
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-netbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=NETBSD %s
-// NETBSD-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang 

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D56353 , I remember @chandlerc thought 
`-f(no-)omit-frame-pointer` should win over `-m(no-)omit-leaf-frame-pointer`. 
I'm not sure what his thoughts on this now. @chandlerc ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294



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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208289.
MaskRay added a comment.

Simplify


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/cl-options.c
  test/Driver/frame-pointer-elim.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -4,8 +4,8 @@
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
 // 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: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -1,48 +1,50 @@
+// 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"
+
 // For these next two tests when optimized we should omit the leaf frame
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
+// RUN: %clang -### -target i386-pc-linux-gnu -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-netbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=NETBSD %s
-// NETBSD-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang -### -target i386-apple-darwin -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=DARWIN %s
-// DARWIN: "-mdisable-fp-elim"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // RUN: %clang -### -target i386-apple-darwin -S -O1 %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=DARWIN-OPT %s
-// DARWIN-OPT-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // RUN: %clang -### -target i386-darwin -S -fomit-frame-pointer %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_ALL %s
-// OMIT_ALL-NOT: "-mdisable-fp-elim"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target i386-darwin -S -momit-leaf-frame-pointer %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_LEAF %s
-// OMIT_LEAF: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target armv7s-apple-ios -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7S %s
@@ -63,11 +65,10 @@
 // WARN-OMIT-LEAF-7S: "-momit-leaf-frame-pointer"
 
 // On the PS4, we default to omitting the frame pointer on leaf functions
-// (OMIT_LEAF check line is above)
 // RUN: %clang -### -target x86_64-scei-ps4 -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_LEAF %s
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target x86_64-scei-ps4 -S -O2 %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_LEAF %s
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 void f0() {}
 void f1() { f0(); }
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -161,28 +161,28 @@
 // RUN: %clang_cl /Os --target=i686-pc-windows-msvc -### -- %s 2>&1 | FileCheck -check-prefix=Os %s
 // RUN: %clang_cl /Os --target=x86_64-pc-windows-msvc -### -- %s 2>&1 | FileCheck -check-prefix=Os %s
 // Os-NOT: -mdisable-fp-elim

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: chandlerc, dim, echristo, rnk, rsmith, void, ychen.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Use a tri-state enum to represent shouldUseFramePointer() and
shouldUseLeafFramePointer().

This simplifies the logic and fixes PR9825:

  need both -fno-omit-frame-pointer and -mno-omit-leaf-frame-pointer to
  generate fp on a leaf function.

and PR24003:

  /Oy- /O2 should not omit leaf frame pointer.

and:

  when CC1 option -mdisable-fp-elim if absent, -momit-leaf-frame-pointer
  can also be omitted.


Repository:
  rC Clang

https://reviews.llvm.org/D64294

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/cl-options.c
  test/Driver/frame-pointer-elim.c
  test/Driver/xcore-opts.c

Index: test/Driver/xcore-opts.c
===
--- test/Driver/xcore-opts.c
+++ test/Driver/xcore-opts.c
@@ -4,8 +4,8 @@
 // RUN: %clang -target xcore %s -g0 -### -o %t.o 2>&1 | FileCheck -check-prefix CHECK-G0 %s
 
 // CHECK: "-nostdsysteminc"
-// CHECK: "-momit-leaf-frame-pointer"
 // 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: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -1,48 +1,50 @@
+// 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"
+
 // For these next two tests when optimized we should omit the leaf frame
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target i386-pc-linux-gnu -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=LINUX %s
-// LINUX-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
+// RUN: %clang -### -target i386-pc-linux-gnu -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=CLOUDABI %s
-// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // 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"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target x86_64-unknown-netbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=NETBSD %s
-// NETBSD-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang -### -target i386-apple-darwin -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=DARWIN %s
-// DARWIN: "-mdisable-fp-elim"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // RUN: %clang -### -target i386-apple-darwin -S -O1 %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=DARWIN-OPT %s
-// DARWIN-OPT-NOT: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // RUN: %clang -### -target i386-darwin -S -fomit-frame-pointer %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_ALL %s
-// OMIT_ALL-NOT: "-mdisable-fp-elim"
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
 // RUN: %clang -### -target i386-darwin -S -momit-leaf-frame-pointer %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_LEAF %s
-// OMIT_LEAF: "-momit-leaf-frame-pointer"
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target armv7s-apple-ios -fomit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=WARN-OMIT-7S %s
@@ -63,11 +65,10 @@
 // WARN-OMIT-LEAF-7S: "-momit-leaf-frame-pointer"
 
 // On the PS4, we default to omitting the frame pointer on leaf functions
-// (OMIT_LEAF check line is above)
 // RUN: %clang -### -target x86_64-scei-ps4 -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_LEAF %s
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target x86_64-scei-ps4 -S -O2 %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OMIT_LEAF %s
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 void