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

[clang][Driver] Support passing arbitrary args to -cc1as with -Xclangas. #100714

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Jul 26, 2024

This is one of the ideas I gave in #97517. I opted to propose this one first as introducing a new -Xclangas option comes with no concerns about GCC compatibility as with -Xassembler.

Note: I don't have commit access.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jul 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 26, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Alex Rønne Petersen (alexrp)

Changes

This is one of the ideas I gave in #97517. I opted to propose this one first as introducing a new -Xclangas option comes with no concerns about GCC compatibility as with -Xassembler.

I am not sure where would be an appropriate place to test this, or how it would make sense to do so. I would appreciate any guidance on this. I tested manually like this:

./bin/clang -target arm-linux-gnueabi -c test.s
test.s:1:1: error: instruction requires: armv5t
blx lr
^make -j./bin/clang -target arm-linux-gnueabi -c test.s -Xclangas -target-feature -Xclangas +v5tfile test.o
test.o: ELF 32-bit LSB relocatable, ARM, EABI5 version 1 (SYSV), not stripped

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

3 Files Affected:

  • (modified) clang/docs/UsersManual.rst (+1)
  • (modified) clang/include/clang/Driver/Options.td (+8)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+6)
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index e9b95739ea2ab..7f55080321fea 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -4876,6 +4876,7 @@ Execute ``clang-cl /?`` to see a list of supported options:
       -v                      Show commands to run and use verbose output
       -W<warning>             Enable the specified warning
       -Xclang <arg>           Pass <arg> to the clang compiler
+      -Xclangas <arg>         Pass <arg> to the clang assembler
 
 The /clang: Option
 ^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 26811bf948ae5..c7839c3941eb9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1028,10 +1028,18 @@ def Xclang : Separate<["-"], "Xclang">,
   HelpText<"Pass <arg> to clang -cc1">, MetaVarName<"<arg>">,
   Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption]>,
   Group<CompileOnly_Group>;
+def Xclangas : Separate<["-"], "Xclangas">,
+  HelpText<"Pass <arg> to clang -cc1as">, MetaVarName<"<arg>">,
+  Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption]>,
+  Group<CompileOnly_Group>;
 def : Joined<["-"], "Xclang=">, Group<CompileOnly_Group>,
   Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption]>,
   Alias<Xclang>,
   HelpText<"Alias for -Xclang">, MetaVarName<"<arg>">;
+def : Joined<["-"], "Xclangas=">, Group<CompileOnly_Group>,
+  Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption]>,
+  Alias<Xclangas>,
+  HelpText<"Alias for -Xclangas">, MetaVarName<"<arg>">;
 def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
   HelpText<"Pass <arg> to fatbinary invocation">, MetaVarName<"<arg>">;
 def Xcuda_ptxas : Separate<["-"], "Xcuda-ptxas">,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4a94df7d5f42e..90b8db11c46c6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8711,6 +8711,12 @@ void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
   CollectArgsForIntegratedAssembler(C, Args, CmdArgs,
                                     getToolChain().getDriver());
 
+  // Forward -Xclangas arguments to -cc1as
+  for (auto Arg : Args.filtered(options::OPT_Xclangas)) {
+    Arg->claim();
+    CmdArgs.push_back(Arg->getValue());
+  }
+
   Args.AddAllArgs(CmdArgs, options::OPT_mllvm);
 
   if (DebugInfoKind > llvm::codegenoptions::NoDebugInfo && Output.isFilename())

@MaskRay
Copy link
Member

MaskRay commented Jul 26, 2024

I closed #52878 as working as intended. I think zig should specify a different -mcpu= or -march=. Sorry but I've closed the feature request #97517 as I don't think Clang can sign up addressing a potential surge of compatibility issues "please remove the incompatible target-feature when I specify -target-feature +X"

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.

3 participants