[clang] [clang][ARM] disable frame pointers by default for bare metal ARM targets (PR #117140)

Ties Stuij via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 29 02:54:53 PST 2024


https://github.com/stuij updated https://github.com/llvm/llvm-project/pull/117140

>From 4a85a0cd98bf328f31465d47c56640abdf7ec08c Mon Sep 17 00:00:00 2001
From: Ties Stuij <ties.stuij at arm.com>
Date: Fri, 15 Nov 2024 13:19:08 +0000
Subject: [PATCH 1/6] [clang][ARM] disable frame pointers by default for bare
 metal ARM targets

because:
- This brings Clang in line with GCC for which this is the default for ARM
- It frees up a register, so performance increase, especially on Thumb/6-M
- It will also decrease code size
---
 clang/lib/Driver/ToolChains/BareMetal.cpp  |  8 +++++-
 clang/lib/Driver/ToolChains/BareMetal.h    |  2 ++
 clang/lib/Driver/ToolChains/CommonArgs.cpp |  5 ++++
 clang/test/Driver/frame-pointer-elim.c     | 29 ++++++++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index f9a73f60973e4c..13b510e7e70994 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -128,8 +128,11 @@ BareMetal::BareMetal(const Driver &D, const llvm::Triple &Triple,
   }
 }
 
+namespace clang {
+namespace driver {
+namespace toolchains {
 /// Is the triple {arm,armeb,thumb,thumbeb}-none-none-{eabi,eabihf} ?
-static bool isARMBareMetal(const llvm::Triple &Triple) {
+bool isARMBareMetal(const llvm::Triple &Triple) {
   if (Triple.getArch() != llvm::Triple::arm &&
       Triple.getArch() != llvm::Triple::thumb &&
       Triple.getArch() != llvm::Triple::armeb &&
@@ -148,6 +151,9 @@ static bool isARMBareMetal(const llvm::Triple &Triple) {
 
   return true;
 }
+} // namespace clang
+} // namespace driver
+} // namespace clang
 
 /// Is the triple {aarch64.aarch64_be}-none-elf?
 static bool isAArch64BareMetal(const llvm::Triple &Triple) {
diff --git a/clang/lib/Driver/ToolChains/BareMetal.h b/clang/lib/Driver/ToolChains/BareMetal.h
index b385c8cf76aab0..ae09bcedd78a28 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -19,6 +19,8 @@ namespace driver {
 
 namespace toolchains {
 
+bool isARMBareMetal(const llvm::Triple &Triple);
+
 class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 public:
   BareMetal(const Driver &D, const llvm::Triple &Triple,
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 8d977149e62485..8d54d0a8649cc9 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -19,6 +19,7 @@
 #include "Arch/SystemZ.h"
 #include "Arch/VE.h"
 #include "Arch/X86.h"
+#include "BareMetal.h"
 #include "HIPAMD.h"
 #include "Hexagon.h"
 #include "MSP430.h"
@@ -151,6 +152,10 @@ static bool useFramePointerForTargetByDefault(const llvm::opt::ArgList &Args,
     }
   }
 
+  if (toolchains::isARMBareMetal(Triple)) {
+    return false;
+  }
+
   return true;
 }
 
diff --git a/clang/test/Driver/frame-pointer-elim.c b/clang/test/Driver/frame-pointer-elim.c
index cdedcc7ae4c89f..667c47b34bc703 100644
--- a/clang/test/Driver/frame-pointer-elim.c
+++ b/clang/test/Driver/frame-pointer-elim.c
@@ -162,5 +162,34 @@
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: not %clang -### --target=riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+
+// On ARM backend bare metal targets, frame pointer is omitted
+// RUN: %clang -### --target=arm-arm-none-eabi -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### --target=arm-arm-none-eabihf -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### --target=arm-arm-none-eabi -S -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-arm-none-eabihf -S -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-arm-none-eabi -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### --target=arm-arm-none-eabihf -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### --target=arm-arm-none-eabi -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-arm-none-eabihf -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
+// AArch64 bare metal targets behave like hosted targets
+// RUN: %clang -### --target=aarch64-none-elf -S %s 2>&1 |  \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=aarch64-none-elf -S -O1 %s 2>&1 |  \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=aarch64-none-elf -S -fno-omit-frame-pointer %s 2>&1 |  \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=aarch64-none-elf -S -O1 -fno-omit-frame-pointer %s 2>&1 |  \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+
 void f0() {}
 void f1() { f0(); }

>From 51a5d729a574a0c3ad149e12bb4f4798bd840d3e Mon Sep 17 00:00:00 2001
From: Ties Stuij <ties.stuij at arm.com>
Date: Thu, 21 Nov 2024 10:59:24 +0000
Subject: [PATCH 2/6] fix clang -> toolchains closing brace comment mislabel

---
 clang/lib/Driver/ToolChains/BareMetal.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 13b510e7e70994..a9e15bb2cbe7a7 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -151,7 +151,7 @@ bool isARMBareMetal(const llvm::Triple &Triple) {
 
   return true;
 }
-} // namespace clang
+} // namespace toolchains
 } // namespace driver
 } // namespace clang
 

>From 9aeb61f9968f4f0f8fd602809fc588263f26e785 Mon Sep 17 00:00:00 2001
From: Ties Stuij <ties.stuij at arm.com>
Date: Thu, 21 Nov 2024 16:33:48 +0000
Subject: [PATCH 3/6] Remove braces around single statement `if` body

---
 clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 8d54d0a8649cc9..c1f11e0c0cfa1e 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -152,9 +152,8 @@ static bool useFramePointerForTargetByDefault(const llvm::opt::ArgList &Args,
     }
   }
 
