[Exegesis] CPU selection, when native arch and target mismatch #131014
Conversation
|
@llvm/pr-subscribers-tools-llvm-exegesis Author: None (AnastasiyaChernikova) ChangesFull diff: https://github.com/llvm/llvm-project/pull/131014.diff 2 Files Affected:
diff --git a/llvm/tools/llvm-exegesis/lib/LlvmState.cpp b/llvm/tools/llvm-exegesis/lib/LlvmState.cpp
index 9502cae993f67..af885a764124b 100644
--- a/llvm/tools/llvm-exegesis/lib/LlvmState.cpp
+++ b/llvm/tools/llvm-exegesis/lib/LlvmState.cpp
@@ -45,6 +45,12 @@ Expected<LLVMState> LLVMState::Create(std::string TripleName,
if (CpuName == "native")
CpuName = std::string(sys::getHostCPUName());
+ if (CpuName.empty()) {
+ std::unique_ptr<MCSubtargetInfo> Empty_STI(
+ TheTarget->createMCSubtargetInfo(TripleName, "", ""));
+ CpuName = Empty_STI->getAllProcessorDescriptions().begin()->Key;
+ }
+
std::unique_ptr<MCSubtargetInfo> STI(
TheTarget->createMCSubtargetInfo(TripleName, CpuName, ""));
assert(STI && "Unable to create subtarget info!");
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index babcffeb9666a..6a37637dcebee 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -44,6 +44,7 @@
#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/TargetParser/Host.h"
+#include "llvm/TargetParser/Triple.h"
#include <algorithm>
#include <string>
@@ -479,6 +480,10 @@ void benchmarkMain() {
#endif
}
+ // case for cross generating, when native arch and target mismatch
+ if (Triple(sys::getProcessTriple()).getArch() != Triple(TripleName).getArch())
+ MCPU = "";
+
InitializeAllExegesisTargets();
#define LLVM_EXEGESIS(TargetName) \
LLVMInitialize##TargetName##AsmPrinter(); \
|
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
0e3afaf to
5a27a96
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
5a27a96 to
b1b378b
Compare
|
At the moment, when building llvm-exegesis for RISC-V, the --mcpu flag must be specified. If this flag is omitted, the mcpu of the architecture of the device on which the assembly takes place (often ARM or x86) will be automatically set. To address this issue, I suggest making the following changes: if the launch line does not contain the mcpu when building for a different architecture, the first one from the list of mcpu for the target architecture will be automatically selected. |
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
b1b378b to
933df87
Compare
There was a problem hiding this comment.
At the moment, when building llvm-exegesis for RISC-V, the --mcpu flag must be specified. If this flag is omitted, the mcpu of the architecture of the device on which the assembly takes place (often ARM or x86) will be automatically set. To address this issue, I suggest making the following changes: if the launch line does not contain the mcpu when building for a different architecture, the first one from the list of mcpu for the target architecture will be automatically selected.
Is the first CPU in the list always a sensible choice? I think it would make more sense to just error out if the user species -mcpu=native when the target arch is set to something different than the host.
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
933df87 to
31d1c61
Compare
In this case, we are talking about a situation where the user does not set any mcpu. In this scenario, exegesis itself puts the mcpu in native. And already during the processing process, the basic mcpu of the architecture on which the assembly is taking place is substituted. I believe that when the mcpu is not set by the user, it is assumed that just the basic one will be used, as it is done in the case of non-cross compilation. That's the functionality I'm offering here. |
Right. I understand that part.
I'm not sure basing how What would be the drawback of simply erroring out if we detect cross compilation with |
It seems to me that the logic would be incorrect. In general, the mcpu flag is not required for exegesis assembly in the general case. Making this mandatory for cross-compilation seems strange to me. After all, the main idea of setting this flag is to choose a specific feature. And if mcpu is not specified, it means the user doesn't care about the implementation nuances |
|
I suggest to set default value for mcpu to be an empty string and move the logic of selecting default cpu to LlvmState.cpp: In case of cross-compilation select 'generic' by default and raise an error if user manually specified 'native'. Otherwise - in case of host compilation, set cpu to 'native' by default. @boomanaiden154 Would that be better? |
This still doesn't address the problem that I have with the current approach. If we are not cross compiling, we know the target CPU uarch because we can just query it. If we are cross compiling, then we do not know the target CPU. Maybe things outside of analysis mode will mostly work fine with a generic CPU, but I see no reason why we shouldn't require the user to specify an exact CPU (ie error out if |
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
31d1c61 to
ff1a11c
Compare
Okay, I agree. Replaced with error output. |
| if ((Triple(sys::getProcessTriple()).getArch() != | ||
| Triple(TripleName).getArch()) && | ||
| (MCPU == "native")) | ||
| ExitWithError("Incorrect cpu. To see all possible options use -mcpu=help"); |
There was a problem hiding this comment.
Can we be a bit more descriptive here? Maybe something like A CPU must be explicitly specified when cross compiling. To see all possible options use -mcpu=help?
There was a problem hiding this comment.
Addressed
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
ff1a11c to
186c3bd
Compare
llvm/test/tools/llvm-exegesis/RISCV/mcpu_not_set_during_cross_compilation.s
Outdated
Show resolved
Hide resolved
AnastasiyaChernikova
force-pushed
the
default_cpu
branch
from
186c3bd to
3d311f4
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/15396 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/144/builds/21316 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/15399 Here is the relevant piece of the build log for the reference |
|
@AnastasiyaChernikova the test you added is failing on several bots. I believe the problem is you are assuming the riscv target is being built, and the test fails on bots where it is not being built (at least for my bots). Can you take a look and fix or revert if you need time to investigate? |
|
1876a89 should fix that. |
No description provided.