Skip to content

Commit e2b515e

Browse files
peterwaller-armpeterwaller-arm
committed
[llvm-mca] Abort on parse error without -skip-unsupported-instructions
Prior to this patch, llvm-mca would continue executing after parse errors. These errors can lead to some confusion since some analysis results are printed on the standard output, and they're printed after the errors, which could otherwise be easy to miss. However it is still useful to be able to continue analysis after errors; so extend the recently added -skip-unsupported-instructions to support this. Two tests which have parse errors for some of the 'RUN' branches are updated to use -skip-unsupported-instructions so they can remain as-is. Add a description of -skip-unsupported-instructions to the llvm-mca command guide, and add it to the llvm-mca --help output: ``` --skip-unsupported-instructions=<value> - Force analysis to continue in the presence of unsupported instructions =none - Exit with an error when an instruction is unsupported for any reason (default) =lack-sched - Skip instructions on input which lack scheduling information =parse-failure - Skip lines on the input which fail to parse for any reason =any - Skip instructions or lines on input which are unsupported for any reason ``` Tests within this patch are intended to cover each of the cases. Reason | Flag | Comment --------------|------|------- none | none | Usual case, existing test suite lack-sched | none | Advises user to use -skip-unsupported-instructions=lack-sched, tested in llvm/test/tools/llvm-mca/X86/BtVer2/unsupported-instruction.s parse-failure | none | Advises user to use -skip-unsupported-instructions=parse-failure, tested in llvm/test/tools/llvm-mca/bad-input.s any | none | (N/A, covered above) lack-sched | any | Continues, prints warnings, tested in llvm/test/tools/llvm-mca/X86/BtVer2/unsupported-instruction.s parse-failure | any | Continues, prints errors, tested in llvm/test/tools/llvm-mca/bad-input.s lack-sched | parse-failure | Advises user to use -skip-unsupported-instructions=lack-sched, tested in llvm/test/tools/llvm-mca/X86/BtVer2/unsupported-instruction.s parse-failure | lack-sched | Advises user to use -skip-unsupported-instructions=parse-failure, tested in llvm/test/tools/llvm-mca/bad-input.s none | * | This would be any test case with skip-unsupported-instructions, coverage added in llvm/test/tools/llvm-mca/X86/BtVer2/simple-test.s any | * | (Logically covered by the other cases)
1 parent c32a4f8 commit e2b515e

11 files changed

+115
-31
lines changed

‎llvm/docs/CommandGuide/llvm-mca.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@ option specifies "``-``", then the output will also be sent to standard output.
234234
no extra information, and InstrumentManager never overrides the default
235235
schedule class for a given instruction.
236236

237+
.. option:: -skip-unsupported-instructions=<reason>
238+
239+
Force :program:`llvm-mca` to continue in the presence of instructions which do
240+
not parse or lack key scheduling information. Note that the resulting analysis
241+
is impacted since those unsupported instructions are ignored as-if they are
242+
not supplied as a part of the input.
243+
244+
The choice of `<reason>` controls the when mca will report an error.
245+
`<reason>` may be `none` (default), `lack-sched`, `parse-failure`, `any`.
246+
237247
EXIT STATUS
238248
-----------
239249

‎llvm/docs/ReleaseNotes.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,14 @@ Changes to the LLVM tools
211211
(`#89162 <https://github.com/llvm/llvm-project/pull/89162>`_)
212212
``--raw-relr`` has been removed.
213213

214+
* llvm-mca now aborts by default if it is given bad input where previously it
215+
would continue. Additionally, it can now continue when it encounters
216+
instructions which lack scheduling information. The behaviour can be
217+
controlled by the newly introduced
218+
`--skip-unsupported-instructions=<none|lack-sched|parse-failure|any>`, as
219+
documented in `--help` output and the command guide. (`#90474
220+
<https://github.com/llvm/llvm-project/pull/90474>`)
221+
214222
Changes to LLDB
215223
---------------------------------
216224

‎llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
2-
# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m3 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM3
2+
# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m3 -resource-pressure=false -skip-unsupported-instructions=parse-failure < %s | FileCheck %s -check-prefixes=ALL,EM3
33
# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m4 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM4
44
# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m5 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM5
55

‎llvm/test/tools/llvm-mca/AArch64/Exynos/float-integer.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
2-
# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m3 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM3
2+
# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m3 -resource-pressure=false -skip-unsupported-instructions=parse-failure < %s | FileCheck %s -check-prefixes=ALL,EM3
33
# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m4 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM4
44
# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m5 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM5
55

