[clang] [clang][driver] Allow unaligned access on ARMv7 and higher by default (PR #82400)

Paul Kirth via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 21 09:07:24 PST 2024


https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/82400

>From 20634c2f54ae667ee8374d12e58e582aa6cdd051 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Tue, 20 Feb 2024 10:34:57 -0800
Subject: [PATCH 1/5] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 clang/lib/Driver/ToolChains/Arch/ARM.cpp | 14 ++++++--------
 clang/test/Driver/arm-alignment.c        |  8 ++++++++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index e6ee2f88a84edf..3bf6056f0c3ecc 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -895,20 +895,18 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
     // defaults this bit to 0 and handles it as a system-wide (not
     // per-process) setting. It is therefore safe to assume that ARMv7+
     // Linux targets support unaligned accesses. The same goes for NaCl
-    // and Windows.
-    //
-    // The above behavior is consistent with GCC.
+    // and Windows. However, ARM's forks of GCC and Clang both allow
+    // unaligned accesses by default for all targets. We follow this
+    // behavior and enable unaligned accesses by default for ARMv7+ targets.
+    // Users can disable behavior via compiler options (-mno-unaliged-access).
+    // See https://github.com/llvm/llvm-project/issues/59560 for more info.
     int VersionNum = getARMSubArchVersionNumber(Triple);
     if (Triple.isOSDarwin() || Triple.isOSNetBSD()) {
       if (VersionNum < 6 ||
           Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m)
         Features.push_back("+strict-align");
-    } else if (Triple.isOSLinux() || Triple.isOSNaCl() ||
-               Triple.isOSWindows()) {
-      if (VersionNum < 7)
+    } else if (VersionNum < 7)
         Features.push_back("+strict-align");
-    } else
-      Features.push_back("+strict-align");
   }
 
   // llvm does not support reserving registers in general. There is support
diff --git a/clang/test/Driver/arm-alignment.c b/clang/test/Driver/arm-alignment.c
index 9177b625729b85..6d0084451e82c7 100644
--- a/clang/test/Driver/arm-alignment.c
+++ b/clang/test/Driver/arm-alignment.c
@@ -22,6 +22,14 @@
 // RUN: %clang -target armv7-windows -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
+/// Ensure that by default before ARMv7 we default to +strict-align
+// RUN: %clang -target armv6 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+
+/// After ARMv7 by default we allow unaligned accesses for all targets
+// RUN: %clang -target armv7 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
+
 // RUN: %clang --target=aarch64 -munaligned-access -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-AARCH64 < %t %s
 

>From a078e658bdcd851d87331ecf87224dc2ddb13c69 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Tue, 20 Feb 2024 10:38:50 -0800
Subject: [PATCH 2/5] git clang-format

Created using spr 1.3.4
---
 clang/lib/Driver/ToolChains/Arch/ARM.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 3bf6056f0c3ecc..a1aea397925931 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -906,7 +906,7 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
           Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m)
         Features.push_back("+strict-align");
     } else if (VersionNum < 7)
-        Features.push_back("+strict-align");
+      Features.push_back("+strict-align");
   }
 
   // llvm does not support reserving registers in general. There is support

>From c21f9aa16bb4d081f41a108171e88ecac2bf2884 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Tue, 20 Feb 2024 12:05:48 -0800
Subject: [PATCH 3/5] Address comments, and update checking

Created using spr 1.3.4
---
 clang/docs/ReleaseNotes.rst              |  2 ++
 clang/lib/Driver/ToolChains/Arch/ARM.cpp | 26 ++++++++----------------
 clang/test/Driver/arm-alignment.c        | 12 +++++------
 3 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 762e8133f5d536..6a4182474b3345 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -304,6 +304,8 @@ X86 Support
 
 Arm and AArch64 Support
 ^^^^^^^^^^^^^^^^^^^^^^^
+- ARMv6+ targets now default to allowing unaligned access, except Armv6-M, and
+  Armv8-M without the Main Extension.
 
 Android Support
 ^^^^^^^^^^^^^^^
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index a1aea397925931..8f37895eaee242 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -886,26 +886,16 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
   } else {
     // Assume pre-ARMv6 doesn't support unaligned accesses.
     //
-    // ARMv6 may or may not support unaligned accesses depending on the
-    // SCTLR.U bit, which is architecture-specific. We assume ARMv6
-    // Darwin and NetBSD targets support unaligned accesses, and others don't.
+    // Assume ARMv6+ supports unaligned accesses, except Armv6-M, and Armv8-M
+    // without the Main Extension. This aligns with the default behavior of
+    // ARM's downstream versions of GCC and Clang
     //
-    // ARMv7 always has SCTLR.U set to 1, but it has a new SCTLR.A bit
-    // which raises an alignment fault on unaligned accesses. Linux
-    // defaults this bit to 0 and handles it as a system-wide (not
-    // per-process) setting. It is therefore safe to assume that ARMv7+
-    // Linux targets support unaligned accesses. The same goes for NaCl
-    // and Windows. However, ARM's forks of GCC and Clang both allow
-    // unaligned accesses by default for all targets. We follow this
-    // behavior and enable unaligned accesses by default for ARMv7+ targets.
-    // Users can disable behavior via compiler options (-mno-unaliged-access).
-    // See https://github.com/llvm/llvm-project/issues/59560 for more info.
+    // Users can disable behavior via -mno-unaliged-access.
     int VersionNum = getARMSubArchVersionNumber(Triple);
-    if (Triple.isOSDarwin() || Triple.isOSNetBSD()) {
-      if (VersionNum < 6 ||
-          Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m)
-        Features.push_back("+strict-align");
-    } else if (VersionNum < 7)
+    if (VersionNum < 6 ||
+        Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
+        Triple.getSubArch() ==
+            llvm::Triple::SubArchType::ARMSubArch_v8m_baseline)
       Features.push_back("+strict-align");
   }
 
diff --git a/clang/test/Driver/arm-alignment.c b/clang/test/Driver/arm-alignment.c
index 6d0084451e82c7..b2b0da4acb98f0 100644
--- a/clang/test/Driver/arm-alignment.c
+++ b/clang/test/Driver/arm-alignment.c
@@ -22,12 +22,10 @@
 // RUN: %clang -target armv7-windows -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
-/// Ensure that by default before ARMv7 we default to +strict-align
-// RUN: %clang -target armv6 -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+// RUN: %clang --target=armv6 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
-/// After ARMv7 by default we allow unaligned accesses for all targets
-// RUN: %clang -target armv7 -### %s 2> %t
+// RUN: %clang --target=armv7 -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
 // RUN: %clang --target=aarch64 -munaligned-access -### %s 2> %t
@@ -65,10 +63,10 @@
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6-unknown-linux -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6-unknown-nacl-gnueabihf -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6m-apple-darwin -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s

>From 59eb180ccc379913d64dda146ee60573a5b529ce Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Tue, 20 Feb 2024 15:24:12 -0800
Subject: [PATCH 4/5] Add a more descriptive release note.

Created using spr 1.3.4
---
 clang/docs/ReleaseNotes.rst | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6a4182474b3345..afb97fcd9bea01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -304,8 +304,16 @@ X86 Support
 
 Arm and AArch64 Support
 ^^^^^^^^^^^^^^^^^^^^^^^
+
 - ARMv6+ targets now default to allowing unaligned access, except Armv6-M, and
-  Armv8-M without the Main Extension.
+  Armv8-M without the Main Extension. Baremetal targets should check that the
+  new default will work with their system configurations, since it requires
+  that SCTLR.A is 0, SCTLR.U is 1, and that the memory in question is
+  configured as "normal" memory. We've made the value judgment that the
+  performance gains here outweigh breakages, since it is difficult to identify
+  performance loss from disabling unaligned access, but incorrect enabling
+  unaligned access will generate an obvious alignment fault on ARMv7+. This is
+  also the default setting for ARM's downstream compilers.
 
 Android Support
 ^^^^^^^^^^^^^^^

>From a082d7cbd019afd234d805715f47cd56a5ba3d34 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Wed, 21 Feb 2024 09:07:09 -0800
Subject: [PATCH 5/5] Remove changes to ARMv6, and update tests + docs

Created using spr 1.3.4
---
 clang/docs/ReleaseNotes.rst              |  5 +++--
 clang/lib/Driver/ToolChains/Arch/ARM.cpp | 28 +++++++++++++++++-------
 clang/test/Driver/arm-alignment.c        |  6 ++---
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index afb97fcd9bea01..19b78609ed1f3b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -305,7 +305,7 @@ X86 Support
 Arm and AArch64 Support
 ^^^^^^^^^^^^^^^^^^^^^^^
 
-- ARMv6+ targets now default to allowing unaligned access, except Armv6-M, and
+- ARMv7+ targets now default to allowing unaligned access, except Armv6-M, and
   Armv8-M without the Main Extension. Baremetal targets should check that the
   new default will work with their system configurations, since it requires
   that SCTLR.A is 0, SCTLR.U is 1, and that the memory in question is
@@ -313,7 +313,8 @@ Arm and AArch64 Support
   performance gains here outweigh breakages, since it is difficult to identify
   performance loss from disabling unaligned access, but incorrect enabling
   unaligned access will generate an obvious alignment fault on ARMv7+. This is
-  also the default setting for ARM's downstream compilers.
+  also the default setting for ARM's downstream compilers. We have not changed
+  the default behavior for ARMv6, but may revisit that decision in the future.
 
 Android Support
 ^^^^^^^^^^^^^^^
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 8f37895eaee242..ba158b92bb44ba 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -886,17 +886,29 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
   } else {
     // Assume pre-ARMv6 doesn't support unaligned accesses.
     //
-    // Assume ARMv6+ supports unaligned accesses, except Armv6-M, and Armv8-M
-    // without the Main Extension. This aligns with the default behavior of
-    // ARM's downstream versions of GCC and Clang
+    // ARMv6 may or may not support unaligned accesses depending on the
+    // SCTLR.U bit, which is architecture-specific. We assume ARMv6
+    // Darwin and NetBSD targets support unaligned accesses, and others don't.
     //
-    // Users can disable behavior via -mno-unaliged-access.
+    // ARMv7 always has SCTLR.U set to 1, but it has a new SCTLR.A bit which
+    // raises an alignment fault on unaligned accesses. Assume ARMv7+ supports
+    // unaligned accesses, except ARMv6-M, and ARMv8-M without the Main
+    // Extension. This aligns with the default behavior of ARM's downstream
+    // versions of GCC and Clang.
+    //
+    // Users can change the default behavior via -m[no-]unaliged-access.
     int VersionNum = getARMSubArchVersionNumber(Triple);
-    if (VersionNum < 6 ||
-        Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
-        Triple.getSubArch() ==
-            llvm::Triple::SubArchType::ARMSubArch_v8m_baseline)
+    if (Triple.isOSDarwin() || Triple.isOSNetBSD()) {
+      if (VersionNum < 6 ||
+          Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m)
+        Features.push_back("+strict-align");
+    } else if (VersionNum < 7 ||
+               Triple.getSubArch() ==
+                   llvm::Triple::SubArchType::ARMSubArch_v6m ||
+               Triple.getSubArch() ==
+                   llvm::Triple::SubArchType::ARMSubArch_v8m_baseline) {
       Features.push_back("+strict-align");
+    }
   }
 
   // llvm does not support reserving registers in general. There is support
diff --git a/clang/test/Driver/arm-alignment.c b/clang/test/Driver/arm-alignment.c
index b2b0da4acb98f0..1bdf4335edfe64 100644
--- a/clang/test/Driver/arm-alignment.c
+++ b/clang/test/Driver/arm-alignment.c
@@ -23,7 +23,7 @@
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
 // RUN: %clang --target=armv6 -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang --target=armv7 -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
@@ -63,10 +63,10 @@
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6-unknown-linux -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6-unknown-nacl-gnueabihf -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6m-apple-darwin -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s



More information about the cfe-commits mailing list