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] Mark -arch as TargetSpecific #74365

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Dec 4, 2023

-arch is a Darwin-specific option that is ignored for other targets
and not known by GCC.

% clang -arch arm64 -c a.c
clang: warning: argument unused during compilation: '-arch arm64' [-Wunused-command-line-argument]

We are utilizing TargetSpecific (from https://reviews.llvm.org/D151590)
to make more options lead to errors for unsupported targets.

`-arch` is a Darwin-specific option that is ignored for other targets
and not known by GCC.
```
% clang -arch arm64 -c a.c
clang: warning: argument unused during compilation: '-arch arm64' [-Wunused-command-line-argument]
```

We are utilizing TargetSpecific (from https://reviews.llvm.org/D151590)
to make more options lead to errors for unsupported targets.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Dec 4, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Fangrui Song (MaskRay)

Changes

-arch is a Darwin-specific option that is ignored for other targets
and not known by GCC.

% clang -arch arm64 -c a.c
clang: warning: argument unused during compilation: '-arch arm64' [-Wunused-command-line-argument]

We are utilizing TargetSpecific (from https://reviews.llvm.org/D151590)
to make more options lead to errors for unsupported targets.


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

4 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+1-1)
  • (modified) clang/test/Driver/arc-exceptions.m (+2-2)
  • (modified) clang/test/Driver/arm-arch-darwin.c (+3-2)
  • (modified) clang/test/Frontend/darwin-eabi.c (+3-3)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 1d04e4f6e7e6d..8c6d1f61ebcf2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -994,7 +994,7 @@ def all__load : Flag<["-"], "all_load">;
 def allowable__client : Separate<["-"], "allowable_client">;
 def ansi : Flag<["-", "--"], "ansi">, Group<CompileOnly_Group>;
 def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
-def arch : Separate<["-"], "arch">, Flags<[NoXarchOption]>;
+def arch : Separate<["-"], "arch">, Flags<[NoXarchOption,TargetSpecific]>;
 def arch__only : Separate<["-"], "arch_only">;
 def autocomplete : Joined<["--"], "autocomplete=">;
 def bind__at__load : Flag<["-"], "bind_at_load">;
diff --git a/clang/test/Driver/arc-exceptions.m b/clang/test/Driver/arc-exceptions.m
index 4501ccd073823..c1dd02d59988c 100644
--- a/clang/test/Driver/arc-exceptions.m
+++ b/clang/test/Driver/arc-exceptions.m
@@ -1,5 +1,5 @@
-// RUN: %clang -### -x objective-c -arch x86_64 -fobjc-arc -fsyntax-only %s 2> %t.log
+// RUN: %clang -### -x objective-c --target=x86_64-apple-macos10.6 -fobjc-arc -fsyntax-only %s 2> %t.log
 // RUN: grep objective-c %t.log
 // RUN: not grep "fobjc-arc-exceptions" %t.log
-// RUN: %clang -### -x objective-c++ -arch x86_64 -fobjc-arc -fsyntax-only %s 2> %t.log
+// RUN: %clang -### -x objective-c++ --target=x86_64-apple-macos10.6 -fobjc-arc -fsyntax-only %s 2> %t.log
 // RUN: grep "fobjc-arc-exceptions" %t.log
diff --git a/clang/test/Driver/arm-arch-darwin.c b/clang/test/Driver/arm-arch-darwin.c
index 55089619d1e71..c523622964738 100644
--- a/clang/test/Driver/arm-arch-darwin.c
+++ b/clang/test/Driver/arm-arch-darwin.c
@@ -1,6 +1,7 @@
 // On Darwin, arch should override CPU for triple purposes
 // RUN: %clang -target armv7m-apple-darwin -arch armv7m -mcpu=cortex-m4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V7M-DARWIN %s
 // CHECK-V7M-DARWIN: "-cc1"{{.*}} "-triple" "thumbv7m-{{.*}} "-target-cpu" "cortex-m4"
-// RUN: %clang -target armv7m -arch armv7m -mcpu=cortex-m4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V7M-OVERRIDDEN %s
-// CHECK-V7M-OVERRIDDEN: "-cc1"{{.*}} "-triple" "thumbv7em-{{.*}} "-target-cpu" "cortex-m4"
 
+/// -arch is unsupported for non-Darwin targets.
+// RUN: not %clang --target=armv7m -arch armv7m -mcpu=cortex-m4 -### -c %s 2>&1 | FileCheck -check-prefix=ERR %s
+// ERR: unsupported option '-arch' for target 'armv7m'
diff --git a/clang/test/Frontend/darwin-eabi.c b/clang/test/Frontend/darwin-eabi.c
index 27471e6cfb0e6..9d62632891cbe 100644
--- a/clang/test/Frontend/darwin-eabi.c
+++ b/clang/test/Frontend/darwin-eabi.c
@@ -1,6 +1,6 @@
-// RUN: %clang -arch armv6m -dM -E %s | FileCheck %s
-// RUN: %clang -arch armv7m -dM -E %s | FileCheck %s
-// RUN: %clang -arch armv7em -dM -E %s | FileCheck %s
+// RUN: %clang --target=armv6m-apple-darwin -dM -E %s | FileCheck %s
+// RUN: %clang --target=armv7m-apple-darwin -dM -E %s | FileCheck %s
+// RUN: %clang --target=armv7em-apple-darwin -dM -E %s | FileCheck %s
 // RUN: %clang_cc1 -triple thumbv7m-apple-unknown-macho -dM -E %s | FileCheck %s
 
 // CHECK-NOT: __ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__

@MaskRay MaskRay merged commit 4e0275a into llvm:main Dec 5, 2023
5 checks passed
@MaskRay MaskRay deleted the driver-arch branch December 5, 2023 16:27
@kovdan01
Copy link
Contributor

kovdan01 commented Dec 6, 2023

@MaskRay Unfortunately, the case I was initially trying to fix still has a problem after applying this patch. Consider LLVM_DEFAULT_TARGET_TRIPLE=aarch64-linux-musl. In such a case, running clang -arch arm64e -c test.c -### will show us "-triple" "aarch64-unknown-linux-musl". As far as I understood from the comment #72821 (comment), a warning -Wunused-command-line-argument should be emitted - but it is not (this does not change even if we really compile to the object file instead of just printing cli args with -###).

Are we supposed to pass -target to clang in addition to -arch if we want to be sure to compile with Apple's triple? It might be reasonable, but in such case many tests which use -arch without -target actually might run with undesired non-Apple triple - I find such behavior a bit misleading. Is this "by design"?

@MaskRay
Copy link
Member Author

MaskRay commented Dec 6, 2023

When the target triple is aarch64 and -arch is specified, e.g. --target=aarch64 -arch arm64e, there is currently no warning/error, because clang/lib/Driver/ToolChains/Arch/AArch64.cpp claims OPT_arch in quite a few places.
We can safely remove these references now that https://reviews.llvm.org/D55731 has checked isOSDarwin (which covers macOS/iOS/watchOS/DriverKit).

Are we supposed to pass -target to clang in addition to -arch if we want to be sure to compile with Apple's triple?

If the default target triple is not Apple's triple, --target= is required. clang -arch arm64 without --target= does not work.
This is the desired behavior.

It might be reasonable, but in such case many tests which use -arch without -target actually might run with undesired non-Apple triple - I find such behavior a bit misleading. Is this "by design"?

These tests need to be fixed to specify --target= if it may run on a non-Apple target.
It's possible that -arch is redundant with --target=.

@kovdan01
Copy link
Contributor

kovdan01 commented Dec 6, 2023

OK, thanks for such a detailed explanation! Closing #72821 as not needed.

MaskRay added a commit that referenced this pull request Dec 6, 2023
See also #74365. clang/lib/Driver/ToolChains/Arch/AArch64.cpp
inappropriately claims OPT_arch, which will be removed by the follow-up
change.
bnbarham added a commit to apple/llvm-project that referenced this pull request May 30, 2024
Temporarily reverts commit 4e0275a to
unblock swift's rebranch build, which currently passes `-arch`
unconditionally.
bnbarham added a commit to apple/llvm-project that referenced this pull request May 30, 2024
Revert "[Driver] Mark -arch as TargetSpecific (llvm#74365)"
bnbarham added a commit to apple/llvm-project that referenced this pull request Jun 5, 2024
Temporarily reverts commit 4e0275a to
unblock swift's rebranch build, which currently passes `-arch`
unconditionally.

(cherry picked from commit 201e84d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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