[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 267700.
nickdesaulniers added a comment.

- double backticks in release notes, -O1 rather than -O2 in tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
 
 * Implemented C-language intrinsics  for the CDE instruction set.
 
+* Clang now defaults to ``-fomit-frame-pointer`` when targeting non-Android
+  Linux for arm and thumb when optimizations are enabled. Users that were
+  previously not specifying a value and relying on the implicit compiler
+  default may wish to specify ``-fno-omit-frame-pointer`` to get the old
+  behavior. This improves compatibility with GCC.
+
 Changes to the MIPS Target
 --
 
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,33 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
+// For AAarch32 (A32, T32) linux targets, default omit frame pointer when
+// optimizations are enabled.
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// For Android, keep the framepointers always.
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Clang.h"
+#include "AMDGPU.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
@@ -15,11 +16,10 @@
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
-#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "Hexagon.h"
-#include "MSP430.h"
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -35,6 +35,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -557,6 +558,13 @@
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  if (Triple.isAndroid())
+return true;
+  LLVM_FALLTHROUGH;
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -314,7 +314,7 @@
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
   // as well the default alignment
-  if 

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/test/Driver/frame-pointer-elim.c:125
+// For Android, keep the framepointers always.
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s

`-O1` might be better. Users can infer that -O2 omits frame pointers as well 
since -O1 omits them.



Comment at: llvm/docs/ReleaseNotes.rst:97
+  not specifying a value and relying on the implicit compiler default may wish
+  to specify `-fno-omit-frame-pointer` to get the old behavior. This improves
+  compatibility with GCC.

Double backquotes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Stephen Hines via Phabricator via cfe-commits
srhines accepted this revision.
srhines added a comment.
This revision is now accepted and ready to land.

Thanks, Nick, for cleaning this up and always striving to make things more 
compatible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 267683.
nickdesaulniers added a comment.

- update release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
 
 * Implemented C-language intrinsics  for the CDE instruction set.
 
+* Clang now defaults to `-fomit-frame-pointer` when targeting non-Android Linux
+  for arm and thumb when optimizations are enabled. Users that were previously
+  not specifying a value and relying on the implicit compiler default may wish
+  to specify `-fno-omit-frame-pointer` to get the old behavior. This improves
+  compatibility with GCC.
+
 Changes to the MIPS Target
 --
 
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,33 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
+// For AAarch32 (A32, T32) linux targets, default omit frame pointer when
+// optimizations are enabled.
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// For Android, keep the framepointers always.
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Clang.h"
+#include "AMDGPU.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
@@ -15,11 +16,10 @@
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
-#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "Hexagon.h"
-#include "MSP430.h"
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -35,6 +35,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -557,6 +558,13 @@
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  if (Triple.isAndroid())
+return true;
+  LLVM_FALLTHROUGH;
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -314,7 +314,7 @@
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
   // as well the default alignment
-  if (IsAAPCS && (Triple.getEnvironment() != 

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 267682.
nickdesaulniers added a comment.

- prefer Triple#isAndroid


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
 
 * Implemented C-language intrinsics  for the CDE instruction set.
 
+* Clang now defaults to `-fomit-frame-pointer` when targeting Linux for arm
+  and thumb when optimizations are enabled. Users that were previously not
+  specifying a value and relying on the implicit compiler default may wish
+  to specify `-fno-omit-frame-pointer` to get the old behavior. This improves
+  compatibility with GCC.
+
 Changes to the MIPS Target
 --
 
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,33 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
+// For AAarch32 (A32, T32) linux targets, default omit frame pointer when
+// optimizations are enabled.
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// For Android, keep the framepointers always.
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Clang.h"
+#include "AMDGPU.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
@@ -15,11 +16,10 @@
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
-#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "Hexagon.h"
-#include "MSP430.h"
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -35,6 +35,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -557,6 +558,13 @@
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  if (Triple.isAndroid())
+return true;
+  LLVM_FALLTHROUGH;
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -314,7 +314,7 @@
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
   // as well the default alignment
-  if (IsAAPCS && (Triple.getEnvironment() != 

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Stephen Hines via Phabricator via cfe-commits
srhines added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:565
+case llvm::Triple::thumbeb:
+  if (Triple.getEnvironment() == llvm::Triple::Android)
+return true;

Triple.isAndroid() is more clear.



Comment at: llvm/docs/ReleaseNotes.rst:94
 
+* Clang now defaults to `-fomit-frame-pointer` when targeting Linux for arm
+  and thumb when optimizations are enabled. Users that were previously not

"non-Android Linux" is probably going to make things easier to understand here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 267663.
nickdesaulniers added a comment.

- add Android target triple handling, see 
https://developer.android.com/ndk/guides/other_build_systems#overview


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
 
 * Implemented C-language intrinsics  for the CDE instruction set.
 
+* Clang now defaults to `-fomit-frame-pointer` when targeting Linux for arm
+  and thumb when optimizations are enabled. Users that were previously not
+  specifying a value and relying on the implicit compiler default may wish
+  to specify `-fno-omit-frame-pointer` to get the old behavior. This improves
+  compatibility with GCC.
+
 Changes to the MIPS Target
 --
 
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,33 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
+// For AAarch32 (A32, T32) linux targets, default omit frame pointer when
+// optimizations are enabled.
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// For Android, keep the framepointers always.
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Clang.h"
+#include "AMDGPU.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
@@ -15,11 +16,10 @@
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
-#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "Hexagon.h"
-#include "MSP430.h"
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -35,6 +35,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -557,6 +558,13 @@
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  if (Triple.getEnvironment() == llvm::Triple::Android)
+return true;
+  LLVM_FALLTHROUGH;
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:22
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"

Note to reviewers: the linter really wanted to touch this header inclusion 
list, since I add "llvm/Support/Compiler.h" below.  Hopefully it's ok to just 
include with this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-06-01 Thread Peter Smith via Phabricator via cfe-commits
psmith added a comment.

FWIW I'm happy for Clang to match GCC behaviour for Linux (non-Android) targets 
with respect to the frame pointer. Outside of Android I would expect most 
developers of linux applications to build with both GCC and clang so they 
should already have the -fno-omit-frame-pointer if they are using it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-05-29 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:560
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:

srhines wrote:
> For the new targets, we should only be changing the default for non-Android 
> cases. Android targets should still default to `-fno-omit-frame-pointer`. 
> This makes the logic here quite a bit more complex.
It shouldn't be too bad, just need these four cases to check 
`Triple.getEnvironment() != llvm::Triple::Android` before calling 
`areOptimizationsEnabled`. Will double up the unit tests though, which is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-05-29 Thread Stephen Hines via Phabricator via cfe-commits
srhines added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:560
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:

For the new targets, we should only be changing the default for non-Android 
cases. Android targets should still default to `-fno-omit-frame-pointer`. This 
makes the logic here quite a bit more complex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

2020-05-29 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers created this revision.
nickdesaulniers added reviewers: kristof.beyls, psmith, olista01.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
nickdesaulniers added a subscriber: vhscampos.

An upgrade of LLVM for CrOS [0] containing [1] triggered a bunch of
errors related to writing to reserved registers for a Linux kernel's
arm64 compat vdso (which is a aarch32 image).

After a discussion on LKML [2], it was determined that
-f{no-}omit-frame-pointer was not being specified. Comparing GCC and
Clang [3], it becomes apparent that GCC defaults to omitting the frame
pointer implicitly when optimizations are enabled, and Clang does not.
ie. setting -O1 (or above) implies -fomit-frame-pointer. Clang was
defaulting to -fno-omit-frame-pointer implicitly unless -fomit-frame-pointer
was set explicitly.

Why this becomes a problem is that the Linux kernel's arm64 compat vdso
contains code that uses r7. r7 is used sometimes for the frame pointer
(for example, when targeting thumb (-mthumb)). See useR7AsFramePointer()
in llvm/llvm-project/llvm/lib/Target/ARM/ARMSubtarget.h. This is mostly
for legacy/compatibility reasons, and the 2019 Q4 revision of the ARM
AAPCS looks to standardize r11 as the frame pointer for aarch32, though
this is not yet implemented in LLVM.

Users that are reliant on the implicit value if unspecified when
optimizations are enabled should explicitly choose -fomit-frame-pointer
(new behavior) or -fno-omit-frame-pointer (old behavior).

[0] https://bugs.chromium.org/p/chromium/issues/detail?id=1084372
[1] https://reviews.llvm.org/D76848
[2] 
https://lore.kernel.org/lkml/20200526173117.155339-1-ndesaulni...@google.com/
[3] https://godbolt.org/z/0oY39t


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80828

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
 
 * Implemented C-language intrinsics  for the CDE instruction 
set.
 
+* Clang now defaults to `-fomit-frame-pointer` when targeting Linux for arm
+  and thumb when optimizations are enabled. Users that were previously not
+  specifying a value and relying on the implicit compiler default may wish
+  to specify `-fno-omit-frame-pointer` to get the old behavior. This improves
+  compatibility with GCC.
+
 Changes to the MIPS Target
 --
 
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,24 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
+// For AAarch32 (A32, T32) linux targets, default omit frame pointer when
+// optimizations are enabled.
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -S %s 
2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -O2 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -O2 -S %s 
2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -O2 -S 
%s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -557,11 +557,15 @@
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
 case llvm::Triple::mipsel:
 case llvm::Triple::systemz:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
 case llvm::Triple::x86:
 case llvm::Triple::x86_64:
   return !areOptimizationsEnabled(Args);


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++