[PATCH] D74698: [Driver] -pg shouldn't unconditionally add -mframe-pointer=all w/ -mfentry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 245274.
nickdesaulniers added a comment.

- drop implicit-check-nots, add check for no os target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74698

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/mfentry.c


Index: clang/test/Driver/mfentry.c
===
--- clang/test/Driver/mfentry.c
+++ clang/test/Driver/mfentry.c
@@ -1,9 +1,19 @@
 // RUN: %clang -target s390x -c -### %s -mfentry 2>&1 | FileCheck %s
 // RUN: %clang -target i386 -c -### %s -mfentry 2>&1 | FileCheck %s
 // RUN: %clang -target x86_64 -c -### %s -mfentry 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O0 -### -E %s 2>&1 | 
FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O2 
-fno-omit-frame-pointer -### -E %s 2>&1 | FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O2 -### -E %s 2>&1 | 
FileCheck -check-prefix=NOFP %s
+// RUN: %clang -target x86_64 -pg -mfentry -O0 -### -E %s 2>&1 | FileCheck 
-check-prefix=FP %s
+// RUN: %clang -target x86_64 -pg -mfentry -O2 -fno-omit-frame-pointer -### -E 
%s 2>&1 | FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64 -pg -mfentry -O2 -### -E %s 2>&1 | FileCheck 
-check-prefix=FP %s
 
 // CHECK: "-mfentry"
 
 // RUN: %clang -target powerpc64le -c -### %s -mfentry 2>&1 | FileCheck 
--check-prefix=ERR %s
 
 // ERR: error: unsupported option '-mfentry' for target 'powerpc64le'
+
+// FP: "-mframe-pointer=all"
+// NOFP: "-mframe-pointer=none"
+void foo(void) {}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -526,7 +526,7 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg))
+  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -586,6 +586,10 @@
   return false;
 }
   }
+  // If no target OS was specified (so no special rules above related to
+  // optimization levels made a decision...
+  if (Args.hasArg(options::OPT_pg) && Args.hasArg(options::OPT_mfentry))
+return false;
 
   return true;
 }
@@ -6150,7 +6154,8 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (FPKeepKind == CodeGenOptions::FramePointerKind::None)
+if (FPKeepKind == CodeGenOptions::FramePointerKind::None &&
+!Args.hasArg(options::OPT_mfentry))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


Index: clang/test/Driver/mfentry.c
===
--- clang/test/Driver/mfentry.c
+++ clang/test/Driver/mfentry.c
@@ -1,9 +1,19 @@
 // RUN: %clang -target s390x -c -### %s -mfentry 2>&1 | FileCheck %s
 // RUN: %clang -target i386 -c -### %s -mfentry 2>&1 | FileCheck %s
 // RUN: %clang -target x86_64 -c -### %s -mfentry 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O0 -### -E %s 2>&1 | FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O2 -fno-omit-frame-pointer -### -E %s 2>&1 | FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O2 -### -E %s 2>&1 | FileCheck -check-prefix=NOFP %s
+// RUN: %clang -target x86_64 -pg -mfentry -O0 -### -E %s 2>&1 | FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64 -pg -mfentry -O2 -fno-omit-frame-pointer -### -E %s 2>&1 | FileCheck -check-prefix=FP %s
+// RUN: %clang -target x86_64 -pg -mfentry -O2 -### -E %s 2>&1 | FileCheck -check-prefix=FP %s
 
 // CHECK: "-mfentry"
 
 // RUN: %clang -target powerpc64le -c -### %s -mfentry 2>&1 | FileCheck --check-prefix=ERR %s
 
 // ERR: error: unsupported option '-mfentry' for target 'powerpc64le'
+
+// FP: "-mframe-pointer=all"
+// NOFP: "-mframe-pointer=none"
+void foo(void) {}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -526,7 +526,7 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg))
+  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -586,6 +586,10 @@
   return false;
 }
   }
+  // If no target OS was specified (so no special rules above related to
+  // optimization levels made a decision...
+  if (Args.hasArg(options::OPT_pg) && 

[PATCH] D74698: [Driver] -pg shouldn't unconditionally add -mframe-pointer=all w/ -mfentry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/test/Driver/mfentry.c:6
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O2 
-fno-omit-frame-pointer -### -E %s 2>&1 | FileCheck -check-prefix=FP 
--implicit-check-not='"-mframe-pointer=none"' %s
+// RUN: %clang -target x86_64-linux-gnu -pg -mfentry -O2 -### -E %s 2>&1 | 
FileCheck -check-prefix=NOFP --implicit-check-not='"-mframe-pointer=all"' %s
 

The only issue left is that `-target x86_64` still produces frame pointers 
since the above `useFramePointerForTargetByDefault()` just returns `true` when 
no target triple OS is specified. Do we care about that case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74698



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