[ARM] Introduce -mtp=auto and make it the default #128901

Zhenhang1213 · 2025-02-26T16:13:46Z

This adds a new value auto to the possible values of the existing -mtp= clang option which controls how the thread pointer is found. auto means the same as soft if the target architecture doesn't support a hardware thread pointer at all; otherwise it means the same as cp15.

This behavior is the default in gcc version 4.1.0 and later. The new auto option is therefore also the default in clang, so this change aligns clang with gcc.

Fixes #123864.

llvmbot · 2025-02-26T16:14:23Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Austin (Zhenhang1213)

Changes

This adds a new value auto to the possible values of the existing -mtp= clang option which controls how the thread pointer is found. auto means the same as soft if the target architecture doesn't support a hardware thread pointer at all; otherwise it means the same as cp15.

This behavior is the default in gcc version 7.3.0 and later. The new auto option is therefore also the default in clang, so this change aligns clang with gcc.

Full diff: https://github.com/llvm/llvm-project/pull/128901.diff

4 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+1-1)
(modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+5-1)
(modified) clang/lib/Driver/ToolChains/Arch/ARM.h (+1)
(modified) clang/test/Driver/arm-thread-pointer.c (+5-1)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e521cbf678d93..2bd6076bea5d4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4711,7 +4711,7 @@ def mexecute_only : Flag<["-"], "mexecute-only">, Group<m_arm_Features_Group>,
 def mno_execute_only : Flag<["-"], "mno-execute-only">, Group<m_arm_Features_Group>,
   HelpText<"Allow generation of data access to code sections (ARM only)">;
 let Flags = [TargetSpecific] in {
-def mtp_mode_EQ : Joined<["-"], "mtp=">, Group<m_arm_Features_Group>, Values<"soft,cp15,tpidrurw,tpidruro,tpidrprw,el0,el1,el2,el3,tpidr_el0,tpidr_el1,tpidr_el2,tpidr_el3,tpidrro_el0">,
+def mtp_mode_EQ : Joined<["-"], "mtp=">, Group<m_arm_Features_Group>, Values<"soft,cp15,tpidrurw,tpidruro,tpidrprw,el0,el1,el2,el3,tpidr_el0,tpidr_el1,tpidr_el2,tpidr_el3,tpidrro_el0,auto">,
   HelpText<"Thread pointer access method. "
            "For AArch32: 'soft' uses a function call, or 'tpidrurw', 'tpidruro' or 'tpidrprw' use the three CP15 registers. 'cp15' is an alias for 'tpidruro'. "
            "For AArch64: 'tpidr_el0', 'tpidr_el1', 'tpidr_el2', 'tpidr_el3' or 'tpidrro_el0' use the five system registers. 'elN' is an alias for 'tpidr_elN'.">;
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 3aee540d501be..ec0be8f9dc587 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -223,6 +223,7 @@ arm::ReadTPMode arm::getReadTPMode(const Driver &D, const ArgList &Args,
             .Case("tpidruro", ReadTPMode::TPIDRURO)
             .Case("tpidrprw", ReadTPMode::TPIDRPRW)
             .Case("soft", ReadTPMode::Soft)
+            .Case("auto", ReadTPMode::Auto)
             .Default(ReadTPMode::Invalid);
     if ((ThreadPointer == ReadTPMode::TPIDRURW ||
          ThreadPointer == ReadTPMode::TPIDRURO ||
@@ -239,7 +240,7 @@ arm::ReadTPMode arm::getReadTPMode(const Driver &D, const ArgList &Args,
       D.Diag(diag::err_drv_invalid_mtp) << A->getAsString(Args);
     return ReadTPMode::Invalid;
   }
-  return ReadTPMode::Soft;
+  return ReadTPMode::Auto;
 }
 
 void arm::setArchNameInTriple(const Driver &D, const ArgList &Args,
@@ -580,6 +581,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
     Features.push_back("+read-tp-tpidruro");
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRPRW)
     Features.push_back("+read-tp-tpidrprw");
+  if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::Auto &&
+      isHardTPSupported(Triple) && !ForAS)
+    Features.push_back("+read-tp-tpidruro");
 
   const Arg *ArchArg = Args.getLastArg(options::OPT_march_EQ);
   const Arg *CPUArg = Args.getLastArg(options::OPT_mcpu_EQ);
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.h b/clang/lib/Driver/ToolChains/Arch/ARM.h
index a23a8793a89e2..622383cf0025d 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -41,6 +41,7 @@ enum class ReadTPMode {
   TPIDRURW,
   TPIDRURO,
   TPIDRPRW,
+  Auto,
 };
 
 enum class FloatABI {
diff --git a/clang/test/Driver/arm-thread-pointer.c b/clang/test/Driver/arm-thread-pointer.c
index 5521e1865b276..985c5046f6d26 100644
--- a/clang/test/Driver/arm-thread-pointer.c
+++ b/clang/test/Driver/arm-thread-pointer.c
@@ -42,4 +42,8 @@
 
 // RUN: %clang --target=armv7-linux -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_NON %s
-// ARMv7_THREAD_POINTER_NON-NOT: "-target-feature" "+read-tp-tpidruro"
+// ARMv7_THREAD_POINTER_NON: "-target-feature" "+read-tp-tpidruro"
+
+// RUN: %clang --target=armv7-linux -mtp=auto -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_Auto %s
+// ARMv7_THREAD_POINTER_Auto: "-target-feature" "+read-tp-tpidruro"

llvmbot · 2025-02-26T16:14:23Z

@llvm/pr-subscribers-backend-arm

Author: Austin (Zhenhang1213)

Changes

This adds a new value auto to the possible values of the existing -mtp= clang option which controls how the thread pointer is found. auto means the same as soft if the target architecture doesn't support a hardware thread pointer at all; otherwise it means the same as cp15.

This behavior is the default in gcc version 7.3.0 and later. The new auto option is therefore also the default in clang, so this change aligns clang with gcc.

Full diff: https://github.com/llvm/llvm-project/pull/128901.diff

4 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+1-1)
(modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+5-1)
(modified) clang/lib/Driver/ToolChains/Arch/ARM.h (+1)
(modified) clang/test/Driver/arm-thread-pointer.c (+5-1)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e521cbf678d93..2bd6076bea5d4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4711,7 +4711,7 @@ def mexecute_only : Flag<["-"], "mexecute-only">, Group<m_arm_Features_Group>,
 def mno_execute_only : Flag<["-"], "mno-execute-only">, Group<m_arm_Features_Group>,
   HelpText<"Allow generation of data access to code sections (ARM only)">;
 let Flags = [TargetSpecific] in {
-def mtp_mode_EQ : Joined<["-"], "mtp=">, Group<m_arm_Features_Group>, Values<"soft,cp15,tpidrurw,tpidruro,tpidrprw,el0,el1,el2,el3,tpidr_el0,tpidr_el1,tpidr_el2,tpidr_el3,tpidrro_el0">,
+def mtp_mode_EQ : Joined<["-"], "mtp=">, Group<m_arm_Features_Group>, Values<"soft,cp15,tpidrurw,tpidruro,tpidrprw,el0,el1,el2,el3,tpidr_el0,tpidr_el1,tpidr_el2,tpidr_el3,tpidrro_el0,auto">,
   HelpText<"Thread pointer access method. "
            "For AArch32: 'soft' uses a function call, or 'tpidrurw', 'tpidruro' or 'tpidrprw' use the three CP15 registers. 'cp15' is an alias for 'tpidruro'. "
            "For AArch64: 'tpidr_el0', 'tpidr_el1', 'tpidr_el2', 'tpidr_el3' or 'tpidrro_el0' use the five system registers. 'elN' is an alias for 'tpidr_elN'.">;
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 3aee540d501be..ec0be8f9dc587 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -223,6 +223,7 @@ arm::ReadTPMode arm::getReadTPMode(const Driver &D, const ArgList &Args,
             .Case("tpidruro", ReadTPMode::TPIDRURO)
             .Case("tpidrprw", ReadTPMode::TPIDRPRW)
             .Case("soft", ReadTPMode::Soft)
+            .Case("auto", ReadTPMode::Auto)
             .Default(ReadTPMode::Invalid);
     if ((ThreadPointer == ReadTPMode::TPIDRURW ||
          ThreadPointer == ReadTPMode::TPIDRURO ||
@@ -239,7 +240,7 @@ arm::ReadTPMode arm::getReadTPMode(const Driver &D, const ArgList &Args,
       D.Diag(diag::err_drv_invalid_mtp) << A->getAsString(Args);
     return ReadTPMode::Invalid;
   }
-  return ReadTPMode::Soft;
+  return ReadTPMode::Auto;
 }
 
 void arm::setArchNameInTriple(const Driver &D, const ArgList &Args,
@@ -580,6 +581,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
     Features.push_back("+read-tp-tpidruro");
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRPRW)
     Features.push_back("+read-tp-tpidrprw");
+  if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::Auto &&
+      isHardTPSupported(Triple) && !ForAS)
+    Features.push_back("+read-tp-tpidruro");
 
   const Arg *ArchArg = Args.getLastArg(options::OPT_march_EQ);
   const Arg *CPUArg = Args.getLastArg(options::OPT_mcpu_EQ);
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.h b/clang/lib/Driver/ToolChains/Arch/ARM.h
index a23a8793a89e2..622383cf0025d 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -41,6 +41,7 @@ enum class ReadTPMode {
   TPIDRURW,
   TPIDRURO,
   TPIDRPRW,
+  Auto,
 };
 
 enum class FloatABI {
diff --git a/clang/test/Driver/arm-thread-pointer.c b/clang/test/Driver/arm-thread-pointer.c
index 5521e1865b276..985c5046f6d26 100644
--- a/clang/test/Driver/arm-thread-pointer.c
+++ b/clang/test/Driver/arm-thread-pointer.c
@@ -42,4 +42,8 @@
 
 // RUN: %clang --target=armv7-linux -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_NON %s
-// ARMv7_THREAD_POINTER_NON-NOT: "-target-feature" "+read-tp-tpidruro"
+// ARMv7_THREAD_POINTER_NON: "-target-feature" "+read-tp-tpidruro"
+
+// RUN: %clang --target=armv7-linux -mtp=auto -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_Auto %s
+// ARMv7_THREAD_POINTER_Auto: "-target-feature" "+read-tp-tpidruro"

github-actions · 2025-02-27T06:51:26Z

✅ With the latest revision this PR passed the C/C++ code formatter.

smithp35

I assume this is a rewrite of #128728

It looks like we've lost an explicit -mtp=auto. I think that part is important, my apologies if my earlier comment implied that we should remove it. I've made a suggestion for how to support it with some minimal changes.

Please can you update clang/docs/ReleaseNotes.rst in the modified compiler flags section (https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst#modified-compiler-flags)

Something like:

The ARM AArch32 ``-mtp`` option accepts and defaults to ``auto``, a value of ``auto`` uses the best available method of providing the frame pointer supported by the hardware. This matches the behavior of ``-mtp`` in gcc. This changes the default behavior for ARM targets that provide the ``TPIDRURO`` register as this will be used instead of a call to the ``__aeabi_read_tp``. Programs that use ``__aeabi_read_tp`` but do not use the ``TPIDRURO`` register must use ``-mtp=soft``. Fixes #123864

clang/lib/Driver/ToolChains/Arch/ARM.cpp

clang/test/Driver/arm-thread-pointer.c

This adds a new value auto to the possible values of the existing -mtp= clang option which controls how the thread pointer is found. auto means the same as soft if the target architecture doesn't support a hardware thread pointer at all; otherwise it means the same as cp15. This behavior is the default in gcc version 4.1.0 and later. The new auto option is therefore also the default in clang, so this change aligns clang with gcc. Fixes llvm#123864.

smithp35

Only one small line to remove, otherwise looks good to me.

clang/lib/Driver/ToolChains/Arch/ARM.h

smithp35

LGTM from me.

Please leave some time for other reviewers to comment before merging.

statham-arm

LGTM too – my comments from #128728 have been addressed, and I have no new complaints to make about this version 🙂

hstk30-hw

LGTM based on gcc logic

This adds a new value auto to the possible values of the existing -mtp= clang option which controls how the thread pointer is found. auto means the same as soft if the target architecture doesn't support a hardware thread pointer at all; otherwise it means the same as cp15. This behavior is the default in gcc version 4.1.0 and later. The new auto option is therefore also the default in clang, so this change aligns clang with gcc. Fixes llvm#123864.

llvmbot added clang Clang issues not falling into any other category backend:ARM clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Feb 26, 2025

CoTinker requested review from statham-arm and smithp35 February 27, 2025 03:27

Zhenhang1213 force-pushed the fix_mtp branch from 88a31a1 to fb062da Compare February 27, 2025 06:48

Zhenhang1213 force-pushed the fix_mtp branch 2 times, most recently from 4bb02c6 to 3c2a6d7 Compare February 27, 2025 07:20

Zhenhang1213 changed the title ~~[ARM] Introduce -mtp=auto and make it the default~~ [ARM] make -mtp=TPIDRURO the default if the target architecture support a hardware thread pointer Feb 27, 2025

smithp35 reviewed Feb 27, 2025

View reviewed changes

clang/lib/Driver/ToolChains/Arch/ARM.cpp Show resolved Hide resolved

clang/test/Driver/arm-thread-pointer.c Show resolved Hide resolved

Zhenhang1213 changed the title ~~[ARM] make -mtp=TPIDRURO the default if the target architecture support a hardware thread pointer~~ [ARM] Introduce -mtp=auto and make it the default Feb 28, 2025

Zhenhang1213 force-pushed the fix_mtp branch 2 times, most recently from e2d7efb to 89f9a4d Compare February 28, 2025 03:05

Zhenhang1213 requested a review from smithp35 February 28, 2025 06:10

Zhenhang1213 force-pushed the fix_mtp branch 2 times, most recently from 1f3fed7 to 77a4ee6 Compare February 28, 2025 06:45

Zhenhang1213 force-pushed the fix_mtp branch from 77a4ee6 to 0db9830 Compare February 28, 2025 08:23

smithp35 reviewed Mar 3, 2025

View reviewed changes

clang/lib/Driver/ToolChains/Arch/ARM.h Outdated Show resolved Hide resolved

Merge branch 'main' into fix_mtp

f900dc7

Zhenhang1213 force-pushed the fix_mtp branch from f309242 to f900dc7 Compare March 3, 2025 10:40

smithp35 approved these changes Mar 3, 2025

View reviewed changes

statham-arm approved these changes Mar 3, 2025

View reviewed changes

hstk30-hw approved these changes Mar 3, 2025

View reviewed changes

hstk30-hw merged commit 742fa8a into llvm:main Mar 3, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ARM] Introduce -mtp=auto and make it the default #128901

[ARM] Introduce -mtp=auto and make it the default #128901

Uh oh!

Zhenhang1213 commented Feb 26, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 26, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 26, 2025

Uh oh!

github-actions bot commented Feb 27, 2025 •

edited

Loading

Uh oh!

smithp35 left a comment

Uh oh!

Uh oh!

Uh oh!

smithp35 left a comment

Uh oh!

Uh oh!

smithp35 left a comment

Uh oh!

statham-arm left a comment

Uh oh!

hstk30-hw left a comment

Uh oh!

Uh oh!

Uh oh!

[ARM] Introduce -mtp=auto and make it the default #128901

[ARM] Introduce -mtp=auto and make it the default #128901

Uh oh!

Conversation

Zhenhang1213 commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 26, 2025

Uh oh!

github-actions bot commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

statham-arm left a comment

Choose a reason for hiding this comment

Uh oh!

hstk30-hw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Zhenhang1213 commented Feb 26, 2025 •

edited

Loading

llvmbot commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading