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

2024-02-22 Thread Paul Kirth via cfe-commits

https://github.com/ilovepi closed 
https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread Eli Friedman via cfe-commits

https://github.com/efriedma-quic approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread Paul Kirth via cfe-commits

ilovepi wrote:

> I would also personally add Armv6 too for consistency, but don't have a 
> strong opinion.

I think we should too, but everyone agrees this is a strict improvement, so I'd 
prefer to land something now and we can hash out the right thing for ARMv6  as 
a follow up. Does that sound good to you?

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread Paul Kirth via cfe-commits


@@ -305,6 +305,17 @@ X86 Support
 Arm and AArch64 Support
 ^^^
 
+- 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
+  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. We have not changed
+  the default behavior for ARMv6, but may revisit that decision in the future.

ilovepi wrote:

Thanks for the suggested edits. I've mostly kept them, but as you mentioned 
added a bit about the performance/size benefits.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread Paul Kirth via cfe-commits


@@ -22,6 +22,12 @@
 // RUN: %clang -target armv7-windows -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
+// RUN: %clang --target=armv6 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+
+// RUN: %clang --target=armv7 -### %s 2> %t

ilovepi wrote:

Thank you for pointing that out. I had misread the RUN lines at the bottom of 
the file, and missed that they were emitting an error rather than checking the 
cc1 options. I used those same targets. Please let me know if I've missed any.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread Paul Kirth via cfe-commits

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 
Date: Tue, 20 Feb 2024 10:34:57 -0800
Subject: [PATCH 1/6] =?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 
Date: Tue, 20 Feb 2024 10:38:50 -0800
Subject: [PATCH 2/6] 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 
Date: Tue, 20 Feb 2024 12:05:48 -0800
Subject: [PATCH 3/6] 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
---

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

2024-02-22 Thread David Green via cfe-commits

davemgreen wrote:

I would also personally add Armv6 too for consistency, but don't have a strong 
opinion.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread David Green via cfe-commits


@@ -305,6 +305,17 @@ X86 Support
 Arm and AArch64 Support
 ^^^
 
+- 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
+  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. We have not changed
+  the default behavior for ARMv6, but may revisit that decision in the future.

davemgreen wrote:

I would up-play the compatibility argument and downplay the judgement call a 
little. And mention the way to disable it. Maybe something like
```
- 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
  configured as "normal" memory. This brings clang in-line with the default 
settings
  for GCC and Arm Compiler. The old behaviour can be restored with
  -mno-unaligned-access.
```
But you might want to re-add the reasoning about the performance/codesize loss.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread David Green via cfe-commits


@@ -22,6 +22,12 @@
 // RUN: %clang -target armv7-windows -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s
 
+// RUN: %clang --target=armv6 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+
+// RUN: %clang --target=armv7 -### %s 2> %t

davemgreen wrote:

Can you add some extra tests for things like 8-m.main, 8-m.base and 6-m, to 
make sure we have coverage?

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread David Green via cfe-commits

https://github.com/davemgreen edited 
https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-22 Thread David Green via cfe-commits

https://github.com/davemgreen commented:

> Unaligned accesses require that SCTLR.A is 0, SCTLR.U is 1, and that the 
> memory in question is configured as "normal" memory. Almost all operating 
> systems do in fact configure their registers/memory this way, but on 
> baremetal it's not really a safe assumption. Changing the default here is 
> basically guaranteed to break someone's code.
> 
> We could make the value judgement that the performance gain outweighs 
> breaking someone's code. Disabled unaligned access is a performance hit users 
> have a hard time discovering; incorrectly enabled unaligned access will 
> generate an obvious alignment fault on v7 and up.
> 
> On pre-v7 processors, you can get the old "rotate" behavior instead of a 
> fault. This is controlled by SCTLR.U on v6, but I don't think there's any 
> reason to expect the bit is configured the "right" way on baremetal. So 
> changing the default for v6 is a bit more dubious.
> 
> The tradeoffs might be a bit different for M-class processors; I think 
> unaligned access works by default there (except for v6m and v8m.baseline).
> 
> This change needs a release note.

The issue that we have found is that as both GCC and Arm Compiler default to 
-munaligned-access, users expect it to be the default. They only notice the 
much bigger codesize/worse performance and don't understand the reason without 
a lot of digging. You are certainly right that someone who has been using clang 
bare metal in the past might hit problems with the new default, but there is a 
high chance they are just using the wrong option without noticing. And I 
believe aligning with GCC/Arm Compiler is a better default going forward, as 
more people start using Clang in bare metal. Hopefully the release note can at 
least make it clear.