‎llvm/test/tools/llvm-mca/X86/BtVer2/simple-test.s

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
22
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=100 < %s | FileCheck %s
3+
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=100 -skip-unsupported-instructions=lack-sched < %s | FileCheck %s
4+
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=100 -skip-unsupported-instructions=parse-failure < %s | FileCheck %s
5+
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=100 -skip-unsupported-instructions=any < %s | FileCheck %s
36

47
add %edi, %eax
58

‎llvm/test/tools/llvm-mca/X86/BtVer2/skip-unsupported-instructions-none-remain.s

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -skip-unsupported-instructions %s 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK-SKIP %s
1+
# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -skip-unsupported-instructions=lack-sched %s 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK-SKIP %s
22
# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 %s 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK-ERROR %s
33

44
# Test defends that if all instructions are skipped leaving an empty input, an error is printed.
@@ -7,7 +7,7 @@ bzhi %eax, %ebx, %ecx
77

88
# CHECK-ALL-NOT: error
99

10-
# CHECK-ERROR: error: found an unsupported instruction in the input assembly sequence, use -skip-unsupported-instructions to ignore.
10+
# CHECK-ERROR: error: found an unsupported instruction in the input assembly sequence, use -skip-unsupported-instructions=lack-sched to ignore these on the input.
1111

1212
# CHECK-SKIP: warning: found an unsupported instruction in the input assembly sequence, skipping with -skip-unsupported-instructions, note accuracy will be impacted:
1313
# CHECK-SKIP: note: instruction: bzhil %eax, %ebx, %ecx

‎llvm/test/tools/llvm-mca/X86/BtVer2/unsupported-instruction.s

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -skip-unsupported-instructions -timeline %s 2>&1 | FileCheck --check-prefix=CHECK-SKIP %s
1+
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -skip-unsupported-instructions=any -timeline %s 2>&1 | FileCheck --check-prefix=CHECK-SKIP %s
2+
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -skip-unsupported-instructions=lack-sched -timeline %s 2>&1 | FileCheck --check-prefix=CHECK-SKIP %s
3+
# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -skip-unsupported-instructions=parse-failure -timeline %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
24
# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
35

4-
# Test checks that unsupported instructions exit with an error, unless -skip-unsupported-instructions is passed, in which case the remaining instructions should be analysed.
6+
# Test checks that unsupported instructions exit with an error, unless -skip-unsupported-instructions=lack-sched is passed, in which case the remaining instructions should be analysed.
7+
# Additionally check that -skip-unsupported-instructions=parse-failure continues to raise the lack of scheduling information.
58

69
# CHECK-SKIP: warning: found an unsupported instruction in the input assembly sequence, skipping with -skip-unsupported-instructions, note accuracy will be impacted:
7-
# CHECK-ERROR: error: found an unsupported instruction in the input assembly sequence, use -skip-unsupported-instructions to ignore.
10+
# CHECK-ERROR: error: found an unsupported instruction in the input assembly sequence, use -skip-unsupported-instructions=lack-sched to ignore these on the input.
811

912
bzhi %eax, %ebx, %ecx
1013

‎llvm/test/tools/llvm-mca/bad-input.s

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# RUN: not llvm-mca %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK %s
2+
# RUN: not llvm-mca -skip-unsupported-instructions=none %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK %s
3+
# RUN: not llvm-mca -skip-unsupported-instructions=lack-sched %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK %s
4+
# RUN: not llvm-mca -skip-unsupported-instructions=parse-failure %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK-SKIP %s
5+
# RUN: not llvm-mca -skip-unsupported-instructions=any %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK-SKIP %s
6+
7+
# Test checks that MCA does not produce a total cycles estimate if it encounters parse errors.
8+
9+
# CHECK-ALL-NOT: Total Cycles:
10+
11+
# CHECK: error: Assembly input parsing had errors, use -skip-unsupported-instructions=parse-failure to drop failing lines from the input.
12+
# CHECK-SKIP: error: no assembly instructions found.
13+
14+
This is not a valid assembly file for any architecture (by virtue of this text.)

‎llvm/tools/llvm-mca/CodeRegionGenerator.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace mca {
2929
CodeRegionGenerator::~CodeRegionGenerator() {}
3030

3131
Expected<const CodeRegions &> AsmCodeRegionGenerator::parseCodeRegions(
32-
const std::unique_ptr<MCInstPrinter> &IP) {
32+
const std::unique_ptr<MCInstPrinter> &IP, bool SkipFailures) {
3333
MCTargetOptions Opts;
3434
Opts.PreserveAsmComments = false;
3535
CodeRegions &Regions = getRegions();
@@ -61,7 +61,16 @@ Expected<const CodeRegions &> AsmCodeRegionGenerator::parseCodeRegions(
6161
"This target does not support assembly parsing.",
6262
inconvertibleErrorCode());
6363
Parser->setTargetParser(*TAP);
64-
Parser->Run(false);
64+
// Parser->Run() confusingly returns true on errors, in which case the errors
65+
// were already shown to the user. SkipFailures implies continuing in the
66+
// presence of any kind of failure within the parser, in which case failing
67+
// input lines are not represented, but the rest of the input remains.
68+
if (Parser->Run(false) && !SkipFailures) {
69+
const char *Message = "Assembly input parsing had errors, use "
70+
"-skip-unsupported-instructions=parse-failure "
71+
"to drop failing lines from the input.";
72+
return make_error<StringError>(Message, inconvertibleErrorCode());
73+
}
6574

6675
if (CCP->hadErr())
6776
return make_error<StringError>("There was an error parsing comments.",

‎llvm/tools/llvm-mca/CodeRegionGenerator.h

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ class CodeRegionGenerator {
148148
CodeRegionGenerator(const CodeRegionGenerator &) = delete;
149149
CodeRegionGenerator &operator=(const CodeRegionGenerator &) = delete;
150150
virtual Expected<const CodeRegions &>
151-
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) = 0;
151+
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP,
152+
bool SkipFailures) = 0;
152153

153154
public:
154155
CodeRegionGenerator() {}
@@ -164,7 +165,8 @@ class AnalysisRegionGenerator : public virtual CodeRegionGenerator {
164165
AnalysisRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {}
165166

166167
virtual Expected<const AnalysisRegions &>
167-
parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP) = 0;
168+
parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP,
169+
bool SkipFailures) = 0;
168170
};
169171

170172
/// Abstract CodeRegionGenerator with InstrumentRegionsRegions member
@@ -176,7 +178,8 @@ class InstrumentRegionGenerator : public virtual CodeRegionGenerator {
176178
InstrumentRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {}
177179

178180
virtual Expected<const InstrumentRegions &>
179-
parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP) = 0;
181+
parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP,
182+
bool SkipFailures) = 0;
180183
};
181184

182185
/// This abstract class is responsible for parsing input ASM and
@@ -202,7 +205,8 @@ class AsmCodeRegionGenerator : public virtual CodeRegionGenerator {
202205

203206
unsigned getAssemblerDialect() const { return AssemblerDialect; }
204207
Expected<const CodeRegions &>
205-
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override;
208+
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP,
209+
bool SkipFailures) override;
206210
};
207211

208212
class AsmAnalysisRegionGenerator final : public AnalysisRegionGenerator,
@@ -222,17 +226,20 @@ class AsmAnalysisRegionGenerator final : public AnalysisRegionGenerator,
222226
MCStreamerWrapper *getMCStreamer() override { return &Streamer; }
223227

224228
Expected<const AnalysisRegions &>
225-
parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
226-
Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP);
229+
parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP,
230+
bool SkipFailures) override {
231+
Expected<const CodeRegions &> RegionsOrErr =
232+
parseCodeRegions(IP, SkipFailures);
227233
if (!RegionsOrErr)
228234
return RegionsOrErr.takeError();
229235
else
230236
return static_cast<const AnalysisRegions &>(*RegionsOrErr);
231237
}
232238

233239
Expected<const CodeRegions &>
234-
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
235-
return AsmCodeRegionGenerator::parseCodeRegions(IP);
240+
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP,
241+
bool SkipFailures) override {
242+
return AsmCodeRegionGenerator::parseCodeRegions(IP, SkipFailures);
236243
}
237244
};
238245

@@ -254,17 +261,20 @@ class AsmInstrumentRegionGenerator final : public InstrumentRegionGenerator,
254261
MCStreamerWrapper *getMCStreamer() override { return &Streamer; }
255262

256263
Expected<const InstrumentRegions &>
257-
parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
258-
Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP);
264+
parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP,
265+
bool SkipFailures) override {
266+
Expected<const CodeRegions &> RegionsOrErr =
267+
parseCodeRegions(IP, SkipFailures);
259268
if (!RegionsOrErr)
260269
return RegionsOrErr.takeError();
261270
else
262271
return static_cast<const InstrumentRegions &>(*RegionsOrErr);
263272
}
264273

265274
Expected<const CodeRegions &>
266-
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
267-
return AsmCodeRegionGenerator::parseCodeRegions(IP);
275+
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP,
276+
bool SkipFailures) override {
277+
return AsmCodeRegionGenerator::parseCodeRegions(IP, SkipFailures);
268278
}
269279
};
270280

‎llvm/tools/llvm-mca/llvm-mca.cpp

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,35 @@ static cl::opt<unsigned>
135135
"(instructions per cycle)"),
136136
cl::cat(ToolOptions), cl::init(0));
137137

