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

[flang][Driver] Add -masm option to flang #81490

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

DavidSpickett
Copy link
Collaborator

@DavidSpickett DavidSpickett commented Feb 12, 2024

The motivation here was a suggestion over in Compiler Explorer. You can use -mllvm already to do this but since gfortran supports -masm, I figured I'd try to add it.

This is done by flang expanding -masm into -mllvm x86-asm-syntax=, then passing that to fc1. Which then collects all the -mllvm options and forwards them on.

The code to expand it comes from clang Clang::AddX86TargetArgs (there are some other places doing the same thing too). However I've removed the -inline-asm that clang adds, as fortran doesn't have inline assembly.

So -masm for flang purely changes the style of assembly output.

$ ./bin/flang-new /tmp/test.f90 -o - -S -target x86_64-linux-gnu
<...>
        pushq   %rbp
$ ./bin/flang-new /tmp/test.f90 -o - -S -target x86_64-linux-gnu -masm=att
<...>
        pushq   %rbp
$ ./bin/flang-new /tmp/test.f90 -o - -S -target x86_64-linux-gnu -masm=intel
<...>
        push    rbp

The test is adapted from clang/test/Driver/masm.c by removing the clang-cl related lines and changing the 32 bit triples to 64 bit triples since flang doesn't support 32 bit targets.

The motivation here was a suggestion over in Compiler Explorer.
You can use `-mllvm` already to do this but since gfortran supports
`-masm`, I figured I'd try to add it.

This is done by flang expanding `-masm` into `-mllvm x86-asm-syntax=`,
then passing that to fc1. Which then collects all the `-mllvm` options
and forwards them on.

The code to expand it comes from clang `Clang::AddX86TargetArgs`
(there are some other places doing the same thing too). However I've
removed the `-inline-asm` that clang adds, as fortran doesn't have
inline assembly.

So -masm for flang purely changes the style of assembly output.

The test is adapted from `clang/test/Driver/masm.c` by removing the
clang-cl related lines and changing the 32 bit triples to 64 bit triples
since flang doesn't support 32 bit targets.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category labels Feb 12, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-flang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: David Spickett (DavidSpickett)

Changes

The motivation here was a suggestion over in Compiler Explorer. You can use -mllvm already to do this but since gfortran supports -masm, I figured I'd try to add it.

This is done by flang expanding -masm into -mllvm x86-asm-syntax=, then passing that to fc1. Which then collects all the -mllvm options and forwards them on.

The code to expand it comes from clang Clang::AddX86TargetArgs (there are some other places doing the same thing too). However I've removed the -inline-asm that clang adds, as fortran doesn't have inline assembly.

So -masm for flang purely changes the style of assembly output.

The test is adapted from clang/test/Driver/masm.c by removing the clang-cl related lines and changing the 32 bit triples to 64 bit triples since flang doesn't support 32 bit targets.


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

4 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+1-1)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+15)
  • (modified) clang/lib/Driver/ToolChains/Flang.h (+7)
  • (added) flang/test/Driver/masm.f90 (+13)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 31e8571758bfce..d5017b901d2906 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4414,7 +4414,7 @@ def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=
 def march_EQ : Joined<["-"], "march=">, Group<m_Group>,
   Flags<[TargetSpecific]>, Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
   HelpText<"For a list of available architectures for the target use '-mcpu=help'">;
-def masm_EQ : Joined<["-"], "masm=">, Group<m_Group>;
+def masm_EQ : Joined<["-"], "masm=">, Group<m_Group>, Visibility<[ClangOption, FlangOption]>;
 def inline_asm_EQ : Joined<["-"], "inline-asm=">, Group<m_Group>,
   Visibility<[ClangOption, CC1Option]>,
   Values<"att,intel">,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 23da08aa593f2d..6168b42dc78292 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -249,6 +249,20 @@ void Flang::AddRISCVTargetArgs(const ArgList &Args,
   }
 }
 
+void Flang::AddX86_64TargetArgs(const ArgList &Args,
+                                ArgStringList &CmdArgs) const {
+  if (Arg *A = Args.getLastArg(options::OPT_masm_EQ)) {
+    StringRef Value = A->getValue();
+    if (Value == "intel" || Value == "att") {
+      CmdArgs.push_back(Args.MakeArgString("-mllvm"));
+      CmdArgs.push_back(Args.MakeArgString("-x86-asm-syntax=" + Value));
+    } else {
+      getToolChain().getDriver().Diag(diag::err_drv_unsupported_option_argument)
+          << A->getSpelling() << Value;
+    }
+  }
+}
+
 static void addVSDefines(const ToolChain &TC, const ArgList &Args,
                          ArgStringList &CmdArgs) {
 
@@ -374,6 +388,7 @@ void Flang::addTargetOptions(const ArgList &Args,
     break;
   case llvm::Triple::x86_64:
     getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false);
+    AddX86_64TargetArgs(Args, CmdArgs);
     break;
   }
 
diff --git a/clang/lib/Driver/ToolChains/Flang.h b/clang/lib/Driver/ToolChains/Flang.h
index ec2e545a1d0b5c..9f5e26b8608324 100644
--- a/clang/lib/Driver/ToolChains/Flang.h
+++ b/clang/lib/Driver/ToolChains/Flang.h
@@ -77,6 +77,13 @@ class LLVM_LIBRARY_VISIBILITY Flang : public Tool {
   void AddRISCVTargetArgs(const llvm::opt::ArgList &Args,
                           llvm::opt::ArgStringList &CmdArgs) const;
 
+  /// Add specific options for X86_64 target.
+  ///
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void AddX86_64TargetArgs(const llvm::opt::ArgList &Args,
+                           llvm::opt::ArgStringList &CmdArgs) const;
+
   /// Extract offload options from the driver arguments and add them to
   /// the command arguments.
   /// \param [in] C The current compilation for the driver invocation
diff --git a/flang/test/Driver/masm.f90 b/flang/test/Driver/masm.f90
new file mode 100644
index 00000000000000..a119f02787eca9
--- /dev/null
+++ b/flang/test/Driver/masm.f90
@@ -0,0 +1,13 @@
+// RUN: %flang -target x86_64-unknown-linux -masm=intel -S %s -### 2>&1 | FileCheck --check-prefix=CHECK-INTEL %s
+// RUN: %flang -target x86_64-unknown-linux -masm=att -S %s -### 2>&1 | FileCheck --check-prefix=CHECK-ATT %s
+// RUN: not %flang --target=x86_64-unknown-linux -S -masm=somerequired %s -### 2>&1 | FileCheck --check-prefix=CHECK-SOMEREQUIRED %s
+// RUN: %flang -target aarch64-unknown-eabi -S -masm=intel %s -### 2>&1 | FileCheck --check-prefix=CHECK-AARCH64 %s
+
+// CHECK-INTEL: "-mllvm" "-x86-asm-syntax=intel"
+// CHECK-ATT: "-mllvm" "-x86-asm-syntax=att"
+// CHECK-SOMEREQUIRED: error: unsupported argument 'somerequired' to option '-masm='
+// CHECK-AARCH64: warning: argument unused during compilation: '-masm=intel'
+// CHECK-AARCH64-NOT: -x86-asm-syntax=intel
+
+integer function fn()
+end function fn

Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

flang/test/Driver/masm.f90 Outdated Show resolved Hide resolved
@DavidSpickett DavidSpickett merged commit 9ca1a15 into llvm:main Feb 13, 2024
4 checks passed
@DavidSpickett DavidSpickett deleted the flang-masm branch February 13, 2024 10:38
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 flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants