Skip to content

[ARM] __ARM_ARCH macro definition fix #81493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

jwestwood921
Copy link
Contributor

This patch changes how the macro __ARM_ARCH is defined to match its defintion in the ACLE. In ACLE 5.4.1, __ARM_ARCH is defined as equal to the major architecture version for ISAs up to and including v8. From v8.1 onwards, its definition is changed to include minor versions, such that for an architecture vX.Y, __ARM_ARCH = X*100 + Y. Before this patch, LLVM defined __ARM_ARCH using only the major architecture version for all architecture versions. This patch adds functionality to define __ARM_ARCH correctly for architectures greater than or equal to v8.1.

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:ARM backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-backend-arm

Author: James Westwood (jwestwood921)

Changes

This patch changes how the macro __ARM_ARCH is defined to match its defintion in the ACLE. In ACLE 5.4.1, __ARM_ARCH is defined as equal to the major architecture version for ISAs up to and including v8. From v8.1 onwards, its definition is changed to include minor versions, such that for an architecture vX.Y, __ARM_ARCH = X*100 + Y. Before this patch, LLVM defined __ARM_ARCH using only the major architecture version for all architecture versions. This patch adds functionality to define __ARM_ARCH correctly for architectures greater than or equal to v8.1.


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

7 Files Affected:

  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+15-2)
  • (modified) clang/lib/Basic/Targets/ARM.cpp (+12-3)
  • (modified) clang/lib/Basic/Targets/ARM.h (+1)
  • (modified) clang/test/Preprocessor/arm-target-features.c (+17-17)
  • (modified) llvm/include/llvm/TargetParser/ARMTargetParser.h (+1)
  • (modified) llvm/lib/TargetParser/ARMTargetParser.cpp (+58)
  • (modified) llvm/unittests/TargetParser/TargetParserTest.cpp (+8)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 68032961451d90..3ef283f824006c 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -367,8 +367,21 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
 
   // ACLE predefines. Many can only have one possible value on v8 AArch64.
   Builder.defineMacro("__ARM_ACLE", "200");
-  Builder.defineMacro("__ARM_ARCH",
-                      std::to_string(ArchInfo->Version.getMajor()));
+
+  // ACLE 5.4.1 ARM/Thumb instruction set architecture
+  // __ARM_ARCH is defined as an integer value indicating the current ARM ISA.
+  // For ISAs up to and including v8, __ARM_ARCH is equal to the major version
+  // number. For ISAs from v8.1 onwards, __ARM_ARCH is scaled up to include the
+  // minor version number, e.g. for ARM architecture ARMvX.Y:
+  // __ARM_ARCH = X * 100 + Y.
+  if (ArchInfo->Version.getMajor() == 8 && ArchInfo->Version.getMinor() == 0)
+    Builder.defineMacro("__ARM_ARCH",
+                        std::to_string(ArchInfo->Version.getMajor()));
+  else
+    Builder.defineMacro("__ARM_ARCH",
+                        std::to_string(ArchInfo->Version.getMajor() * 100 +
+                                       ArchInfo->Version.getMinor().value()));
+
   Builder.defineMacro("__ARM_ARCH_PROFILE",
                       std::string("'") + (char)ArchInfo->Profile + "'");
 
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 55b71557452fa0..9ac558caa2886a 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -130,6 +130,7 @@ void ARMTargetInfo::setArchInfo(llvm::ARM::ArchKind Kind) {
   SubArch = llvm::ARM::getSubArch(ArchKind);
   ArchProfile = llvm::ARM::parseArchProfile(SubArch);
   ArchVersion = llvm::ARM::parseArchVersion(SubArch);
+  ArchMinorVersion = llvm::ARM::parseArchMinorVersion(SubArch);
 
   // cache CPU related strings
   CPUAttr = getCPUAttr();
@@ -736,9 +737,17 @@ void ARMTargetInfo::getTargetDefines(const LangOptions &Opts,
   if (!CPUAttr.empty())
     Builder.defineMacro("__ARM_ARCH_" + CPUAttr + "__");
 
-  // ACLE 6.4.1 ARM/Thumb instruction set architecture
-  // __ARM_ARCH is defined as an integer value indicating the current ARM ISA
-  Builder.defineMacro("__ARM_ARCH", Twine(ArchVersion));
+  // ACLE 5.4.1 ARM/Thumb instruction set architecture
+  // __ARM_ARCH is defined as an integer value indicating the current ARM ISA.
+  // For ISAs up to and including v8, __ARM_ARCH is equal to the major version
+  // number. For ISAs from v8.1 onwards, __ARM_ARCH is scaled up to include the
+  // minor version number, e.g. for ARM architecture ARMvX.Y:
+  // __ARM_ARCH = X * 100 + Y.
+  if (ArchVersion >= 9 || ArchMinorVersion != 0)
+    Builder.defineMacro("__ARM_ARCH",
+                        Twine(ArchVersion * 100 + ArchMinorVersion));
+  else
+    Builder.defineMacro("__ARM_ARCH", Twine(ArchVersion));
 
   if (ArchVersion >= 8) {
     // ACLE 6.5.7 Crypto Extension
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 71322a094f5edb..df06e4d120637a 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -60,6 +60,7 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
   llvm::ARM::ArchKind ArchKind = llvm::ARM::ArchKind::ARMV4T;
   llvm::ARM::ProfileKind ArchProfile;
   unsigned ArchVersion;
+  unsigned ArchMinorVersion;
 
   LLVM_PREFERRED_TYPE(FPUMode)
   unsigned FPU : 5;
diff --git a/clang/test/Preprocessor/arm-target-features.c b/clang/test/Preprocessor/arm-target-features.c
index 236c9f2479b705..733d068b09b1fe 100644
--- a/clang/test/Preprocessor/arm-target-features.c
+++ b/clang/test/Preprocessor/arm-target-features.c
@@ -737,7 +737,7 @@
 
 // Test whether predefines are as expected when targeting cortex-m55 (softfp FP ABI as default).
 // RUN: %clang -target arm-eabi -mcpu=cortex-m55 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=M55 %s
-// M55: #define __ARM_ARCH 8
+// M55: #define __ARM_ARCH 801
 // M55: #define __ARM_ARCH_8_1M_MAIN__ 1
 // M55: #define __ARM_ARCH_EXT_IDIV__ 1
 // M55-NOT: __ARM_ARCH_ISA_ARM
@@ -764,7 +764,7 @@
 // KRAIT-ALLOW-FP-INSTR:#define  __ARM_VFPV4__ 1
 
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M %s
-// CHECK-V81M: #define __ARM_ARCH 8
+// CHECK-V81M: #define __ARM_ARCH 801
 // CHECK-V81M: #define __ARM_ARCH_8_1M_MAIN__ 1
 // CHECK-V81M: #define __ARM_ARCH_ISA_THUMB 2
 // CHECK-V81M: #define __ARM_ARCH_PROFILE 'M'
@@ -821,14 +821,14 @@
 // CHECK-V8M-CDE-MASK2: #define __ARM_FEATURE_CDE_COPROC 0xff
 
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
-// CHECK-V81A: #define __ARM_ARCH 8
+// CHECK-V81A: #define __ARM_ARCH 801
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
 // CHECK-V81A: #define __ARM_ARCH_PROFILE 'A'
 // CHECK-V81A: #define __ARM_FEATURE_QRDMX 1
 // CHECK-V81A: #define __ARM_FP 0xe
 
 // RUN: %clang -target armv8.2a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V82A %s
-// CHECK-V82A: #define __ARM_ARCH 8
+// CHECK-V82A: #define __ARM_ARCH 802
 // CHECK-V82A: #define __ARM_ARCH_8_2A__ 1
 // CHECK-V82A: #define __ARM_ARCH_PROFILE 'A'
 // CHECK-V82A: #define __ARM_FEATURE_QRDMX 1
@@ -838,67 +838,67 @@
 // CHECK-DRIVERKIT-NOT: #define __ARM_PCS_VFP 1
 
 // RUN: %clang -target armv8.3a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V83A %s
-// CHECK-V83A: #define __ARM_ARCH 8
+// CHECK-V83A: #define __ARM_ARCH 803
 // CHECK-V83A: #define __ARM_ARCH_8_3A__ 1
 // CHECK-V83A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.4a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V84A %s
-// CHECK-V84A: #define __ARM_ARCH 8
+// CHECK-V84A: #define __ARM_ARCH 804
 // CHECK-V84A: #define __ARM_ARCH_8_4A__ 1
 // CHECK-V84A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.5a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V85A %s
-// CHECK-V85A: #define __ARM_ARCH 8
+// CHECK-V85A: #define __ARM_ARCH 805
 // CHECK-V85A: #define __ARM_ARCH_8_5A__ 1
 // CHECK-V85A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.6a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V86A %s
-// CHECK-V86A: #define __ARM_ARCH 8
+// CHECK-V86A: #define __ARM_ARCH 806
 // CHECK-V86A: #define __ARM_ARCH_8_6A__ 1
 // CHECK-V86A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.7a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V87A %s
-// CHECK-V87A: #define __ARM_ARCH 8
+// CHECK-V87A: #define __ARM_ARCH 807
 // CHECK-V87A: #define __ARM_ARCH_8_7A__ 1
 // CHECK-V87A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.8a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V88A %s
-// CHECK-V88A: #define __ARM_ARCH 8
+// CHECK-V88A: #define __ARM_ARCH 808
 // CHECK-V88A: #define __ARM_ARCH_8_8A__ 1
 // CHECK-V88A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.9a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V89A %s
-// CHECK-V89A: #define __ARM_ARCH 8
+// CHECK-V89A: #define __ARM_ARCH 809
 // CHECK-V89A: #define __ARM_ARCH_8_9A__ 1
 // CHECK-V89A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V9A %s
-// CHECK-V9A: #define __ARM_ARCH 9
+// CHECK-V9A: #define __ARM_ARCH 900
 // CHECK-V9A: #define __ARM_ARCH_9A__ 1
 // CHECK-V9A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V91A %s
-// CHECK-V91A: #define __ARM_ARCH 9
+// CHECK-V91A: #define __ARM_ARCH 901
 // CHECK-V91A: #define __ARM_ARCH_9_1A__ 1
 // CHECK-V91A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.2a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V92A %s
-// CHECK-V92A: #define __ARM_ARCH 9
+// CHECK-V92A: #define __ARM_ARCH 902
 // CHECK-V92A: #define __ARM_ARCH_9_2A__ 1
 // CHECK-V92A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.3a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V93A %s
-// CHECK-V93A: #define __ARM_ARCH 9
+// CHECK-V93A: #define __ARM_ARCH 903
 // CHECK-V93A: #define __ARM_ARCH_9_3A__ 1
 // CHECK-V93A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.4a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V94A %s
-// CHECK-V94A: #define __ARM_ARCH 9
+// CHECK-V94A: #define __ARM_ARCH 904
 // CHECK-V94A: #define __ARM_ARCH_9_4A__ 1
 // CHECK-V94A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.5a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V95A %s
-// CHECK-V95A: #define __ARM_ARCH 9
+// CHECK-V95A: #define __ARM_ARCH 905
 // CHECK-V95A: #define __ARM_ARCH_9_5A__ 1
 // CHECK-V95A: #define __ARM_ARCH_PROFILE 'A'
 
diff --git a/llvm/include/llvm/TargetParser/ARMTargetParser.h b/llvm/include/llvm/TargetParser/ARMTargetParser.h
index c42d66f048fccc..ec3817134a5ac0 100644
--- a/llvm/include/llvm/TargetParser/ARMTargetParser.h
+++ b/llvm/include/llvm/TargetParser/ARMTargetParser.h
@@ -258,6 +258,7 @@ uint64_t parseArchExt(StringRef ArchExt);
 ArchKind parseCPUArch(StringRef CPU);
 ProfileKind parseArchProfile(StringRef Arch);
 unsigned parseArchVersion(StringRef Arch);
+unsigned parseArchMinorVersion(StringRef Arch);
 
 void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values);
 StringRef computeDefaultTargetABI(const Triple &TT, StringRef CPU);
diff --git a/llvm/lib/TargetParser/ARMTargetParser.cpp b/llvm/lib/TargetParser/ARMTargetParser.cpp
index 67f937ebc33f9f..fac701946e282c 100644
--- a/llvm/lib/TargetParser/ARMTargetParser.cpp
+++ b/llvm/lib/TargetParser/ARMTargetParser.cpp
@@ -94,6 +94,64 @@ unsigned ARM::parseArchVersion(StringRef Arch) {
   llvm_unreachable("Unhandled architecture");
 }
 
+unsigned ARM::parseArchMinorVersion(StringRef Arch) {
+  Arch = getCanonicalArchName(Arch);
+  switch (parseArch(Arch)) {
+  case ArchKind::ARMV4:
+  case ArchKind::ARMV4T:
+  case ArchKind::ARMV5T:
+  case ArchKind::ARMV5TE:
+  case ArchKind::IWMMXT:
+  case ArchKind::IWMMXT2:
+  case ArchKind::XSCALE:
+  case ArchKind::ARMV5TEJ:
+  case ArchKind::ARMV6:
+  case ArchKind::ARMV6K:
+  case ArchKind::ARMV6T2:
+  case ArchKind::ARMV6KZ:
+  case ArchKind::ARMV6M:
+  case ArchKind::ARMV7A:
+  case ArchKind::ARMV7VE:
+  case ArchKind::ARMV7R:
+  case ArchKind::ARMV7M:
+  case ArchKind::ARMV7S:
+  case ArchKind::ARMV7EM:
+  case ArchKind::ARMV7K:
+  case ArchKind::ARMV8A:
+  case ArchKind::ARMV8R:
+  case ArchKind::ARMV8MBaseline:
+  case ArchKind::ARMV8MMainline:
+  case ArchKind::ARMV9A:
+  case ArchKind::INVALID:
+    return 0;
+  case ArchKind::ARMV8_1A:
+  case ArchKind::ARMV8_1MMainline:
+  case ArchKind::ARMV9_1A:
+    return 1;
+  case ArchKind::ARMV8_2A:
+  case ArchKind::ARMV9_2A:
+    return 2;
+  case ArchKind::ARMV8_3A:
+  case ArchKind::ARMV9_3A:
+    return 3;
+  case ArchKind::ARMV8_4A:
+  case ArchKind::ARMV9_4A:
+    return 4;
+  case ArchKind::ARMV8_5A:
+  case ArchKind::ARMV9_5A:
+    return 5;
+  case ArchKind::ARMV8_6A:
+    return 6;
+  case ArchKind::ARMV8_7A:
+    return 7;
+  case ArchKind::ARMV8_8A:
+    return 8;
+  case ArchKind::ARMV8_9A:
+    return 9;
+  }
+  llvm_unreachable("Unhandled architecture");
+}
+
 static ARM::ProfileKind getProfileKind(ARM::ArchKind AK) {
   switch (AK) {
   case ARM::ArchKind::ARMV6M:
diff --git a/llvm/unittests/TargetParser/TargetParserTest.cpp b/llvm/unittests/TargetParser/TargetParserTest.cpp
index e89fc687451cd7..c6ee39fa416021 100644
--- a/llvm/unittests/TargetParser/TargetParserTest.cpp
+++ b/llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -976,6 +976,14 @@ TEST(TargetParserTest, ARMparseArchVersion) {
       EXPECT_EQ(5u, ARM::parseArchVersion(ARMArch[i]));
 }
 
+TEST(TargetParserTest, ARMparseArchMinorVersion) {
+  for (unsigned i = 0; i < std::size(ARMArch); i++)
+    if (((std::string)ARMArch[i]).find(".") == 5)
+      EXPECT_EQ((ARMArch[i][6] - 48u), ARM::parseArchMinorVersion(ARMArch[i]));
+    else
+      EXPECT_EQ(0u, ARM::parseArchMinorVersion(ARMArch[i]));
+}
+
 TEST(TargetParserTest, getARMCPUForArch) {
   // Platform specific defaults.
   {

@llvmbot
Copy link
Member

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-clang

Author: James Westwood (jwestwood921)

Changes

This patch changes how the macro __ARM_ARCH is defined to match its defintion in the ACLE. In ACLE 5.4.1, __ARM_ARCH is defined as equal to the major architecture version for ISAs up to and including v8. From v8.1 onwards, its definition is changed to include minor versions, such that for an architecture vX.Y, __ARM_ARCH = X*100 + Y. Before this patch, LLVM defined __ARM_ARCH using only the major architecture version for all architecture versions. This patch adds functionality to define __ARM_ARCH correctly for architectures greater than or equal to v8.1.


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

7 Files Affected:

  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+15-2)
  • (modified) clang/lib/Basic/Targets/ARM.cpp (+12-3)
  • (modified) clang/lib/Basic/Targets/ARM.h (+1)
  • (modified) clang/test/Preprocessor/arm-target-features.c (+17-17)
  • (modified) llvm/include/llvm/TargetParser/ARMTargetParser.h (+1)
  • (modified) llvm/lib/TargetParser/ARMTargetParser.cpp (+58)
  • (modified) llvm/unittests/TargetParser/TargetParserTest.cpp (+8)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 68032961451d90..3ef283f824006c 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -367,8 +367,21 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
 
   // ACLE predefines. Many can only have one possible value on v8 AArch64.
   Builder.defineMacro("__ARM_ACLE", "200");
-  Builder.defineMacro("__ARM_ARCH",
-                      std::to_string(ArchInfo->Version.getMajor()));
+
+  // ACLE 5.4.1 ARM/Thumb instruction set architecture
+  // __ARM_ARCH is defined as an integer value indicating the current ARM ISA.
+  // For ISAs up to and including v8, __ARM_ARCH is equal to the major version
+  // number. For ISAs from v8.1 onwards, __ARM_ARCH is scaled up to include the
+  // minor version number, e.g. for ARM architecture ARMvX.Y:
+  // __ARM_ARCH = X * 100 + Y.
+  if (ArchInfo->Version.getMajor() == 8 && ArchInfo->Version.getMinor() == 0)
+    Builder.defineMacro("__ARM_ARCH",
+                        std::to_string(ArchInfo->Version.getMajor()));
+  else
+    Builder.defineMacro("__ARM_ARCH",
+                        std::to_string(ArchInfo->Version.getMajor() * 100 +
+                                       ArchInfo->Version.getMinor().value()));
+
   Builder.defineMacro("__ARM_ARCH_PROFILE",
                       std::string("'") + (char)ArchInfo->Profile + "'");
 
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 55b71557452fa0..9ac558caa2886a 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -130,6 +130,7 @@ void ARMTargetInfo::setArchInfo(llvm::ARM::ArchKind Kind) {
   SubArch = llvm::ARM::getSubArch(ArchKind);
   ArchProfile = llvm::ARM::parseArchProfile(SubArch);
   ArchVersion = llvm::ARM::parseArchVersion(SubArch);
+  ArchMinorVersion = llvm::ARM::parseArchMinorVersion(SubArch);
 
   // cache CPU related strings
   CPUAttr = getCPUAttr();
@@ -736,9 +737,17 @@ void ARMTargetInfo::getTargetDefines(const LangOptions &Opts,
   if (!CPUAttr.empty())
     Builder.defineMacro("__ARM_ARCH_" + CPUAttr + "__");
 
-  // ACLE 6.4.1 ARM/Thumb instruction set architecture
-  // __ARM_ARCH is defined as an integer value indicating the current ARM ISA
-  Builder.defineMacro("__ARM_ARCH", Twine(ArchVersion));
+  // ACLE 5.4.1 ARM/Thumb instruction set architecture
+  // __ARM_ARCH is defined as an integer value indicating the current ARM ISA.
+  // For ISAs up to and including v8, __ARM_ARCH is equal to the major version
+  // number. For ISAs from v8.1 onwards, __ARM_ARCH is scaled up to include the
+  // minor version number, e.g. for ARM architecture ARMvX.Y:
+  // __ARM_ARCH = X * 100 + Y.
+  if (ArchVersion >= 9 || ArchMinorVersion != 0)
+    Builder.defineMacro("__ARM_ARCH",
+                        Twine(ArchVersion * 100 + ArchMinorVersion));
+  else
+    Builder.defineMacro("__ARM_ARCH", Twine(ArchVersion));
 
   if (ArchVersion >= 8) {
     // ACLE 6.5.7 Crypto Extension
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 71322a094f5edb..df06e4d120637a 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -60,6 +60,7 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
   llvm::ARM::ArchKind ArchKind = llvm::ARM::ArchKind::ARMV4T;
   llvm::ARM::ProfileKind ArchProfile;
   unsigned ArchVersion;
+  unsigned ArchMinorVersion;
 
   LLVM_PREFERRED_TYPE(FPUMode)
   unsigned FPU : 5;
diff --git a/clang/test/Preprocessor/arm-target-features.c b/clang/test/Preprocessor/arm-target-features.c
index 236c9f2479b705..733d068b09b1fe 100644
--- a/clang/test/Preprocessor/arm-target-features.c
+++ b/clang/test/Preprocessor/arm-target-features.c
@@ -737,7 +737,7 @@
 
 // Test whether predefines are as expected when targeting cortex-m55 (softfp FP ABI as default).
 // RUN: %clang -target arm-eabi -mcpu=cortex-m55 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=M55 %s
-// M55: #define __ARM_ARCH 8
+// M55: #define __ARM_ARCH 801
 // M55: #define __ARM_ARCH_8_1M_MAIN__ 1
 // M55: #define __ARM_ARCH_EXT_IDIV__ 1
 // M55-NOT: __ARM_ARCH_ISA_ARM
@@ -764,7 +764,7 @@
 // KRAIT-ALLOW-FP-INSTR:#define  __ARM_VFPV4__ 1
 
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M %s
-// CHECK-V81M: #define __ARM_ARCH 8
+// CHECK-V81M: #define __ARM_ARCH 801
 // CHECK-V81M: #define __ARM_ARCH_8_1M_MAIN__ 1
 // CHECK-V81M: #define __ARM_ARCH_ISA_THUMB 2
 // CHECK-V81M: #define __ARM_ARCH_PROFILE 'M'
@@ -821,14 +821,14 @@
 // CHECK-V8M-CDE-MASK2: #define __ARM_FEATURE_CDE_COPROC 0xff
 
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
-// CHECK-V81A: #define __ARM_ARCH 8
+// CHECK-V81A: #define __ARM_ARCH 801
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
 // CHECK-V81A: #define __ARM_ARCH_PROFILE 'A'
 // CHECK-V81A: #define __ARM_FEATURE_QRDMX 1
 // CHECK-V81A: #define __ARM_FP 0xe
 
 // RUN: %clang -target armv8.2a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V82A %s
-// CHECK-V82A: #define __ARM_ARCH 8
+// CHECK-V82A: #define __ARM_ARCH 802
 // CHECK-V82A: #define __ARM_ARCH_8_2A__ 1
 // CHECK-V82A: #define __ARM_ARCH_PROFILE 'A'
 // CHECK-V82A: #define __ARM_FEATURE_QRDMX 1
@@ -838,67 +838,67 @@
 // CHECK-DRIVERKIT-NOT: #define __ARM_PCS_VFP 1
 
 // RUN: %clang -target armv8.3a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V83A %s
-// CHECK-V83A: #define __ARM_ARCH 8
+// CHECK-V83A: #define __ARM_ARCH 803
 // CHECK-V83A: #define __ARM_ARCH_8_3A__ 1
 // CHECK-V83A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.4a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V84A %s
-// CHECK-V84A: #define __ARM_ARCH 8
+// CHECK-V84A: #define __ARM_ARCH 804
 // CHECK-V84A: #define __ARM_ARCH_8_4A__ 1
 // CHECK-V84A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.5a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V85A %s
-// CHECK-V85A: #define __ARM_ARCH 8
+// CHECK-V85A: #define __ARM_ARCH 805
 // CHECK-V85A: #define __ARM_ARCH_8_5A__ 1
 // CHECK-V85A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.6a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V86A %s
-// CHECK-V86A: #define __ARM_ARCH 8
+// CHECK-V86A: #define __ARM_ARCH 806
 // CHECK-V86A: #define __ARM_ARCH_8_6A__ 1
 // CHECK-V86A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.7a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V87A %s
-// CHECK-V87A: #define __ARM_ARCH 8
+// CHECK-V87A: #define __ARM_ARCH 807
 // CHECK-V87A: #define __ARM_ARCH_8_7A__ 1
 // CHECK-V87A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.8a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V88A %s
-// CHECK-V88A: #define __ARM_ARCH 8
+// CHECK-V88A: #define __ARM_ARCH 808
 // CHECK-V88A: #define __ARM_ARCH_8_8A__ 1
 // CHECK-V88A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv8.9a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V89A %s
-// CHECK-V89A: #define __ARM_ARCH 8
+// CHECK-V89A: #define __ARM_ARCH 809
 // CHECK-V89A: #define __ARM_ARCH_8_9A__ 1
 // CHECK-V89A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V9A %s
-// CHECK-V9A: #define __ARM_ARCH 9
+// CHECK-V9A: #define __ARM_ARCH 900
 // CHECK-V9A: #define __ARM_ARCH_9A__ 1
 // CHECK-V9A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V91A %s
-// CHECK-V91A: #define __ARM_ARCH 9
+// CHECK-V91A: #define __ARM_ARCH 901
 // CHECK-V91A: #define __ARM_ARCH_9_1A__ 1
 // CHECK-V91A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.2a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V92A %s
-// CHECK-V92A: #define __ARM_ARCH 9
+// CHECK-V92A: #define __ARM_ARCH 902
 // CHECK-V92A: #define __ARM_ARCH_9_2A__ 1
 // CHECK-V92A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.3a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V93A %s
-// CHECK-V93A: #define __ARM_ARCH 9
+// CHECK-V93A: #define __ARM_ARCH 903
 // CHECK-V93A: #define __ARM_ARCH_9_3A__ 1
 // CHECK-V93A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.4a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V94A %s
-// CHECK-V94A: #define __ARM_ARCH 9
+// CHECK-V94A: #define __ARM_ARCH 904
 // CHECK-V94A: #define __ARM_ARCH_9_4A__ 1
 // CHECK-V94A: #define __ARM_ARCH_PROFILE 'A'
 
 // RUN: %clang -target armv9.5a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V95A %s
-// CHECK-V95A: #define __ARM_ARCH 9
+// CHECK-V95A: #define __ARM_ARCH 905
 // CHECK-V95A: #define __ARM_ARCH_9_5A__ 1
 // CHECK-V95A: #define __ARM_ARCH_PROFILE 'A'
 
diff --git a/llvm/include/llvm/TargetParser/ARMTargetParser.h b/llvm/include/llvm/TargetParser/ARMTargetParser.h
index c42d66f048fccc..ec3817134a5ac0 100644
--- a/llvm/include/llvm/TargetParser/ARMTargetParser.h
+++ b/llvm/include/llvm/TargetParser/ARMTargetParser.h
@@ -258,6 +258,7 @@ uint64_t parseArchExt(StringRef ArchExt);
 ArchKind parseCPUArch(StringRef CPU);
 ProfileKind parseArchProfile(StringRef Arch);
 unsigned parseArchVersion(StringRef Arch);
+unsigned parseArchMinorVersion(StringRef Arch);
 
 void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values);
 StringRef computeDefaultTargetABI(const Triple &TT, StringRef CPU);
diff --git a/llvm/lib/TargetParser/ARMTargetParser.cpp b/llvm/lib/TargetParser/ARMTargetParser.cpp
index 67f937ebc33f9f..fac701946e282c 100644
--- a/llvm/lib/TargetParser/ARMTargetParser.cpp
+++ b/llvm/lib/TargetParser/ARMTargetParser.cpp
@@ -94,6 +94,64 @@ unsigned ARM::parseArchVersion(StringRef Arch) {
   llvm_unreachable("Unhandled architecture");
 }
 
+unsigned ARM::parseArchMinorVersion(StringRef Arch) {
+  Arch = getCanonicalArchName(Arch);
+  switch (parseArch(Arch)) {
+  case ArchKind::ARMV4:
+  case ArchKind::ARMV4T:
+  case ArchKind::ARMV5T:
+  case ArchKind::ARMV5TE:
+  case ArchKind::IWMMXT:
+  case ArchKind::IWMMXT2:
+  case ArchKind::XSCALE:
+  case ArchKind::ARMV5TEJ:
+  case ArchKind::ARMV6:
+  case ArchKind::ARMV6K:
+  case ArchKind::ARMV6T2:
+  case ArchKind::ARMV6KZ:
+  case ArchKind::ARMV6M:
+  case ArchKind::ARMV7A:
+  case ArchKind::ARMV7VE:
+  case ArchKind::ARMV7R:
+  case ArchKind::ARMV7M:
+  case ArchKind::ARMV7S:
+  case ArchKind::ARMV7EM:
+  case ArchKind::ARMV7K:
+  case ArchKind::ARMV8A:
+  case ArchKind::ARMV8R:
+  case ArchKind::ARMV8MBaseline:
+  case ArchKind::ARMV8MMainline:
+  case ArchKind::ARMV9A:
+  case ArchKind::INVALID:
+    return 0;
+  case ArchKind::ARMV8_1A:
+  case ArchKind::ARMV8_1MMainline:
+  case ArchKind::ARMV9_1A:
+    return 1;
+  case ArchKind::ARMV8_2A:
+  case ArchKind::ARMV9_2A:
+    return 2;
+  case ArchKind::ARMV8_3A:
+  case ArchKind::ARMV9_3A:
+    return 3;
+  case ArchKind::ARMV8_4A:
+  case ArchKind::ARMV9_4A:
+    return 4;
+  case ArchKind::ARMV8_5A:
+  case ArchKind::ARMV9_5A:
+    return 5;
+  case ArchKind::ARMV8_6A:
+    return 6;
+  case ArchKind::ARMV8_7A:
+    return 7;
+  case ArchKind::ARMV8_8A:
+    return 8;
+  case ArchKind::ARMV8_9A:
+    return 9;
+  }
+  llvm_unreachable("Unhandled architecture");
+}
+
 static ARM::ProfileKind getProfileKind(ARM::ArchKind AK) {
   switch (AK) {
   case ARM::ArchKind::ARMV6M:
diff --git a/llvm/unittests/TargetParser/TargetParserTest.cpp b/llvm/unittests/TargetParser/TargetParserTest.cpp
index e89fc687451cd7..c6ee39fa416021 100644
--- a/llvm/unittests/TargetParser/TargetParserTest.cpp
+++ b/llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -976,6 +976,14 @@ TEST(TargetParserTest, ARMparseArchVersion) {
       EXPECT_EQ(5u, ARM::parseArchVersion(ARMArch[i]));
 }
 
+TEST(TargetParserTest, ARMparseArchMinorVersion) {
+  for (unsigned i = 0; i < std::size(ARMArch); i++)
+    if (((std::string)ARMArch[i]).find(".") == 5)
+      EXPECT_EQ((ARMArch[i][6] - 48u), ARM::parseArchMinorVersion(ARMArch[i]));
+    else
+      EXPECT_EQ(0u, ARM::parseArchMinorVersion(ARMArch[i]));
+}
+
 TEST(TargetParserTest, getARMCPUForArch) {
   // Platform specific defaults.
   {

@jwestwood921 jwestwood921 changed the title __ARM_ARCH macro definition fix [ARM] __ARM_ARCH macro definition fix Feb 12, 2024
@pinskia
Copy link

pinskia commented Feb 12, 2024

Note the corresponding GCC bug is at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99312 (and there was a patch posted but never reviewed; https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566322.html).

Copy link
Collaborator

@ostannard ostannard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some minor changes to the comments which don't need re-review.

Builder.defineMacro("__ARM_ARCH",
std::to_string(ArchInfo->Version.getMajor()));

// ACLE 5.4.1 ARM/Thumb instruction set architecture
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the ACLE doesn't use section numbers any more, and and the ARM/Thumb ISAs only apply to AArch32. I think this line of the comment can just be removed.

// ACLE 6.4.1 ARM/Thumb instruction set architecture
// __ARM_ARCH is defined as an integer value indicating the current ARM ISA
Builder.defineMacro("__ARM_ARCH", Twine(ArchVersion));
// ACLE 5.4.1 ARM/Thumb instruction set architecture
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro doesn't actually have anything to do with the ARM/Thumb ISAs, this line of the comment can just be deleted.

@davemgreen
Copy link
Collaborator

I'm a little worried people might be relying on the existing behaviour, with both clang and GCC having this wrong for a while. If we are going to do it can you add a release note to clang explaining the new behaviour?

This patch changes how the macro __ARM_ARCH is defined to match its defintion in the ACLE. In ACLE 5.4.1, __ARM_ARCH is defined as equal
to the major architecture version for ISAs up to and including v8. From v8.1 onwards, its definition is changed to include minor versions,
such that for an architecture vX.Y, __ARM_ARCH = X*100 + Y. Before this patch, LLVM defined __ARM_ARCH using only the major architecture
version for all architecture versions. This patch adds functionality to define __ARM_ARCH correctly for architectures greater than or
equal to v8.1.
@ostannard ostannard merged commit 89c1bf1 into llvm:main Feb 13, 2024
Copy link

@jwestwood921 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

tmatheson-arm added a commit that referenced this pull request Feb 19, 2024
This reverts commit 89c1bf1.

This has been unimplemenented for a while, and GCC does not implement
it, therefore we need to consider whether we should just deprecate it
in the ACLE instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 backend:ARM clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants