-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[llvm-mca] Add -skip-unsupported-instructions option #89733
[llvm-mca] Add -skip-unsupported-instructions option #89733
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-tools-llvm-mca Author: Peter Waller (peterwaller-arm) ChangesPrior to this patch, if llvm-mca encountered an instruction which parses However, it would still be useful to allow MCA to continue even in the A user could previously have worked around such unsupported instructions Note that this behaviour of instructions with no scheduling information Testing the feature is a little awkward currently, it relies on an instruction Full diff: https://github.com/llvm/llvm-project/pull/89733.diff 4 Files Affected:
diff --git a/llvm/lib/MCA/InstrBuilder.cpp b/llvm/lib/MCA/InstrBuilder.cpp
index 1a82e45763a267..6261b208ff196f 100644
--- a/llvm/lib/MCA/InstrBuilder.cpp
+++ b/llvm/lib/MCA/InstrBuilder.cpp
@@ -542,7 +542,7 @@ InstrBuilder::createInstrDescImpl(const MCInst &MCI,
const MCSchedClassDesc &SCDesc = *SM.getSchedClassDesc(SchedClassID);
if (SCDesc.NumMicroOps == MCSchedClassDesc::InvalidNumMicroOps) {
return make_error<InstructionError<MCInst>>(
- "found an unsupported instruction in the input assembly sequence.",
+ "found an unsupported instruction in the input assembly sequence",
MCI);
}
diff --git a/llvm/test/tools/llvm-mca/AArch64/Neoverse/V2-skip-unsupported-inst.s b/llvm/test/tools/llvm-mca/AArch64/Neoverse/V2-skip-unsupported-inst.s
new file mode 100644
index 00000000000000..8c34dbe98cd352
--- /dev/null
+++ b/llvm/test/tools/llvm-mca/AArch64/Neoverse/V2-skip-unsupported-inst.s
@@ -0,0 +1,8 @@
+# RUN: llvm-mca -march=aarch64 -mcpu=neoverse-v2 -skip-unsupported-instructions %s |& FileCheck --check-prefix=CHECK-SKIP %s
+# RUN: not llvm-mca -march=aarch64 -mcpu=neoverse-v2 %s |& FileCheck --check-prefix=CHECK-ERROR %s
+
+# CHECK-SKIP: warning: found an unsupported instruction in the input assembly sequence, skipping with -skip-unsupported-instructions, note accuracy will be impacted:
+# CHECK-ERROR: error: found an unsupported instruction in the input assembly sequence, use -skip-unsupported-instructions to ignore.
+
+# Lacks scheduling information and is therefore unsupported by llvm-mca.
+steor x0, [x1]
diff --git a/llvm/tools/llvm-mca/CodeRegion.h b/llvm/tools/llvm-mca/CodeRegion.h
index ce107fd8f3b62e..952eb634b6a9dd 100644
--- a/llvm/tools/llvm-mca/CodeRegion.h
+++ b/llvm/tools/llvm-mca/CodeRegion.h
@@ -61,6 +61,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/MC/MCInst.h"
#include "llvm/MCA/CustomBehaviour.h"
@@ -97,6 +98,19 @@ class CodeRegion {
Instructions.emplace_back(Instruction);
}
+ // Remove the given instructions from the set, for unsupported instructions being skipped.
+ // Returns an ArrayRef for the updated vector of Instructions.
+ [[nodiscard]]
+ llvm::ArrayRef<llvm::MCInst> dropInstructions(const llvm::SmallPtrSetImpl<const llvm::MCInst*> &Insts) {
+ if (Insts.empty())
+ return Instructions;
+ Instructions.erase(std::remove_if(Instructions.begin(), Instructions.end(),
+ [&Insts](const llvm::MCInst &Inst) {
+ return Insts.contains(&Inst);
+ }), Instructions.end());
+ return Instructions;
+ }
+
llvm::SMLoc startLoc() const { return RangeStart; }
llvm::SMLoc endLoc() const { return RangeEnd; }
diff --git a/llvm/tools/llvm-mca/llvm-mca.cpp b/llvm/tools/llvm-mca/llvm-mca.cpp
index eb71cffba6dd22..891e578c9120e2 100644
--- a/llvm/tools/llvm-mca/llvm-mca.cpp
+++ b/llvm/tools/llvm-mca/llvm-mca.cpp
@@ -237,6 +237,11 @@ static cl::opt<bool> DisableInstrumentManager(
"ignores instruments.)."),
cl::cat(ViewOptions), cl::init(false));
+static cl::opt<bool> SkipUnsupportedInstructions(
+ "skip-unsupported-instructions",
+ cl::desc("Make unsupported instruction errors into warnings."),
+ cl::cat(ViewOptions), cl::init(false));
+
namespace {
const Target *getTarget(const char *ProgName) {
@@ -574,11 +579,11 @@ int main(int argc, char **argv) {
DenseMap<const MCInst *, SmallVector<mca::Instrument *>>
InstToInstruments;
SmallVector<std::unique_ptr<mca::Instruction>> LoweredSequence;
+ SmallPtrSet<const MCInst*, 16> DroppedInsts;
for (const MCInst &MCI : Insts) {
SMLoc Loc = MCI.getLoc();
const SmallVector<mca::Instrument *> Instruments =
InstrumentRegions.getActiveInstruments(Loc);
- InstToInstruments.insert({&MCI, Instruments});
Expected<std::unique_ptr<mca::Instruction>> Inst =
IB.createInstruction(MCI, Instruments);
@@ -588,7 +593,10 @@ int main(int argc, char **argv) {
[&IP, &STI](const mca::InstructionError<MCInst> &IE) {
std::string InstructionStr;
raw_string_ostream SS(InstructionStr);
- WithColor::error() << IE.Message << '\n';
+ if (SkipUnsupportedInstructions)
+ WithColor::warning() << IE.Message << ", skipping with -skip-unsupported-instructions, note accuracy will be impacted:\n";
+ else
+ WithColor::error() << IE.Message << ", use -skip-unsupported-instructions to ignore.";
IP->printInst(&IE.Inst, 0, "", *STI, SS);
SS.flush();
WithColor::note()
@@ -597,14 +605,20 @@ int main(int argc, char **argv) {
// Default case.
WithColor::error() << toString(std::move(NewE));
}
+ if (SkipUnsupportedInstructions) {
+ DroppedInsts.insert(&MCI);
+ continue;
+ }
return 1;
}
IPP->postProcessInstruction(Inst.get(), MCI);
-
+ InstToInstruments.insert({&MCI, Instruments});
LoweredSequence.emplace_back(std::move(Inst.get()));
}
+ Insts = Region->dropInstructions(DroppedInsts);
+
mca::CircularSourceMgr S(LoweredSequence,
PrintInstructionTables ? 1 : Iterations);
|
7c88785
to
c1fc189
Compare
Putting back into draft while I sort out what CI precommit caught. |
85d411e
to
9e23353
Compare
Fixed a few issues; I've waited a few hours for the windows builder to see if there are further issues but it seems there may be a while longer to wait. If it shows issues I'll address them tomorrow. |
llvm/test/tools/llvm-mca/AArch64/Neoverse/V2-skip-unsupported-inst.s
Outdated
Show resolved
Hide resolved
cd801c3
to
e452750
Compare
e452750
to
f2563d1
Compare
656d305
to
961b9a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Prior to this patch, if llvm-mca encountered an instruction which parses but has no scheduler info, the instruction is always reported as unsupported, and llvm-mca halts with an error. However, it would still be useful to allow MCA to continue even in the case of instructions lacking scheduling information. Obviously if scheduling information is lacking, it's not possible to give an accurate analysis for those instructions, and therefore a warning is emitted. A user could previously have worked around such unsupported instructions manually by deleting such instructions from the input, but this provides them a way of doing this for bulk inputs where they may not have a list of such unsupported instructions to drop up front. Note that this behaviour of instructions with no scheduling information under -skip-unsupported-instructions is analagous to current instructions which fail to parse: those are currently dropped from the input with a message printed, after which the analysis continues.
961b9a3
to
b5499a0
Compare
Prior to this patch, if llvm-mca encountered an instruction which parses
but has no scheduler info, the instruction is always reported as
unsupported, and llvm-mca halts with an error.
However, it would still be useful to allow MCA to continue even in the
case of instructions lacking scheduling information. Obviously if
scheduling information is lacking, it's not possible to give an accurate
analysis for those instructions, and therefore a warning is emitted.
A user could previously have worked around such unsupported instructions
manually by deleting such instructions from the input, but this provides
them a way of doing this for bulk inputs where they may not have a list
of such unsupported instructions to drop up front.
Note that this behaviour of instructions with no scheduling information
under -skip-unsupported-instructions is analagous to current
instructions which fail to parse: those are currently dropped from the
input with a message printed, after which the analysis continues.
Testing the feature is a little awkward currently, it relies on an instructionwhich is currently marked as unsupported, which may not remain so; should the
situation change it would be necessary to find an alternative unsupported
instruction or drop the test.
A test is added to check that analysis still reports an error if all instructions are removed from the input, to mirror the current behaviour of giving an error if no instructions are supplied.