Skip to content

Commit 1b35040

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 1b35040

File tree

10 files changed

+103
-31
lines changed

10 files changed

+103
-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/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: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ 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,
33+
bool SkipFailures) {
3334
MCTargetOptions Opts;
3435
Opts.PreserveAsmComments = false;
3536
CodeRegions &Regions = getRegions();
@@ -61,7 +62,16 @@ Expected<const CodeRegions &> AsmCodeRegionGenerator::parseCodeRegions(
6162
"This target does not support assembly parsing.",
6263
inconvertibleErrorCode());
6364
Parser->setTargetParser(*TAP);
64-
Parser->Run(false);
65+
// Parser->Run() confusingly returns true on errors, in which case the errors
66+
// were already shown to the user. SkipFailures implies continuing in the
67+
// presence of any kind of failure within the parser, in which case failing
68+
// input lines are not represented, but the rest of the input remains.
69+
if (Parser->Run(false) && !SkipFailures) {
70+
const char *Message = "Assembly input parsing had errors, use "
71+
"-skip-unsupported-instructions=parse-failure "
72+
"to drop failing lines from the input.";
73+
return make_error<StringError>(Message, inconvertibleErrorCode());
74+
}
6575

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

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

Lines changed: 24 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,21 @@ 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(
243+
IP, SkipFailures);
236244
}
237245
};
238246

@@ -254,17 +262,21 @@ class AsmInstrumentRegionGenerator final : public InstrumentRegionGenerator,
254262
MCStreamerWrapper *getMCStreamer() override { return &Streamer; }
255263

256264
Expected<const InstrumentRegions &>
257-
parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
258-
Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP);
265+
parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP,
266+
bool SkipFailures) override {
267+
Expected<const CodeRegions &> RegionsOrErr =
268+
parseCodeRegions(IP, SkipFailures);
259269
if (!RegionsOrErr)
260270
return RegionsOrErr.takeError();
261271
else
262272
return static_cast<const InstrumentRegions &>(*RegionsOrErr);
263273
}
264274

265275
Expected<const CodeRegions &>
266-
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
267-
return AsmCodeRegionGenerator::parseCodeRegions(IP);
276+
parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP,
277+
bool SkipFailures) override {
278+
return AsmCodeRegionGenerator::parseCodeRegions(
279+
IP, SkipFailures);
268280
}
269281
};
270282

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

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

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

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-
245264
namespace {
246265

247266
const Target *getTarget(const char *ProgName) {
@@ -440,7 +459,7 @@ int main(int argc, char **argv) {
440459
mca::AsmAnalysisRegionGenerator CRG(*TheTarget, SrcMgr, ACtx, *MAI, *STI,
441460
*MCII);
442461
Expected<const mca::AnalysisRegions &> RegionsOrErr =
443-
CRG.parseAnalysisRegions(std::move(IPtemp));
462+
CRG.parseAnalysisRegions(std::move(IPtemp), shouldSkip(SkipType::PARSE_FAILURE));
444463
if (!RegionsOrErr) {
445464
if (auto Err =
446465
handleErrors(RegionsOrErr.takeError(), [](const StringError &E) {
@@ -482,7 +501,8 @@ int main(int argc, char **argv) {
482501
mca::AsmInstrumentRegionGenerator IRG(*TheTarget, SrcMgr, ICtx, *MAI, *STI,
483502
*MCII, *IM);
484503
Expected<const mca::InstrumentRegions &> InstrumentRegionsOrErr =
485-
IRG.parseInstrumentRegions(std::move(IPtemp));
504+
IRG.parseInstrumentRegions(std::move(IPtemp),
505+
shouldSkip(SkipType::PARSE_FAILURE));
486506
if (!InstrumentRegionsOrErr) {
487507
if (auto Err = handleErrors(InstrumentRegionsOrErr.takeError(),
488508
[](const StringError &E) {
@@ -593,15 +613,15 @@ int main(int argc, char **argv) {
593613
[&IP, &STI](const mca::InstructionError<MCInst> &IE) {
594614
std::string InstructionStr;
595615
raw_string_ostream SS(InstructionStr);
596-
if (SkipUnsupportedInstructions)
616+
if (shouldSkip(SkipType::LACK_SCHED))
597617
WithColor::warning()
598618
<< IE.Message
599619
<< ", skipping with -skip-unsupported-instructions, "
600620
"note accuracy will be impacted:\n";
601621
else
602622
WithColor::error()
603623
<< IE.Message
604-
<< ", use -skip-unsupported-instructions to ignore.\n";
624+
<< ", use -skip-unsupported-instructions=lack-sched to ignore these on the input.\n";
605625
IP->printInst(&IE.Inst, 0, "", *STI, SS);
606626
SS.flush();
607627
WithColor::note()
@@ -610,7 +630,7 @@ int main(int argc, char **argv) {
610630
// Default case.
611631
WithColor::error() << toString(std::move(NewE));
612632
}
613-
if (SkipUnsupportedInstructions) {
633+
if (shouldSkip(SkipType::LACK_SCHED)) {
614634
DroppedInsts.insert(&MCI);
615635
continue;
616636
}

0 commit comments

Comments
 (0)