Skip to content

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

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.

Sign up for GitHub

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 6 commits into from
Feb 22, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
20634c2
[𝘀𝗽𝗿] initial version
ilovepi Feb 20, 2024
a078e65
git clang-format
ilovepi Feb 20, 2024
c21f9aa
Address comments, and update checking
ilovepi Feb 20, 2024
59eb180
Add a more descriptive release note.
ilovepi Feb 20, 2024
a082d7c
Remove changes to ARMv6, and update tests + docs
ilovepi Feb 21, 2024
10fdeb1
Update release notes and add tests for missing targets
ilovepi Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. This brings Clang in-line with the default
settings for GCC and Arm Compiler. Aside from making Clang align with other
compilers, changing the default brings major performance and code size
improvements for most targets. We have not changed the default behavior for
ARMv6, but may revisit that decision in the future. Users can restore the old
behavior with -m[no-]unaligned-access.

Android Support
^^^^^^^^^^^^^^^

Expand Down
24 changes: 12 additions & 12 deletions clang/lib/Driver/ToolChains/Arch/ARM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,25 +890,25 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
// SCTLR.U bit, which is architecture-specific. We assume ARMv6
// Darwin and NetBSD targets support unaligned accesses, and others don't.
//
// 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.
// 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.
//
// The above behavior is consistent with GCC.
// Users can change the default behavior via -m[no-]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
} 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
Expand Down
15 changes: 15 additions & 0 deletions clang/test/Driver/arm-alignment.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@
// 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-ARM < %t %s

// RUN: %clang -target thumbv6m-none-gnueabi -mcpu=cortex-m0 -### %s 2> %t
// RUN: FileCheck --check-prefix CHECK-ALIGNED-ARM <%t %s

// RUN: %clang -target thumb-none-gnueabi -mcpu=cortex-m0 -### %s 2> %t
// RUN: FileCheck --check-prefix CHECK-ALIGNED-ARM <%t %s

// RUN: %clang -target thumbv8m.base-none-gnueabi -### %s 2> %t
// RUN: FileCheck --check-prefix CHECK-ALIGNED-ARM <%t %s

// RUN: %clang --target=aarch64 -munaligned-access -### %s 2> %t
// RUN: FileCheck --check-prefix=CHECK-UNALIGNED-AARCH64 < %t %s

Expand Down