[llvm] 8eda716 - [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer
Nick Desaulniers via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 2 16:00:07 PDT 2020
Author: Nick Desaulniers
Date: 2020-06-02T15:54:14-07:00
New Revision: 8eda71616fecd098cbd7d2447859c8ac1315966f
URL: https://github.com/llvm/llvm-project/commit/8eda71616fecd098cbd7d2447859c8ac1315966f
DIFF: https://github.com/llvm/llvm-project/commit/8eda71616fecd098cbd7d2447859c8ac1315966f.diff
LOG: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer
Summary:
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-ndesaulniers@google.com/
[3] https://godbolt.org/z/0oY39t
Reviewers: kristof.beyls, psmith, danalbert, srhines, MaskRay, ostannard, efriedma
Reviewed By: psmith, danalbert, srhines, MaskRay, efriedma
Subscribers: efriedma, olista01, MaskRay, vhscampos, cfe-commits, llvm-commits, manojgupta, llozano, glider, hctim, eugenis, pcc, peter.smith, srhines
Tags: #clang, #llvm
Differential Revision: https://reviews.llvm.org/D80828
Added:
Modified:
clang/lib/Basic/Targets/ARM.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/frame-pointer-elim.c
llvm/docs/ReleaseNotes.rst
Removed:
################################################################################
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index f02a9d373609..08b6a9c0b917 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -314,7 +314,7 @@ ARMTargetInfo::ARMTargetInfo(const llvm::Triple &Triple,
// Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
// as well the default alignment
- if (IsAAPCS && (Triple.getEnvironment() != llvm::Triple::Android))
+ if (IsAAPCS && !Triple.isAndroid())
DefaultAlignForAttributeAligned = MaxVectorAlign = 64;
// Do force alignment of members that follow zero length bitfields. If
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4dfc41c6d3d2..d86fe499d69e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/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 @@ static bool useFramePointerForTargetByDefault(const ArgList &Args,
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:
diff --git a/clang/test/Driver/frame-pointer-elim.c b/clang/test/Driver/frame-pointer-elim.c
index 47c1c0549212..fd74da7768eb 100644
--- a/clang/test/Driver/frame-pointer-elim.c
+++ b/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(); }
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index a55d14c0cfa2..ea1fad1f6390 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@ During this release ...
* Implemented C-language intrinsics ``<arm_cde.h>`` 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
--------------------------
More information about the llvm-commits
mailing list