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

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8eda71616fec: [Clang][A32/T32][Linux] -O1 implies 
-fomit-frame-pointer (authored by nickdesaulniers).

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)
   //

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

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

thanks homies, committed as 
https://github.com/llvm/llvm-project/commit/8eda71616fecd098cbd7d2447859c8ac1315966f


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] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits 
efriedma accepted this revision.
efriedma added a comment.

LGTM.  For non-Android, I think it makes sense to align with gcc as much as 
possible.

> 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.

It's also really expensive to use r11 as a frame pointer in Thumb1.  I guess 
it's not impossible, though.


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] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Peter Smith via Phabricator via cfe-commits 
psmith accepted this revision.
psmith added a comment.

LGTM from an Arm person now that the Android changes have been made.




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

nickdesaulniers wrote:
> 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?
One possible solution here is to commit the reordering separately using an 
[NFC] refactoring commit, no separate review required, then commit the rest of 
the patch after that. Anyone doing a source code search will only pick up 
significant changes.


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] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a subscriber: olista01.
MaskRay added a comment.

In D80828#2069287 , @nickdesaulniers 
wrote:

> May I please have a non-Googler to review+(accept|reject) the revision?


I guess @olista01 is an inactive account. Changed to @ostannard


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] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

May I please have a non-Googler to review+(accept|reject) the revision?


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] -O1 implies -fomit-frame-pointer

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

In D80828#2067110 , @MaskRay wrote:

> (You can change `[Clang]` to `[Driver]` as `[Clang]` may carry less 
> information. `[Driver]` emphasizes this is related to clangDriver. Nothing in 
> sema/codegen/analyzer/etc is affected.)


Isn't that ambiguous which driver is being modified? For instance, lldb and 
lld's drivers aren't being modified.


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] -O1 implies -fomit-frame-pointer

2020-06-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added a comment.

(You can change `[Clang]` to `[Driver]` as `[Clang]` may carry less 
information. `[Driver]` emphasizes this is related to clangDriver. Nothing in 
sema/codegen/analyzer/etc is affected.)


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