M-Profile needs a bit set in the bootcode, IIRC.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-21 Thread Paul Kirth via cfe-commits

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 
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 
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 
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
---

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

2024-02-21 Thread Paul Kirth via cfe-commits


@@ -886,28 +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.
-//
-// The above behavior is consistent with GCC.
+// 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 (Triple.isOSLinux() || Triple.isOSNaCl() ||
-   Triple.isOSWindows()) {
-  if (VersionNum < 7)
-Features.push_back("+strict-align");
-} else
+if (VersionNum < 6 ||
+Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
+Triple.getSubArch() ==
+llvm::Triple::SubArchType::ARMSubArch_v8m_baseline)

ilovepi wrote:

I think this is a good start. We can discuss relaxing our treatment of v6 later 
without much trouble. I'll go ahead and update the comments and release notes 
to match this a bit more closely.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Eli Friedman via cfe-commits


@@ -886,28 +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.
-//
-// The above behavior is consistent with GCC.
+// 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 (Triple.isOSLinux() || Triple.isOSNaCl() ||
-   Triple.isOSWindows()) {
-  if (VersionNum < 7)
-Features.push_back("+strict-align");
-} else
+if (VersionNum < 6 ||
+Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
+Triple.getSubArch() ==
+llvm::Triple::SubArchType::ARMSubArch_v8m_baseline)

efriedma-quic wrote:

I don't have a strong opinion on how we treat v6a. I guess I'd lean towards 
being conservative, I guess, like what you wrote?

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits


@@ -886,28 +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.
-//
-// The above behavior is consistent with GCC.
+// 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 (Triple.isOSLinux() || Triple.isOSNaCl() ||
-   Triple.isOSWindows()) {
-  if (VersionNum < 7)
-Features.push_back("+strict-align");
-} else
+if (VersionNum < 6 ||
+Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
+Triple.getSubArch() ==
+llvm::Triple::SubArchType::ARMSubArch_v8m_baseline)

ilovepi wrote:

@efriedma-quic I'm not too sure what the correct check would be here given your 
comment. Maybe this? 

```cpp
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");
} 
```

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits


@@ -305,6 +305,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. 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.
+

ilovepi wrote:

@efriedma-quic @davemgreen Is this more in line with what you would expect in 
the release notes?

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits

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 
Date: Tue, 20 Feb 2024 10:34:57 -0800
Subject: [PATCH 1/4] =?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 
Date: Tue, 20 Feb 2024 10:38:50 -0800
Subject: [PATCH 2/4] 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 
Date: Tue, 20 Feb 2024 12:05:48 -0800
Subject: [PATCH 3/4] 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
---

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

2024-02-20 Thread Eli Friedman via cfe-commits

efriedma-quic wrote:

Unaligned accesses require that SCTLR.A is 0, SCTLR.U is 1, and that the memory 
in question is configured as "normal" memory.  Almost all operating systems do 
in fact configure their registers/memory this way, but on baremetal it's not 
really a safe assumption. Changing the default here is basically guaranteed to 
break someone's code.

We could make the value judgement that the performance gain outweighs breaking 
someone's code. Disabled unaligned access is a performance hit users have a 
hard time discovering; incorrectly enabled unaligned access will generate an 
obvious alignment fault on v7 and up.

On pre-v7 processors, you can get the old "rotate" behavior instead of a fault. 
 This is controlled by SCTLR.U on v6, but I don't think there's any reason to 
expect the bit is configured the "right" way on baremetal.  So changing the 
default for v6 is a bit more dubious.

The tradeoffs might be a bit different for M-class processors; I think 
unaligned access works by default there (except for v6m and v8m.baseline).

This change needs a release note.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits

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 
Date: Tue, 20 Feb 2024 10:34:57 -0800
Subject: [PATCH 1/3] =?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 
Date: Tue, 20 Feb 2024 10:38:50 -0800
Subject: [PATCH 2/3] 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 
Date: Tue, 20 Feb 2024 12:05:48 -0800
Subject: [PATCH 3/3] 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
---

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

2024-02-20 Thread Fangrui Song via cfe-commits


@@ -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

MaskRay wrote:

`--target=` for new tests.

`-target ` has been deprecated since about Clang 3.4

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Peter Smith via cfe-commits

smithp35 wrote:

Off the top of my head we default to `-ffunction-sections`  and 
`-fdata-sections` as this helps Garbage Collection, and we have a linker 
feature that can merge constant pool entries between functions that needs the 
gaps between the functions.


https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits


@@ -895,19 +895,17 @@ 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.

ilovepi wrote:

Will do.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits

ilovepi wrote:

> Hi - I like the change. We have this code in the downstream compiler, which 
> also enables this for Armv6, but specifically disables it for v6m and 
> v8m.baseline.
> 
> ```
>   if (VersionNum < 6 ||
>   Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
>   Triple.getSubArch() == 
> llvm::Triple::SubArchType::ARMSubArch_v8m_baseline) {
> Features.push_back("+strict-align");
>   }
> ```
> 
> I don't have a strong opinion about what happens with ARMv6, but this 
> deserves a release note. And v6m/v8m.baseline probably deserve specific code 
> and a test.

Those are good points, echoed by @smithp35. I'll update the patch accordingly.

Off hand, do either of you know if there are other differences in the driver 
that that we would want to consider adopting?

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread David Green via cfe-commits


@@ -895,19 +895,17 @@ 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.

davemgreen wrote:

Also, I would drop the reference to the github issue from the comment. People 
can inspect the git history if they need to go looking for a reason behind it. 

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Peter Smith via cfe-commits


@@ -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)

smithp35 wrote:

I don't think this is quite right. Armv6 (but not Armv6-M) can support 
unaligned accesses. V8-M.main (logical extension of Armv7-M) supports unaligned 
accesses but v8-M.base (logical extension of Armv6-M) does not. Yet both will 
get VersionNum of 8.

Although not quite at the same point Arm's downstream fork uses
```
if (VersionNum < 6 ||
  Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
  Triple.getSubArch() == 
llvm::Triple::SubArchType::ARMSubArch_v8m_baseline) {
Features.push_back("+strict-align");
```
I don't suppose many care about Arm v6 but we'll need to make sure that 
strict-align is added for v8-m.base.

Arm's fork of clang mentions this in the documentation for 
-mno-unaligned-access 
https://developer.arm.com/documentation/101754/0621/armclang-Reference/armclang-Command-line-Options/-munaligned-access---mno-unaligned-access
 



 

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread David Green via cfe-commits

davemgreen wrote:

Hi - I like the change. We have this code in the downstream compiler, which 
also enables this for Armv6, but specifically disables it for v6m and 
v8m.baseline.
```
  if (VersionNum < 6 ||
  Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m ||
  Triple.getSubArch() == 
llvm::Triple::SubArchType::ARMSubArch_v8m_baseline) {
Features.push_back("+strict-align");
  }
```

I don't have a strong opinion about what happens with ARMv6, but this deserves 
a release note.

https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Petr Hosek via cfe-commits

https://github.com/petrhosek approved this pull request.


https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits

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 
Date: Tue, 20 Feb 2024 10:34:57 -0800
Subject: [PATCH 1/2] =?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 
Date: Tue, 20 Feb 2024 10:38:50 -0800
Subject: [PATCH 2/2] 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 9facaaddadd63a1668c212c8a9ef94a5ad4c6629 
20634c2f54ae667ee8374d12e58e582aa6cdd051 -- 
clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/test/Driver/arm-alignment.c
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 3bf6056f0c..a1aea39792 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -906,7 +906,7 @@ fp16_fml_fallthrough:
   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

``




https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-backend-arm

Author: Paul Kirth (ilovepi)


Changes

ARM's Clang and GCC embedded compilers default to allowing unaligned
access for ARMv7+. This patch changes the Clang driver default to match.
Users can opt out with `-mno-unaligned-access`.

Fixes #59560


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


2 Files Affected:

- (modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+6-8) 
- (modified) clang/test/Driver/arm-alignment.c (+8) 


``diff
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
 

``




https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread via cfe-commits

llvmbot wrote:



@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Paul Kirth (ilovepi)


Changes

ARM's Clang and GCC embedded compilers default to allowing unaligned
access for ARMv7+. This patch changes the Clang driver default to match.
Users can opt out with `-mno-unaligned-access`.

Fixes #59560


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


2 Files Affected:

- (modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+6-8) 
- (modified) clang/test/Driver/arm-alignment.c (+8) 


``diff
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
 

``




https://github.com/llvm/llvm-project/pull/82400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-02-20 Thread Paul Kirth via cfe-commits

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

ARM's Clang and GCC embedded compilers default to allowing unaligned
access for ARMv7+. This patch changes the Clang driver default to match.
Users can opt out with `-mno-unaligned-access`.

Fixes #59560


>From 20634c2f54ae667ee8374d12e58e582aa6cdd051 Mon Sep 17 00:00:00 2001
From: Paul Kirth 
Date: Tue, 20 Feb 2024 10:34:57 -0800
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
 =?UTF-8?q?l=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
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits