[MC] Add support for -mcpu=native. #159414
Conversation
Support -mcpu=native by querying the Host CPU Name.
|
@llvm/pr-subscribers-llvm-mc Author: Cameron McInally (mcinally) ChangesSupport -mcpu=native by querying the Host CPU Name. Full diff: https://github.com/llvm/llvm-project/pull/159414.diff 2 Files Affected:
diff --git a/llvm/test/MC/AsmParser/native.s b/llvm/test/MC/AsmParser/native.s
new file mode 100644
index 0000000000000..b5228dfd77396
--- /dev/null
+++ b/llvm/test/MC/AsmParser/native.s
@@ -0,0 +1,3 @@
+# RUN: llvm-mc -filetype=obj -mcpu=native %s 2>&1 | FileCheck %s
+
+# CHECK-NOT: 'native' is not a recognized processor for this target (ignoring processor)
diff --git a/llvm/tools/llvm-mc/llvm-mc.cpp b/llvm/tools/llvm-mc/llvm-mc.cpp
index 136cd69526a3c..cb38a04f249e1 100644
--- a/llvm/tools/llvm-mc/llvm-mc.cpp
+++ b/llvm/tools/llvm-mc/llvm-mc.cpp
@@ -467,6 +467,10 @@ int main(int argc, char **argv) {
FeaturesStr = Features.getString();
}
+ // Replace -mcpu=native with Host CPU.
+ if (MCPU == "native")
+ MCPU = std::string(llvm::sys::getHostCPUName());
+
std::unique_ptr<MCSubtargetInfo> STI(
TheTarget->createMCSubtargetInfo(TheTriple, MCPU, FeaturesStr));
assert(STI && "Unable to create subtarget info!");
|
| // Replace -mcpu=native with Host CPU. | ||
| if (MCPU == "native") | ||
| MCPU = std::string(llvm::sys::getHostCPUName()); | ||
|
|
There was a problem hiding this comment.
Do we need to pass call getHostCPUFeatures too and pass that information to MAttr?
I know on X86, getHostCPUName can return "haswell" on Pentium CPUs that don't support AVX2, but use the haswell microarchitecture. "haswell" implies AVX2. getHostCPUFeatures will return -avx2 for these CPUs.
There was a problem hiding this comment.
Hm, that's a good point.
It doesn't look like X86 supports -mcpu= in Clang.
I do see that -march= is supported, but Clang just maps to llvm::sys::getHostCPUName(), like this patch, but with the obvious FIXME.
clang/lib/Driver/ToolChains/Arch/X86.cpp:
std::string x86::getX86TargetCPU(const Driver &D, const ArgList &Args,
const llvm::Triple &Triple) {
if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
StringRef CPU = A->getValue();
if (CPU != "native")
return std::string(CPU);
// FIXME: Reject attempts to use -march=native unless the target matches
// the host.
CPU = llvm::sys::getHostCPUName();
if (!CPU.empty() && CPU != "generic")
return std::string(CPU);
I don't have a strong opinion here, but do have a strong opinion that MC needs to support -mcpu=native in some way. So whatever you suggest is fine with me.
There was a problem hiding this comment.
Clang does use getHostCPUFeatures with -mcpu=native. I wrote the code originally. It's here.
void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args,
std::vector<StringRef> &Features) {
// Claim and report unsupported -mabi=. Note: we don't support "sysv_abi" or
// "ms_abi" as default function attributes.
if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_mabi_EQ)) {
StringRef DefaultAbi =
(Triple.isOSWindows() || Triple.isUEFI()) ? "ms" : "sysv";
if (A->getValue() != DefaultAbi)
D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getSpelling() << Triple.getTriple();
}
// If -march=native, autodetect the feature list.
if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
if (StringRef(A->getValue()) == "native") {
for (auto &F : llvm::sys::getHostCPUFeatures())
Features.push_back(
Args.MakeArgString((F.second ? "+" : "-") + F.first()));
}
}
There was a problem hiding this comment.
I do see that -march= is supported, but Clang just maps to llvm::sys::getHostCPUName(), like this patch, but with the obvious FIXME.
I think that fixme is maybe saying we shouldn't call getHostCPU if you explicitly pass --target=x86_64 on a non-x86 machine?
There was a problem hiding this comment.
Clang does use getHostCPUFeatures with -mcpu=native. I wrote the code originally. It's here.
That's -march now. Maybe changed when -mcpu was deprecated on X86.
$ clang test.c -mcpu=native
clang: error: unsupported option '-mcpu=' for target 'x86_64-unknown-linux-gnu'
There was a problem hiding this comment.
Clang does use getHostCPUFeatures with -mcpu=native. I wrote the code originally. It's here.
That's
-marchnow. Maybe changed when-mcpuwas deprecated on X86.$ clang test.c -mcpu=native clang: error: unsupported option '-mcpu=' for target 'x86_64-unknown-linux-gnu'
Sorry I meant -march=native. For all intents and purpose -march on X86 is -mcpu. I've been on RISC-V for too long now so I my brain thinks -mcpu.
There was a problem hiding this comment.
Latest patch addresses this. I think it's best to allow the command line -mattr's to override the default with native, but I don't feel strongly about it. Thoughts?
There was a problem hiding this comment.
Latest patch addresses this. I think it's best to allow the command line -mattr's to override the default with native, but I don't feel strongly about it. Thoughts?
I agree.
llvm/test/MC/AsmParser/native.s
Outdated
| @@ -0,0 +1,3 @@ | |||
| # RUN: llvm-mc -filetype=obj -mcpu=native %s 2>&1 | FileCheck %s | |||
|
|
|||
| # CHECK-NOT: 'native' is not a recognized processor for this target (ignoring processor) | |||
There was a problem hiding this comment.
This is too specific for a not check. Better to verify the output is empty
There was a problem hiding this comment.
Looks like FileCheck doesn't like an empty input file (really stderr piped to stdout):
FileCheck error: '<stdin>' is empty.
I could make the CHECK-NOT more narrow. Maybe something like:
# CHECK-NOT: native
Does that seem acceptable, @arsenm?
There was a problem hiding this comment.
I know there's some trick to do this. I ran into this recently but forget what I did
There was a problem hiding this comment.
I guess checking native works though
There was a problem hiding this comment.
Updated in latest patch.
Also update negative check to look for no stderr output.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvm::sys::getHostCPUName() and llvm::sys::getHostCPUFeatures() appear to need special cases for _WIN32 on X86 (similar to the existing _WIN32 on ARM special cases) to restrict what CPUs and Features are available with that OS. For now, limit this test to Linux systems only, as this test does not check very much being that it is sensitve to the Host machine. Originally llvm#159414.
|
I started seeing this error on a downstream CI: My downstream target is not X86, so -mcpu=native can't work. |
Please help me understand what you're seeing. So you're cross-compiling for your target on a Could we check |
|
The thinking there is that if we're cross-compiling, then we shouldn't be using -mcpu=native anyway. |
That's correct. I'm building a toolchain on an x86 machine, that will run on an x86 machine, and that will target a non-x86 architecture by default (unless overridden by
I'm not sure. What is I don't think there is generic solution and we will have to specialize the test for some target(s). E.g., |
Yes, but that's what the test currently does :) |
Yup, let's do that. I notice that Clang does similar for a |
It's not possible to use `-mcpu=native` when the Target's Triple doesn't match the Host's. Move this test to the AArch64 directory so that it isn't run while cross-compiling. Originally llvm#159414
It's not possible to use `-mcpu=native` when the Target's Triple doesn't match the Host's. Move this test to the X86 directory so that it isn't run while cross-compiling. Originally #159414 --------- Co-authored-by: Cameron McInally <cmcinally@nvidia.com>
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/25367 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/25227 Here is the relevant piece of the build log for the reference |
Support -mcpu=native by querying the Host CPU Name.