-  if (toolchains::isARMBareMetal(Triple)) {
+  if (toolchains::isARMBareMetal(Triple))
     return false;
-  }
 
   return true;
 }

>From d953a8dd1a02e91f313bd618544d6afa1d9c7d8f Mon Sep 17 00:00:00 2001
From: Ties Stuij <ties.stuij at arm.com>
Date: Tue, 26 Nov 2024 10:41:45 +0000
Subject: [PATCH 4/6] Add Apple embedded tests

also add a few more non-apple embedded tests and make isARMBareMetal slightly
more elegant
---
 clang/lib/Driver/ToolChains/BareMetal.cpp |  9 +++++----
 clang/test/Driver/frame-pointer-elim.c    | 24 +++++++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index a9e15bb2cbe7a7..6fa3912080e052 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -133,10 +133,11 @@ namespace driver {
 namespace toolchains {
 /// Is the triple {arm,armeb,thumb,thumbeb}-none-none-{eabi,eabihf} ?
 bool isARMBareMetal(const llvm::Triple &Triple) {
-  if (Triple.getArch() != llvm::Triple::arm &&
-      Triple.getArch() != llvm::Triple::thumb &&
-      Triple.getArch() != llvm::Triple::armeb &&
-      Triple.getArch() != llvm::Triple::thumbeb)
+  auto arch = Triple.getArch();
+  if (arch != llvm::Triple::arm &&
+      arch != llvm::Triple::thumb &&
+      arch != llvm::Triple::armeb &&
+      arch != llvm::Triple::thumbeb)
     return false;
 
   if (Triple.getVendor() != llvm::Triple::UnknownVendor)
diff --git a/clang/test/Driver/frame-pointer-elim.c b/clang/test/Driver/frame-pointer-elim.c
index 667c47b34bc703..f64ff6efc72610 100644
--- a/clang/test/Driver/frame-pointer-elim.c
+++ b/clang/test/Driver/frame-pointer-elim.c
@@ -180,6 +180,30 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### --target=arm-arm-none-eabihf -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=armeb-arm-none-eabi -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### --target=thumb-arm-none-eabi -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### --target=thumbeb-arm-none-eabi -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+
+// Check that for Apple bare metal targets, we're keeping frame pointers by default
+// RUN: %clang -### --target=thumbv6m-apple-none-macho -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=thumbv6m-apple-none-macho -S -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-apple-none-macho -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-apple-none-macho -S -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=thumbv6m-apple-none-macho -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=thumbv6m-apple-none-macho -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-apple-none-macho -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### --target=arm-apple-none-macho -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
 
 // AArch64 bare metal targets behave like hosted targets
 // RUN: %clang -### --target=aarch64-none-elf -S %s 2>&1 |  \

>From b972b8b9f0da001fd172dfe720ff7fe80e453ea2 Mon Sep 17 00:00:00 2001
From: Ties Stuij <ties.stuij at arm.com>
Date: Tue, 26 Nov 2024 11:51:57 +0000
Subject: [PATCH 5/6] satisfy clang-format

---
 clang/lib/Driver/ToolChains/BareMetal.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 6fa3912080e052..ce6b8ed812861d 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -134,10 +134,8 @@ namespace toolchains {
 /// Is the triple {arm,armeb,thumb,thumbeb}-none-none-{eabi,eabihf} ?
 bool isARMBareMetal(const llvm::Triple &Triple) {
   auto arch = Triple.getArch();
-  if (arch != llvm::Triple::arm &&
-      arch != llvm::Triple::thumb &&
-      arch != llvm::Triple::armeb &&
-      arch != llvm::Triple::thumbeb)
+  if (arch != llvm::Triple::arm && arch != llvm::Triple::thumb &&
+      arch != llvm::Triple::armeb && arch != llvm::Triple::thumbeb)
     return false;
 
   if (Triple.getVendor() != llvm::Triple::UnknownVendor)

>From 26c854f29c2c15736f42e4fa6bf3df14b42ea64b Mon Sep 17 00:00:00 2001
From: Ties Stuij <ties.stuij at arm.com>
Date: Fri, 29 Nov 2024 10:51:48 +0000
Subject: [PATCH 6/6] Rename isARMBareMetal -> isArmEABIBareMetal

To make it clearer what the fn is testing for.
---
 clang/lib/Driver/ToolChains/BareMetal.cpp  | 6 +++---
 clang/lib/Driver/ToolChains/BareMetal.h    | 2 +-
 clang/lib/Driver/ToolChains/CommonArgs.cpp | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index ce6b8ed812861d..af8fee100482a2 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -132,7 +132,7 @@ namespace clang {
 namespace driver {
 namespace toolchains {
 /// Is the triple {arm,armeb,thumb,thumbeb}-none-none-{eabi,eabihf} ?
-bool isARMBareMetal(const llvm::Triple &Triple) {
+bool isARMEABIBareMetal(const llvm::Triple &Triple) {
   auto arch = Triple.getArch();
   if (arch != llvm::Triple::arm && arch != llvm::Triple::thumb &&
       arch != llvm::Triple::armeb && arch != llvm::Triple::thumbeb)
@@ -272,7 +272,7 @@ void BareMetal::findMultilibs(const Driver &D, const llvm::Triple &Triple,
 }
 
 bool BareMetal::handlesTarget(const llvm::Triple &Triple) {
-  return isARMBareMetal(Triple) || isAArch64BareMetal(Triple) ||
+  return isARMEABIBareMetal(Triple) || isAArch64BareMetal(Triple) ||
          isRISCVBareMetal(Triple) || isPPCBareMetal(Triple);
 }
 
@@ -566,7 +566,7 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   // The R_ARM_TARGET2 relocation must be treated as R_ARM_REL32 on arm*-*-elf
   // and arm*-*-eabi (the default is R_ARM_GOT_PREL, used on arm*-*-linux and
   // arm*-*-*bsd).
-  if (isARMBareMetal(TC.getTriple()))
+  if (isARMEABIBareMetal(TC.getTriple()))
     CmdArgs.push_back("--target2=rel");
 
   CmdArgs.push_back("-o");
diff --git a/clang/lib/Driver/ToolChains/BareMetal.h b/clang/lib/Driver/ToolChains/BareMetal.h
index ae09bcedd78a28..49f3415909d742 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -19,7 +19,7 @@ namespace driver {
 
 namespace toolchains {
 
-bool isARMBareMetal(const llvm::Triple &Triple);
+bool isARMEABIBareMetal(const llvm::Triple &Triple);
 
 class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 public:
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index c1f11e0c0cfa1e..722c139a8a602b 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -152,7 +152,7 @@ static bool useFramePointerForTargetByDefault(const llvm::opt::ArgList &Args,
     }
   }
 
-  if (toolchains::isARMBareMetal(Triple))
+  if (toolchains::isARMEABIBareMetal(Triple))
     return false;
 
   return true;



More information about the cfe-commits mailing list