[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
https://github.com/AlexisPerry created
https://github.com/llvm/llvm-project/pull/96688
This aims to reland the changes and fix the broken test from PR
https://github.com/llvm/llvm-project/pull/95043
>From 2312d31b14aecc6eeea2e81d221ee004e5de3efc Mon Sep 17 00:00:00 2001
From: Alexis Perry-Holby
Date: Thu, 6 Jun 2024 14:02:52 -0600
Subject: [PATCH 01/12] [flang] Add basic -mtune support
---
clang/include/clang/Driver/Options.td | 7 ---
clang/lib/Driver/ToolChains/Flang.cpp | 8
flang/include/flang/Frontend/TargetOptions.h | 3 +++
flang/include/flang/Lower/Bridge.h| 6 +++---
.../flang/Optimizer/CodeGen/CGPasses.td | 4
.../include/flang/Optimizer/CodeGen/Target.h | 19 ++-
.../Optimizer/Dialect/Support/FIRContext.h| 7 +++
.../flang/Optimizer/Transforms/Passes.td | 3 +++
flang/lib/Frontend/CompilerInvocation.cpp | 4
flang/lib/Frontend/FrontendActions.cpp| 3 ++-
flang/lib/Lower/Bridge.cpp| 3 ++-
flang/lib/Optimizer/CodeGen/CodeGen.cpp | 6 +-
flang/lib/Optimizer/CodeGen/Target.cpp| 11 +++
flang/lib/Optimizer/CodeGen/TargetRewrite.cpp | 12 +++-
flang/lib/Optimizer/CodeGen/TypeConverter.cpp | 3 ++-
.../Optimizer/Dialect/Support/FIRContext.cpp | 18 ++
flang/tools/bbc/bbc.cpp | 2 +-
flang/tools/tco/tco.cpp | 4
flang/unittests/Optimizer/FIRContextTest.cpp | 3 +++
.../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 9 +
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 1 +
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 5 +
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 3 +++
23 files changed, 131 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index d44faa55c456f..b81f480e1ed2b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5403,6 +5403,7 @@ def module_file_info : Flag<["-"], "module-file-info">,
Flags<[]>,
HelpText<"Provide information about a particular module file">;
def mthumb : Flag<["-"], "mthumb">, Group;
def mtune_EQ : Joined<["-"], "mtune=">, Group,
+ Visibility<[ClangOption, FlangOption]>,
HelpText<"Only supported on AArch64, PowerPC, RISC-V, SPARC, SystemZ, and
X86">;
def multi__module : Flag<["-"], "multi_module">;
def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
@@ -6722,9 +6723,6 @@ def emit_hlfir : Flag<["-"], "emit-hlfir">,
Group,
let Visibility = [CC1Option, CC1AsOption] in {
-def tune_cpu : Separate<["-"], "tune-cpu">,
- HelpText<"Tune for a specific cpu type">,
- MarshallingInfoString>;
def target_abi : Separate<["-"], "target-abi">,
HelpText<"Target a particular ABI type">,
MarshallingInfoString>;
@@ -6751,6 +6749,9 @@ def darwin_target_variant_triple : Separate<["-"],
"darwin-target-variant-triple
let Visibility = [CC1Option, CC1AsOption, FC1Option] in {
+def tune_cpu : Separate<["-"], "tune-cpu">,
+ HelpText<"Tune for a specific cpu type">,
+ MarshallingInfoString>;
def target_cpu : Separate<["-"], "target-cpu">,
HelpText<"Target a specific cpu type">,
MarshallingInfoString>;
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index 42b45dba2bd31..3dc7ee0ea2bff 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -17,6 +17,7 @@
#include "llvm/Support/Path.h"
#include "llvm/TargetParser/RISCVISAInfo.h"
#include "llvm/TargetParser/RISCVTargetParser.h"
+#include "llvm/TargetParser/Host.h"
#include
@@ -411,6 +412,13 @@ void Flang::addTargetOptions(const ArgList &Args,
}
// TODO: Add target specific flags, ABI, mtune option etc.
+ if (const Arg *A = Args.getLastArg(options::OPT_mtune_EQ)) {
+CmdArgs.push_back("-tune-cpu");
+if (strcmp(A->getValue(), "native") == 0)
+ CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
+else
+ CmdArgs.push_back(A->getValue());
+ }
}
void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs,
diff --git a/flang/include/flang/Frontend/TargetOptions.h
b/flang/include/flang/Frontend/TargetOptions.h
index ef5d270a2185d..a7a7192c55cb1 100644
--- a/flang/include/flang/Frontend/TargetOptions.h
+++ b/flang/include/flang/Frontend/TargetOptions.h
@@ -32,6 +32,9 @@ class TargetOptions {
/// If given, the name of the target CPU to generate code for.
std::string cpu;
+ /// If given, the name of the target CPU to tune code for.
+ std::string tuneCPU;
+
/// The list of target specific features to enable or disable, as written on
/// the command line.
std::vector featuresAsWritten;
diff --git a/flang/include/flang/Lower/Bridge.h
b/flang/include/flang/Lower/Bridge.h
index 52110b861b680..4379ed512cdf0 100644
--- a/flang/include/flang/Lower/Brid
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s gysit wrote: Should this test also be guarded with `REQUIRES: x86-registered-target`? https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,7 @@ +// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s gysit wrote: This may require x86-registered-target as well? https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s AlexisPerry wrote: Thank you so much for the catch! Yes, this guard is absolutely needed. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,7 @@ +// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s AlexisPerry wrote: Thanks again! Yes, you are absolutely correct. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
https://github.com/AlexisPerry updated
https://github.com/llvm/llvm-project/pull/96688
>From 2312d31b14aecc6eeea2e81d221ee004e5de3efc Mon Sep 17 00:00:00 2001
From: Alexis Perry-Holby
Date: Thu, 6 Jun 2024 14:02:52 -0600
Subject: [PATCH 01/13] [flang] Add basic -mtune support
---
clang/include/clang/Driver/Options.td | 7 ---
clang/lib/Driver/ToolChains/Flang.cpp | 8
flang/include/flang/Frontend/TargetOptions.h | 3 +++
flang/include/flang/Lower/Bridge.h| 6 +++---
.../flang/Optimizer/CodeGen/CGPasses.td | 4
.../include/flang/Optimizer/CodeGen/Target.h | 19 ++-
.../Optimizer/Dialect/Support/FIRContext.h| 7 +++
.../flang/Optimizer/Transforms/Passes.td | 3 +++
flang/lib/Frontend/CompilerInvocation.cpp | 4
flang/lib/Frontend/FrontendActions.cpp| 3 ++-
flang/lib/Lower/Bridge.cpp| 3 ++-
flang/lib/Optimizer/CodeGen/CodeGen.cpp | 6 +-
flang/lib/Optimizer/CodeGen/Target.cpp| 11 +++
flang/lib/Optimizer/CodeGen/TargetRewrite.cpp | 12 +++-
flang/lib/Optimizer/CodeGen/TypeConverter.cpp | 3 ++-
.../Optimizer/Dialect/Support/FIRContext.cpp | 18 ++
flang/tools/bbc/bbc.cpp | 2 +-
flang/tools/tco/tco.cpp | 4
flang/unittests/Optimizer/FIRContextTest.cpp | 3 +++
.../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 9 +
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 1 +
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 5 +
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 3 +++
23 files changed, 131 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index d44faa55c456f..b81f480e1ed2b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5403,6 +5403,7 @@ def module_file_info : Flag<["-"], "module-file-info">,
Flags<[]>,
HelpText<"Provide information about a particular module file">;
def mthumb : Flag<["-"], "mthumb">, Group;
def mtune_EQ : Joined<["-"], "mtune=">, Group,
+ Visibility<[ClangOption, FlangOption]>,
HelpText<"Only supported on AArch64, PowerPC, RISC-V, SPARC, SystemZ, and
X86">;
def multi__module : Flag<["-"], "multi_module">;
def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
@@ -6722,9 +6723,6 @@ def emit_hlfir : Flag<["-"], "emit-hlfir">,
Group,
let Visibility = [CC1Option, CC1AsOption] in {
-def tune_cpu : Separate<["-"], "tune-cpu">,
- HelpText<"Tune for a specific cpu type">,
- MarshallingInfoString>;
def target_abi : Separate<["-"], "target-abi">,
HelpText<"Target a particular ABI type">,
MarshallingInfoString>;
@@ -6751,6 +6749,9 @@ def darwin_target_variant_triple : Separate<["-"],
"darwin-target-variant-triple
let Visibility = [CC1Option, CC1AsOption, FC1Option] in {
+def tune_cpu : Separate<["-"], "tune-cpu">,
+ HelpText<"Tune for a specific cpu type">,
+ MarshallingInfoString>;
def target_cpu : Separate<["-"], "target-cpu">,
HelpText<"Target a specific cpu type">,
MarshallingInfoString>;
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index 42b45dba2bd31..3dc7ee0ea2bff 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -17,6 +17,7 @@
#include "llvm/Support/Path.h"
#include "llvm/TargetParser/RISCVISAInfo.h"
#include "llvm/TargetParser/RISCVTargetParser.h"
+#include "llvm/TargetParser/Host.h"
#include
@@ -411,6 +412,13 @@ void Flang::addTargetOptions(const ArgList &Args,
}
// TODO: Add target specific flags, ABI, mtune option etc.
+ if (const Arg *A = Args.getLastArg(options::OPT_mtune_EQ)) {
+CmdArgs.push_back("-tune-cpu");
+if (strcmp(A->getValue(), "native") == 0)
+ CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
+else
+ CmdArgs.push_back(A->getValue());
+ }
}
void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs,
diff --git a/flang/include/flang/Frontend/TargetOptions.h
b/flang/include/flang/Frontend/TargetOptions.h
index ef5d270a2185d..a7a7192c55cb1 100644
--- a/flang/include/flang/Frontend/TargetOptions.h
+++ b/flang/include/flang/Frontend/TargetOptions.h
@@ -32,6 +32,9 @@ class TargetOptions {
/// If given, the name of the target CPU to generate code for.
std::string cpu;
+ /// If given, the name of the target CPU to tune code for.
+ std::string tuneCPU;
+
/// The list of target specific features to enable or disable, as written on
/// the command line.
std::vector featuresAsWritten;
diff --git a/flang/include/flang/Lower/Bridge.h
b/flang/include/flang/Lower/Bridge.h
index 52110b861b680..4379ed512cdf0 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -65,11 +65,11 @@ class LoweringBridge {
const Fortran::
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s banach-space wrote: Hm, ninja `check-mlir` worked for me just fine on Aarch64 đ€ I think that `mlir-translate` doesn't really care :) Put differently, `REQUIRES` is not needed here. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s gysit wrote: Interesting! Did you build your LLVM with the x86 target enabled (besides Aarch64)? If not, `mlir-translate` may indeed be fine with the attribute independent of the target architecture. Note that `mlir-translate` runs `llvm::verifyModule` which I thought could be unhappy with unsupported target specific attributes. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s AlexisPerry wrote: I ran into failures if I didn't include the guard. I have separate builds for x86 and Aarch64. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s banach-space wrote: I have X86 disabled - I just double checked. And yes, this test works for me just fine without the guard. Alexis, what error do you get when it fails for you? To me, it would make sense if `mlir-translate` didn't care - it doesn't have to interpret these attributes, does it? Btw @AlexisPerry , there's no need to clone "tune-cpu.f90" as [tune-cpu-llvm-aarch.f90](https://github.com/llvm/llvm-project/pull/96688/files#diff-879569867a123346813864ce36bc15249acb8b57bc335c808eb4d30818ec8685). Instead, you can use conditional RUN lines like here: * https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/pic-flags.f90 https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s AlexisPerry wrote: I have been trying to reproduce my error ever since saying I had one and have been unsuccessful. It now appears that mlir-translate doesn't care about the target. @banach-space That is a much better way of doing things. Thank you for the suggestion and I will make the changes. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s gysit wrote: > To me, it would make sense if mlir-translate didn't care - it doesn't have to > interpret these attributes, does it? It doesn't do anything with them except for running the LLVM verifier. If things pass on Aarch64, then the guards are not necessary. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
