[ARM] mtp = auto using hard point while arch supports thumb2 and hardtp

[Zhenhang1213]

mtp = auto using hard point while arch supports thumb2 and hardtp

reference:https://reviews.llvm.org/D114116

[llvmbot]

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-arm

Author: Austin (Zhenhang1213)

Changes

We follow GCC mtp=auto when arch is arm_arch6k and support thumb2

reference:https://reviews.llvm.org/D114116

---

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

3 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+10-3)
• (modified) clang/lib/Driver/ToolChains/Arch/ARM.h (+1)
• (modified) clang/test/Driver/arm-thread-pointer.c (+16)

diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 51454de1b9dcc..b7d5b3a9a882a 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -208,10 +208,17 @@ bool arm::useAAPCSForMachO(const llvm::Triple &T) {
 bool arm::isHardTPSupported(const llvm::Triple &Triple) {
   int Ver = getARMSubArchVersionNumber(Triple);
   llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName());
-  return Triple.isARM() || AK == llvm::ARM::ArchKind::ARMV6T2 ||
-         (Ver >= 7 && AK != llvm::ARM::ArchKind::ARMV8MBaseline);
+  return Triple.isARM() || (Ver >= 7 && AK != llvm::ARM::ArchKind::ARMV8MBaseline);
 }
 
+
+// We follow GCC mtp=auto when arch is arm_arch6k and support thumb2
+bool arm::isHardTPAndThumb2(const llvm::Triple &Triple) {
+  llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName());
+  return Triple.isARM() && AK >= llvm::ARM::ArchKind::ARMV6K && AK != llvm::ARM::ArchKind::ARMV8MBaseline;
+}
+
+
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver &D, const ArgList &Args,
                                    const llvm::Triple &Triple, bool ForAS) {
@@ -240,7 +247,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 (isHardTPSupported(Triple) ? ReadTPMode::TPIDRURO : ReadTPMode::Soft);
+  return (isHardTPAndThumb2(Triple) ? ReadTPMode::TPIDRURO : ReadTPMode::Soft);
 }
 
 void arm::setArchNameInTriple(const Driver &D, const ArgList &Args,
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.h b/clang/lib/Driver/ToolChains/Arch/ARM.h
index a23a8793a89e2..0641580900fd1 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -57,6 +57,7 @@ FloatABI getARMFloatABI(const Driver &D, const llvm::Triple &Triple,
 void setFloatABIInTriple(const Driver &D, const llvm::opt::ArgList &Args,
                          llvm::Triple &triple);
 bool isHardTPSupported(const llvm::Triple &Triple);
+bool isHardTPAndThumb2(const llvm::Triple &Triple);
 ReadTPMode getReadTPMode(const Driver &D, const llvm::opt::ArgList &Args,
                          const llvm::Triple &Triple, bool ForAS);
 void setArchNameInTriple(const Driver &D, const llvm::opt::ArgList &Args,
diff --git a/clang/test/Driver/arm-thread-pointer.c b/clang/test/Driver/arm-thread-pointer.c
index 985c5046f6d26..b487a8d83fe34 100644
--- a/clang/test/Driver/arm-thread-pointer.c
+++ b/clang/test/Driver/arm-thread-pointer.c
@@ -47,3 +47,19 @@
 // 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"
+
+// RUN: %clang --target=armv5t-linux -mtp=auto -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5t_THREAD_POINTER_Auto %s
+// ARMv5t_THREAD_POINTER_Auto-NOT: "-target-feature" "+read-tp-tpidruro"
+
+// RUN: %clang --target=armv6k-linux -mtp=auto -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6k_THREAD_POINTER_Auto %s
+// ARMv6k_THREAD_POINTER_Auto: "-target-feature" "+read-tp-tpidruro"
+
+// RUN: %clang --target=armv6-linux -mtp=auto -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6_THREAD_POINTER_Auto %s
+// ARMv6_THREAD_POINTER_Auto-NOT: "-target-feature" "+read-tp-tpidruro"
+
+// RUN: %clang --target=armv6kz-linux -mtp=auto -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6kz_THREAD_POINTER_Auto %s
+// ARMv6kz_THREAD_POINTER_Auto: "-target-feature" "+read-tp-tpidruro"

[Zhenhang1213]

@smithp35 @statham-arm

[statham-arm]

I'm sorry to complain about the commit message again, but could you please expand this to say what you're actually doing?

We follow GCC mtp=auto when arch is arm_arch6k and support thumb2

This message by itself doesn't say what the change does when arch is arm_arch6k. Have you made it start using TPIDRURO, or made it stop? All you're saying is that you do the same as GCC, which doesn't help a reader who doesn't already know what GCC does.

[VladiKrapp-Arm]

The updated version looks better.

I think we need to remove ARMV6T2 from the list, as it does not seem to support the feature.
https://developer.arm.com/documentation/ddi0290/g/system-control-coprocessor/system-control-processor-registers/c13--process-id-register?lang=en
This describes different functionality for mrc p15 c13 than that of ARMV6K, ARMV6KZ and later A/R architectures, for example
https://developer.arm.com/documentation/ddi0333/h/system-control-coprocessor/system-control-processor-registers/c13--thread-and-process-id-registers?lang=en

It seems the current version will select tpidruro mode for -mtp=auto for any architecture that supports it.
Note that this does not seem to be the gcc behaviour, as gcc will only set it for armv6k automatically, and for nothing else.
https://github.com/gcc-mirror/gcc/blob/fdf846fdddcc0467b9f025757f081c5d54319d08/gcc/config/arm/arm.cc#L3972-L3978
This seems a defect in gcc, rather than something to emulate.

[VladiKrapp-Arm]

@Zhenhang1213 Might I suggest changes along the lines of

VladiKrapp-Arm@63e4f10

This separates the check for whether the platform allows hardware tls from whether it is possible to encode using thumb2. Both these checks should be made for auto mode, so we don't automatically select this mode for the user when they might have some functions compiled with soft mode.
@smithp35 : This still isn't exactly the same as what gcc does, because there auto mode is enabled if it's armv6k and arm mode. This check could be done here as well, but could still cause problems if user compiles as in your example with attribute((target("thumb"))).

[VladiKrapp-Arm]

@Zhenhang1213 :
Code looks ok.
Are all edge cases covered by the lit test now?

Could you please adjust the commit message to reflect the actual changes?

[Zhenhang1213]

@Zhenhang1213 : Code looks ok. Are all edge cases covered by the lit test now?

Could you please adjust the commit message to reflect the actual changes?

OK， I add more tests to cover, thx!

[VladiKrapp-Arm]

LGTM

[statham-arm]

Thanks for the extra test. LGTM now.