Skip to content

[Driver] Don't alias -mstrict-align to -mno-unaligned-access #85350

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 5 commits into from
Mar 15, 2024
Merged

[Driver] Don't alias -mstrict-align to -mno-unaligned-access #85350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
94997d2
[𝘀𝗽𝗿] initial version
MaskRay Mar 15, 2024
acb5760
also remove cc1 options
MaskRay Mar 15, 2024
586ab47
add clang/docs/ReleaseNotes.rst
MaskRay Mar 15, 2024
fe7eb31
better description
MaskRay Mar 15, 2024
a3d4ddb
rebase
MaskRay Mar 15, 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
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ Removed Compiler Flags
-------------------------

- The ``-freroll-loops`` flag has been removed. It had no effect since Clang 13.
- ``-m[no-]unaligned-access`` is removed for RISC-V and LoongArch.
``-m[no-]strict-align``, also supported by GCC, should be used instead.
(`#85350 <https://github.com/llvm/llvm-project/pull/85350>`_.)

Attribute Changes in Clang
--------------------------
Expand Down
16 changes: 6 additions & 10 deletions clang/include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4696,21 +4696,17 @@ def mrvv_vector_bits_EQ : Joined<["-"], "mrvv-vector-bits=">, Group<m_Group>,
" (RISC-V only)")>;

def munaligned_access : Flag<["-"], "munaligned-access">, Group<m_Group>,
HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64/LoongArch/RISC-V only)">;
HelpText<"Allow memory accesses to be unaligned (AArch32 only)">;
def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">, Group<m_Group>,
HelpText<"Force all memory accesses to be aligned (AArch32/AArch64/LoongArch/RISC-V only)">;
HelpText<"Force all memory accesses to be aligned (AArch32 only)">;
def munaligned_symbols : Flag<["-"], "munaligned-symbols">, Group<m_Group>,
HelpText<"Expect external char-aligned symbols to be without ABI alignment (SystemZ only)">;
def mno_unaligned_symbols : Flag<["-"], "mno-unaligned-symbols">, Group<m_Group>,
HelpText<"Expect external char-aligned symbols to be without ABI alignment (SystemZ only)">;
} // let Flags = [TargetSpecific]
def mstrict_align : Flag<["-"], "mstrict-align">, Alias<mno_unaligned_access>,
Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
HelpText<"Force all memory accesses to be aligned (same as mno-unaligned-access)">;
def mno_strict_align : Flag<["-"], "mno-strict-align">, Alias<munaligned_access>,
Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
HelpText<"Allow memory accesses to be unaligned (same as munaligned-access)">;
let Flags = [TargetSpecific] in {
def mstrict_align : Flag<["-"], "mstrict-align">,
HelpText<"Force all memory accesses to be aligned (AArch64/LoongArch/RISC-V only)">;
def mno_strict_align : Flag<["-"], "mno-strict-align">,
HelpText<"Allow memory accesses to be unaligned (AArch64/LoongArch/RISC-V only)">;
def mno_thumb : Flag<["-"], "mno-thumb">, Group<m_arm_Features_Group>;
def mrestrict_it: Flag<["-"], "mrestrict-it">, Group<m_arm_Features_Group>,
HelpText<"Disallow generation of complex IT blocks. It is off by default.">;
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/Driver/ToolChains/Arch/AArch64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,11 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
}
}

if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access)) {
if (A->getOption().matches(options::OPT_mno_unaligned_access))
if (Arg *A = Args.getLastArg(
options::OPT_mstrict_align, options::OPT_mno_strict_align,
options::OPT_mno_unaligned_access, options::OPT_munaligned_access)) {
if (A->getOption().matches(options::OPT_mstrict_align) ||
A->getOption().matches(options::OPT_mno_unaligned_access))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why keep mno_unaligned_access for AArch64 while remove it from RISC-V and LoongArch?

In fact mno_unaligned_access is not supported by GCC for AArch64?

Copy link
Contributor

Choose a reason for hiding this comment

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

For MIPSr6, since mno_unaligned_access has been in GCC for almost 2 years, I prefer support these 2 options both for LLVM and GCC.

In fact, I don't think that it is a bad idea to support these 2 options both: it will make life easier for users.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we prefer strict-align, we may claim mno_unaligned_access as obsolete in our documents.

Copy link
Member Author

Choose a reason for hiding this comment

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

AArch64 supports Apple OSes. I am unclear whether have any unintended -munaligned-access/-mno-unaligned-access. I'd like to remove them as well, but probably later.

For RISC-V and LoongArch, they have Linux support and the GCC influence is very strong. The Clang support is also relatively new. Removing the alias likely cause no disruption at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM.

I will add strict_align for MIPSr6 to GCC, and rebase my PR to LLVM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. When you implement -munaligned-access for LLVM's MIPS port, be sure to update the HelpText in clang/include/clang/Driver/Options.td AArch32/MIPS only

Copy link
Collaborator

Choose a reason for hiding this comment

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

Preventing unaligned access can be useful in AArch64, it is an option we do use to build our embedded C-libraries with (not a focus for GCC). It is documented in the toolchain manual https://developer.arm.com/documentation/101754/0621/armclang-Reference/armclang-Command-line-Options/-munaligned-access---mno-unaligned-access

In summary, we'd like to keep it for AArch64.

AArch64 always has the option of using unaligned accesses, but they can be disabled by writing the SCTLR register, and accesses to Device memory always need to be aligned. Code that runs before the MMU is enabled runs as if Device memory.

Unaligned accesses to Normal memory
The behavior of unaligned accesses to Normal memory is dependent on all of the following:
• The instruction causing the memory access.
• The memory attributes of the accessed memory.
• The value of SCTLR_ELx.{A, nAA}.
• Whether or not FEAT_LSE2 is implemented.

Copy link
Member Author

@MaskRay MaskRay Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the AArch64 notes.
This HelpText (AArch32 only) change is to discourage -munaligned-access for AArch64, since GCC doesn't support -munaligned-access.

Personally I assume that most modern architectures should switch to -mstrict-align, if they have an aligned vs unaligned access difference.

(Related, I have been trying to make more code compliant under -fsanitize=alignment, which might makes memory access operations with hardware check easier.
)

Features.push_back("+strict-align");
} else if (Triple.isOSOpenBSD())
Features.push_back("+strict-align");
Expand Down
19 changes: 11 additions & 8 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 @@ -868,21 +868,24 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
}
}

// Kernel code has more strict alignment requirements.
if (KernelOrKext) {
Features.push_back("+strict-align");
} else if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access)) {
if (A->getOption().matches(options::OPT_munaligned_access)) {
if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access,
options::OPT_mstrict_align,
options::OPT_mno_strict_align)) {
// Kernel code has more strict alignment requirements.
if (KernelOrKext ||
A->getOption().matches(options::OPT_mno_unaligned_access) ||
A->getOption().matches(options::OPT_mstrict_align)) {
Features.push_back("+strict-align");
} else {
// No v6M core supports unaligned memory access (v6M ARM ARM A3.2).
if (Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m)
D.Diag(diag::err_target_unsupported_unaligned) << "v6m";
// v8M Baseline follows on from v6M, so doesn't support unaligned memory
// access either.
else if (Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v8m_baseline)
D.Diag(diag::err_target_unsupported_unaligned) << "v8m.base";
} else
Features.push_back("+strict-align");
}
} else {
// Assume pre-ARMv6 doesn't support unaligned accesses.
//
Expand Down
7 changes: 3 additions & 4 deletions clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,9 @@ void loongarch::getLoongArchTargetFeatures(const Driver &D,
}
}

// Select the `ual` feature determined by -m[no-]unaligned-access
// or the alias -m[no-]strict-align.
AddTargetFeature(Args, Features, options::OPT_munaligned_access,
options::OPT_mno_unaligned_access, "ual");
// Select the `ual` feature determined by -m[no-]strict-align.
AddTargetFeature(Args, Features, options::OPT_mno_strict_align,
options::OPT_mstrict_align, "ual");

// Accept but warn about these TargetSpecific options.
if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ))
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Driver/ToolChains/Arch/RISCV.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
Features.push_back("-relax");
}

// -mno-unaligned-access is default, unless -munaligned-access is specified.
AddTargetFeature(Args, Features, options::OPT_munaligned_access,
options::OPT_mno_unaligned_access, "fast-unaligned-access");
// -mstrict-align is default, unless -mno-strict-align is specified.
AddTargetFeature(Args, Features, options::OPT_mno_strict_align,
options::OPT_mstrict_align, "fast-unaligned-access");

// Now add any that the user explicitly requested on the command line,
// which may override the defaults.
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Driver/apple-kext-mkernel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
// CHECK-X86-2: "-fno-rtti"
// CHECK-X86-2-NOT: "-fno-common"

// RUN: not %clang -target x86_64-apple-darwin11 -arch armv7 -mkernel -mstrict-align -### -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-ARM %s
// RUN: not %clang -target x86_64-apple-darwin11 -arch armv7 -mkernel -mstrict-align -### -fsyntax-only -fbuiltin -fno-builtin -fcommon -fno-common %s 2>&1 | FileCheck --check-prefix=CHECK-ARM %s
// RUN: %clang --target=x86_64-apple-darwin11 -arch armv7 -mkernel -mstrict-align -### -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-ARM %s
// RUN: %clang --target=x86_64-apple-darwin11 -arch armv7 -mkernel -mstrict-align -### -fsyntax-only -fbuiltin -fno-builtin -fcommon -fno-common %s 2>&1 | FileCheck --check-prefix=CHECK-ARM %s

// CHECK-ARM: "-target-feature" "+long-calls"
// CHECK-ARM: "-target-feature" "+strict-align"
Expand Down
38 changes: 6 additions & 32 deletions clang/test/Driver/loongarch-munaligned-access.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,61 +1,35 @@
/// Test -m[no-]unaligned-access and -m[no-]strict-align options.

// RUN: %clang --target=loongarch64 -munaligned-access -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-unaligned-access -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mstrict-align -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-strict-align -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-UNALIGNED
// RUN: %clang --target=loongarch64 -munaligned-access -mno-unaligned-access -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-unaligned-access -munaligned-access -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-UNALIGNED
// RUN: %clang --target=loongarch64 -mstrict-align -mno-strict-align -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-strict-align -mstrict-align -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -munaligned-access -mstrict-align -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mstrict-align -munaligned-access -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-unaligned-access -mno-strict-align -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-strict-align -mno-unaligned-access -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1-NO-UNALIGNED

// RUN: %clang --target=loongarch64 -munaligned-access -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-unaligned-access -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mstrict-align -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-strict-align -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-UNALIGNED
// RUN: %clang --target=loongarch64 -munaligned-access -mno-unaligned-access -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-unaligned-access -munaligned-access -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-UNALIGNED
// RUN: %clang --target=loongarch64 -mstrict-align -mno-strict-align -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-strict-align -mstrict-align -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -munaligned-access -mstrict-align -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-NO-UNALIGNED
// RUN: %clang --target=loongarch64 -mstrict-align -munaligned-access -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-unaligned-access -mno-strict-align -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-UNALIGNED
// RUN: %clang --target=loongarch64 -mno-strict-align -mno-unaligned-access -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR-NO-UNALIGNED

// RUN: not %clang -### --target=loongarch64 -mno-unaligned-access -munaligned-access %s 2>&1 | \
// RUN: FileCheck %s --check-prefix=ERR

// CC1-UNALIGNED: "-target-feature" "+ual"
// CC1-NO-UNALIGNED: "-target-feature" "-ual"

// IR-UNALIGNED: attributes #[[#]] ={{.*}}"target-features"="{{(.*,)?}}+ual{{(,.*)?}}"
// IR-NO-UNALIGNED: attributes #[[#]] ={{.*}}"target-features"="{{(.*,)?}}-ual{{(,.*)?}}"

// ERR: error: unsupported option '-mno-unaligned-access' for target 'loongarch64'
// ERR: error: unsupported option '-munaligned-access' for target 'loongarch64'

int foo(void) {
return 3;
}
7 changes: 4 additions & 3 deletions clang/test/Driver/munaligned-access-unused.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/// Check -m[no-]unaligned-access and -m[no-]strict-align are warned unused on a target that does not support them.
/// Check -m[no-]unaligned-access and -m[no-]strict-align are errored on a target that does not support them.

// RUN: not %clang --target=x86_64 -munaligned-access -fsyntax-only %s -### 2>&1 | FileCheck %s -DOPTION=unaligned-access
// RUN: not %clang --target=x86_64 -mno-unaligned-access -fsyntax-only %s -### 2>&1 | FileCheck %s -DOPTION=no-unaligned-access
// RUN: not %clang --target=x86_64 -mstrict-align -fsyntax-only %s -### 2>&1 | FileCheck %s -DOPTION=strict-align
// RUN: not %clang --target=x86_64 -mno-strict-align -fsyntax-only %s -### 2>&1 | FileCheck %s -DOPTION=no-strict-align
// RUN: not %clang --target=x86_64 -mno-strict-align -mstrict-align -fsyntax-only %s -### 2>&1 | FileCheck %s --check-prefix=ALIGN

// CHECK: error: unsupported option '-m{{(no-)?}}unaligned-access' for target '{{.*}}'
// ALIGN: error: unsupported option '-mno-strict-align' for target '{{.*}}'
// ALIGN: error: unsupported option '-mstrict-align' for target '{{.*}}'
2 changes: 0 additions & 2 deletions clang/test/Driver/riscv-features.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
// NO-FORCE-SW-SCS: "-target-feature" "-forced-sw-shadow-stack"
// DEFAULT-NOT: "-target-feature" "+forced-sw-shadow-stack"

// RUN: %clang --target=riscv32-unknown-elf -### %s -munaligned-access 2>&1 | FileCheck %s -check-prefix=FAST-UNALIGNED-ACCESS
// RUN: %clang --target=riscv32-unknown-elf -### %s -mno-unaligned-access 2>&1 | FileCheck %s -check-prefix=NO-FAST-UNALIGNED-ACCESS
// RUN: %clang --target=riscv32-unknown-elf -### %s -mno-strict-align 2>&1 | FileCheck %s -check-prefix=FAST-UNALIGNED-ACCESS
// RUN: %clang --target=riscv32-unknown-elf -### %s -mstrict-align 2>&1 | FileCheck %s -check-prefix=NO-FAST-UNALIGNED-ACCESS

Expand Down