Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Mar 15, 2024

GCC ports only supports one of the options, with -mstrict-align
preferred by newer ports. They reject adding -m[no-]unaligned-access to
newer ports that use -m[no-]strict-align.
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111555).

We should not support aliases, either. Since the behavior has been
long-time for ARM (a146a48), support
both forms for ARM for now but remove -m[no-]unaligned-access for
RISC-V/LoongArch (see also
riscv-non-isa/riscv-c-api-doc#62).

While here, add TargetSpecific to ensure errors on unsupported targets
(https://reviews.llvm.org/D151590) and remove unneeded CC1 options.

Created using spr 1.3.5-bogner
@MaskRay MaskRay requested review from SixWeining and wangpc-pp and removed request for SixWeining March 15, 2024 02:05
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:ARM backend:AArch64 backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' backend:loongarch labels Mar 15, 2024
@MaskRay MaskRay requested a review from SixWeining March 15, 2024 02:05
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 15, 2024

@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-backend-loongarch

Author: Fangrui Song (MaskRay)

Changes

GCC ports only supports one of the options, with -mstrict-align
preferred by newer ports. And they reject adding such aliases
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111555).

We should not support aliases, either. Since the behavior has been
long-time for ARM (a146a48), support
aliases for ARM but fix other architectures.
https://reviews.llvm.org/D149946


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

9 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+8-10)
  • (modified) clang/lib/Driver/ToolChains/Arch/AArch64.cpp (+5-3)
  • (modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+11-8)
  • (modified) clang/lib/Driver/ToolChains/Arch/LoongArch.cpp (+3-4)
  • (modified) clang/lib/Driver/ToolChains/Arch/RISCV.cpp (+3-3)
  • (modified) clang/test/Driver/apple-kext-mkernel.c (+2-2)
  • (modified) clang/test/Driver/loongarch-munaligned-access.c (+6-32)
  • (modified) clang/test/Driver/munaligned-access-unused.c (+4-3)
  • (modified) clang/test/Driver/riscv-features.c (-2)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index a7e43b4d179a4d..a62966ae5506e2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4696,21 +4696,19 @@ 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">,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Force all memory accesses to be aligned (AArch64/LoongArch/RISC-V only)">;
+def mno_strict_align : Flag<["-"], "mno-strict-align">,
+  Visibility<[ClangOption, CC1Option]>,
+  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.">;
diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index aa3b80cb16e55a..3e6e29584df3ac 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -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))
       Features.push_back("+strict-align");
   } else if (Triple.isOSOpenBSD())
     Features.push_back("+strict-align");
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index ba158b92bb44ba..a68368c4758651 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -868,12 +868,16 @@ 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";
@@ -881,8 +885,7 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
       // 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.
     //
diff --git a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
index 31153a67ad2840..d23f9b36efb9ac 100644
--- a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
@@ -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))
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index a46b44f9ad2b2d..5165bccc6d7e30 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -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.
diff --git a/clang/test/Driver/apple-kext-mkernel.c b/clang/test/Driver/apple-kext-mkernel.c
index f03ed4a09b47d3..ac476e96802583 100644
--- a/clang/test/Driver/apple-kext-mkernel.c
+++ b/clang/test/Driver/apple-kext-mkernel.c
@@ -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"
diff --git a/clang/test/Driver/loongarch-munaligned-access.c b/clang/test/Driver/loongarch-munaligned-access.c
index 44edb2eb17e6ab..a545679949973a 100644
--- a/clang/test/Driver/loongarch-munaligned-access.c
+++ b/clang/test/Driver/loongarch-munaligned-access.c
@@ -1,54 +1,25 @@
 /// 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"
@@ -56,6 +27,9 @@
 // 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;
 }
diff --git a/clang/test/Driver/munaligned-access-unused.c b/clang/test/Driver/munaligned-access-unused.c
index 1d86edb798ef2d..4060b53c42fe5f 100644
--- a/clang/test/Driver/munaligned-access-unused.c
+++ b/clang/test/Driver/munaligned-access-unused.c
@@ -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 '{{.*}}'
diff --git a/clang/test/Driver/riscv-features.c b/clang/test/Driver/riscv-features.c
index fc5fb0f27e3af4..fe74ac773ef8ca 100644
--- a/clang/test/Driver/riscv-features.c
+++ b/clang/test/Driver/riscv-features.c
@@ -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
 

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 15, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Fangrui Song (MaskRay)

Changes

GCC ports only supports one of the options, with -mstrict-align
preferred by newer ports. And they reject adding such aliases
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111555).

We should not support aliases, either. Since the behavior has been
long-time for ARM (a146a48), support
aliases for ARM but fix other architectures.
https://reviews.llvm.org/D149946


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