138+
enum class SkipType { NONE, LACK_SCHED, PARSE_FAILURE, ANY_FAILURE };
139+
140+
static cl::opt<enum SkipType> SkipUnsupportedInstructions(
141+
"skip-unsupported-instructions",
142+
cl::desc("Force analysis to continue in the presence of unsupported "
143+
"instructions"),
144+
cl::values(
145+
clEnumValN(SkipType::NONE, "none",
146+
"Exit with an error when an instruction is unsupported for "
147+
"any reason (default)"),
148+
clEnumValN(
149+
SkipType::LACK_SCHED, "lack-sched",
150+
"Skip instructions on input which lack scheduling information"),
151+
clEnumValN(
152+
SkipType::PARSE_FAILURE, "parse-failure",
153+
"Skip lines on the input which fail to parse for any reason"),
154+
clEnumValN(SkipType::ANY_FAILURE, "any",
155+
"Skip instructions or lines on input which are unsupported "
156+
"for any reason")),
157+
cl::init(SkipType::NONE), cl::cat(ViewOptions));
158+
159+
bool shouldSkip(enum SkipType skipType) {
160+
if (SkipUnsupportedInstructions == SkipType::NONE)
161+
return false;
162+
if (SkipUnsupportedInstructions == SkipType::ANY_FAILURE)
163+
return true;
164+
return skipType == SkipUnsupportedInstructions;
165+
}
166+
138167
static cl::opt<bool>
139168
PrintRegisterFileStats("register-file-stats",
140169
cl::desc("Print register file statistics"),
@@ -237,11 +266,6 @@ static cl::opt<bool> DisableInstrumentManager(
237266
"ignores instruments.)."),
238267
cl::cat(ViewOptions), cl::init(false));
239268

240-
static cl::opt<bool> SkipUnsupportedInstructions(
241-
"skip-unsupported-instructions",
242-
cl::desc("Make unsupported instruction errors into warnings."),
243-
cl::cat(ViewOptions), cl::init(false));
244-
245269
namespace {
246270

247271
const Target *getTarget(const char *ProgName) {
@@ -440,7 +464,8 @@ int main(int argc, char **argv) {
440464
mca::AsmAnalysisRegionGenerator CRG(*TheTarget, SrcMgr, ACtx, *MAI, *STI,
441465
*MCII);
442466
Expected<const mca::AnalysisRegions &> RegionsOrErr =
443-
CRG.parseAnalysisRegions(std::move(IPtemp));
467+
CRG.parseAnalysisRegions(std::move(IPtemp),
468+
shouldSkip(SkipType::PARSE_FAILURE));
444469
if (!RegionsOrErr) {
445470
if (auto Err =
446471
handleErrors(RegionsOrErr.takeError(), [](const StringError &E) {
@@ -482,7 +507,8 @@ int main(int argc, char **argv) {
482507
mca::AsmInstrumentRegionGenerator IRG(*TheTarget, SrcMgr, ICtx, *MAI, *STI,
483508
*MCII, *IM);
484509
Expected<const mca::InstrumentRegions &> InstrumentRegionsOrErr =
485-
IRG.parseInstrumentRegions(std::move(IPtemp));
510+
IRG.parseInstrumentRegions(std::move(IPtemp),
511+
shouldSkip(SkipType::PARSE_FAILURE));
486512
if (!InstrumentRegionsOrErr) {
487513
if (auto Err = handleErrors(InstrumentRegionsOrErr.takeError(),
488514
[](const StringError &E) {
@@ -593,15 +619,16 @@ int main(int argc, char **argv) {
593619
[&IP, &STI](const mca::InstructionError<MCInst> &IE) {
594620
std::string InstructionStr;
595621
raw_string_ostream SS(InstructionStr);
596-
if (SkipUnsupportedInstructions)
622+
if (shouldSkip(SkipType::LACK_SCHED))
597623
WithColor::warning()
598624
<< IE.Message
599625
<< ", skipping with -skip-unsupported-instructions, "
600626
"note accuracy will be impacted:\n";
601627
else
602628
WithColor::error()
603629
<< IE.Message
604-
<< ", use -skip-unsupported-instructions to ignore.\n";
630+
<< ", use -skip-unsupported-instructions=lack-sched to "
631+
"ignore these on the input.\n";
605632
IP->printInst(&IE.Inst, 0, "", *STI, SS);
606633
SS.flush();
607634
WithColor::note()
@@ -610,7 +637,7 @@ int main(int argc, char **argv) {
610637
// Default case.
611638
WithColor::error() << toString(std::move(NewE));
612639
}
613-
if (SkipUnsupportedInstructions) {
640+
if (shouldSkip(SkipType::LACK_SCHED)) {
614641
DroppedInsts.insert(&MCI);
615642
continue;
616643
}

0 commit comments

Comments
 (0)