9 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+8-10)
  • (modified) clang/lib/Driver/ToolChains/Arch/AArch64.cpp (+5-3)
  • (modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+11-8)
  • (modified) clang/lib/Driver/ToolChains/Arch/LoongArch.cpp (+3-4)
  • (modified) clang/lib/Driver/ToolChains/Arch/RISCV.cpp (+3-3)
  • (modified) clang/test/Driver/apple-kext-mkernel.c (+2-2)
  • (modified) clang/test/Driver/loongarch-munaligned-access.c (+6-32)
  • (modified) clang/test/Driver/munaligned-access-unused.c (+4-3)
  • (modified) clang/test/Driver/riscv-features.c (-2)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index a7e43b4d179a4d..a62966ae5506e2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4696,21 +4696,19 @@ 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">,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Force all memory accesses to be aligned (AArch64/LoongArch/RISC-V only)">;
+def mno_strict_align : Flag<["-"], "mno-strict-align">,
+  Visibility<[ClangOption, CC1Option]>,
+  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.">;
diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index aa3b80cb16e55a..3e6e29584df3ac 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -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))
       Features.push_back("+strict-align");
   } else if (Triple.isOSOpenBSD())
     Features.push_back("+strict-align");
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index ba158b92bb44ba..a68368c4758651 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -868,12 +868,16 @@ 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";
@@ -881,8 +885,7 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
       // 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.
     //
diff --git a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
index 31153a67ad2840..d23f9b36efb9ac 100644
--- a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
@@ -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))
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index a46b44f9ad2b2d..5165bccc6d7e30 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -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.
diff --git a/clang/test/Driver/apple-kext-mkernel.c b/clang/test/Driver/apple-kext-mkernel.c
index f03ed4a09b47d3..ac476e96802583 100644
--- a/clang/test/Driver/apple-kext-mkernel.c
+++ b/clang/test/Driver/apple-kext-mkernel.c
@@ -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"
diff --git a/clang/test/Driver/loongarch-munaligned-access.c b/clang/test/Driver/loongarch-munaligned-access.c
index 44edb2eb17e6ab..a545679949973a 100644
--- a/clang/test/Driver/loongarch-munaligned-access.c
+++ b/clang/test/Driver/loongarch-munaligned-access.c
@@ -1,54 +1,25 @@
 /// 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"
@@ -56,6 +27,9 @@
 // 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;
 }
diff --git a/clang/test/Driver/munaligned-access-unused.c b/clang/test/Driver/munaligned-access-unused.c
index 1d86edb798ef2d..4060b53c42fe5f 100644
--- a/clang/test/Driver/munaligned-access-unused.c
+++ b/clang/test/Driver/munaligned-access-unused.c
@@ -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 '{{.*}}'
diff --git a/clang/test/Driver/riscv-features.c b/clang/test/Driver/riscv-features.c
index fc5fb0f27e3af4..fe74ac773ef8ca 100644
--- a/clang/test/Driver/riscv-features.c
+++ b/clang/test/Driver/riscv-features.c
@@ -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
 

Copy link

github-actions bot commented Mar 15, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff b8db3e7c7dddaa14c314a05b92c9fa3df38734e4 a3d4ddb6b2073dfa26a860c4ea0a6bd87ae55c78 -- clang/lib/Driver/ToolChains/Arch/AArch64.cpp clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/lib/Driver/ToolChains/Arch/LoongArch.cpp clang/lib/Driver/ToolChains/Arch/RISCV.cpp clang/test/Driver/apple-kext-mkernel.c clang/test/Driver/loongarch-munaligned-access.c clang/test/Driver/munaligned-access-unused.c clang/test/Driver/riscv-features.c
View the diff from clang-format here.
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index a68368c475..b41bb81e6e 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -868,10 +868,9 @@ fp16_fml_fallthrough:
     }
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
-                                      options::OPT_munaligned_access,
-                                      options::OPT_mstrict_align,
-                                      options::OPT_mno_strict_align)) {
+  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) ||

@MaskRay MaskRay requested a review from wzssyqa March 15, 2024 02:18
Created using spr 1.3.5-bogner
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

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

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM. cc @asb @topperc
Some context of RISCV target: riscv-non-isa/riscv-c-api-doc#62

Copy link
Contributor

@SixWeining SixWeining left a comment

Choose a reason for hiding this comment

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

LGTM from LoongArch side as GCC only have -m[no-]strict-align.
BTW, should clang release notes be updated for this change?

Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit cd07125 into main Mar 15, 2024
3 of 5 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/driver-dont-alias-mstrict-align-to-mno-unaligned-access branch March 15, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 backend:ARM backend:loongarch backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants