[llvm] [llvm-mca] Abort on parse error without -skip-unsupported-instructions (PR #90474)
Peter Waller via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 29 06:57:54 PDT 2024
https://github.com/peterwaller-arm created https://github.com/llvm/llvm-project/pull/90474
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.
>From 0f8416d4596b8385076e678231bfaf0e16c1f8c8 Mon Sep 17 00:00:00 2001
From: Peter Waller <peter.waller at arm.com>
Date: Tue, 23 Apr 2024 14:46:41 +0100
Subject: [PATCH] [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.
---
.../AArch64/Exynos/float-divide-multiply.s | 2 +-
.../llvm-mca/AArch64/Exynos/float-integer.s | 2 +-
llvm/test/tools/llvm-mca/bad-input.test | 11 +++++++++
llvm/tools/llvm-mca/CodeRegionGenerator.cpp | 6 +++--
llvm/tools/llvm-mca/CodeRegionGenerator.h | 24 +++++++++----------
llvm/tools/llvm-mca/llvm-mca.cpp | 4 ++--
6 files changed, 31 insertions(+), 18 deletions(-)
create mode 100644 llvm/test/tools/llvm-mca/bad-input.test
diff --git a/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s b/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s
index ecfd019452afcd..b726933f1b5573 100644
--- a/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s
+++ b/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s
@@ -1,5 +1,5 @@
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
-# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m3 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM3
+# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m3 -resource-pressure=false -skip-unsupported-instructions < %s | FileCheck %s -check-prefixes=ALL,EM3
# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m4 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM4
# RUN: llvm-mca -march=aarch64 -mcpu=exynos-m5 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM5
diff --git a/llvm/test/tools/llvm-mca/AArch64/Exynos/float-integer.s b/llvm/test/tools/llvm-mca/AArch64/Exynos/float-integer.s
index 16c710553f754a..46baecead72bee 100644
--- a/llvm/test/tools/llvm-mca/AArch64/Exynos/float-integer.s
+++ b/llvm/test/tools/llvm-mca/AArch64/Exynos/float-integer.s
@@ -1,5 +1,5 @@
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
-# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m3 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM3
+# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m3 -resource-pressure=false -skip-unsupported-instructions < %s | FileCheck %s -check-prefixes=ALL,EM3
# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m4 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM4
# RUN: llvm-mca -mtriple=aarch64-linux-gnu -mcpu=exynos-m5 -resource-pressure=false < %s | FileCheck %s -check-prefixes=ALL,EM5
diff --git a/llvm/test/tools/llvm-mca/bad-input.test b/llvm/test/tools/llvm-mca/bad-input.test
new file mode 100644
index 00000000000000..67565afe28ca97
--- /dev/null
+++ b/llvm/test/tools/llvm-mca/bad-input.test
@@ -0,0 +1,11 @@
+# RUN: not llvm-mca -march=aarch64 %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK %s
+# RUN: not llvm-mca -march=aarch64 -skip-unsupported-instructions %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK-SKIP %s
+
+# Test checks that MCA does not produce a total cycles estimate if it encounters parse errors.
+
+# CHECK-ALL-NOT: Total Cycles:
+
+# CHECK: error: Assembly input parsing had errors.
+# CHECK-SKIP: error: no assembly instructions found.
+
+This is not a valid assembly file (by virtue of this text.)
diff --git a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
index 5241b584b74661..c84d32514895d2 100644
--- a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
+++ b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
@@ -29,7 +29,7 @@ namespace mca {
CodeRegionGenerator::~CodeRegionGenerator() {}
Expected<const CodeRegions &> AsmCodeRegionGenerator::parseCodeRegions(
- const std::unique_ptr<MCInstPrinter> &IP) {
+ const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) {
MCTargetOptions Opts;
Opts.PreserveAsmComments = false;
CodeRegions &Regions = getRegions();
@@ -61,7 +61,9 @@ Expected<const CodeRegions &> AsmCodeRegionGenerator::parseCodeRegions(
"This target does not support assembly parsing.",
inconvertibleErrorCode());
Parser->setTargetParser(*TAP);
- Parser->Run(false);
+ if (Parser->Run(false) && !SkipUnsupportedInstructions)
+ return make_error<StringError>("Assembly input parsing had errors.",
+ inconvertibleErrorCode());
if (CCP->hadErr())
return make_error<StringError>("There was an error parsing comments.",
diff --git a/llvm/tools/llvm-mca/CodeRegionGenerator.h b/llvm/tools/llvm-mca/CodeRegionGenerator.h
index 68da567f3e0f32..11fab45629ce44 100644
--- a/llvm/tools/llvm-mca/CodeRegionGenerator.h
+++ b/llvm/tools/llvm-mca/CodeRegionGenerator.h
@@ -148,7 +148,7 @@ class CodeRegionGenerator {
CodeRegionGenerator(const CodeRegionGenerator &) = delete;
CodeRegionGenerator &operator=(const CodeRegionGenerator &) = delete;
virtual Expected<const CodeRegions &>
- parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) = 0;
+ parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) = 0;
public:
CodeRegionGenerator() {}
@@ -164,7 +164,7 @@ class AnalysisRegionGenerator : public virtual CodeRegionGenerator {
AnalysisRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {}
virtual Expected<const AnalysisRegions &>
- parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP) = 0;
+ parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) = 0;
};
/// Abstract CodeRegionGenerator with InstrumentRegionsRegions member
@@ -176,7 +176,7 @@ class InstrumentRegionGenerator : public virtual CodeRegionGenerator {
InstrumentRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {}
virtual Expected<const InstrumentRegions &>
- parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP) = 0;
+ parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) = 0;
};
/// This abstract class is responsible for parsing input ASM and
@@ -202,7 +202,7 @@ class AsmCodeRegionGenerator : public virtual CodeRegionGenerator {
unsigned getAssemblerDialect() const { return AssemblerDialect; }
Expected<const CodeRegions &>
- parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override;
+ parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) override;
};
class AsmAnalysisRegionGenerator final : public AnalysisRegionGenerator,
@@ -222,8 +222,8 @@ class AsmAnalysisRegionGenerator final : public AnalysisRegionGenerator,
MCStreamerWrapper *getMCStreamer() override { return &Streamer; }
Expected<const AnalysisRegions &>
- parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
- Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP);
+ parseAnalysisRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) override {
+ Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP, SkipUnsupportedInstructions);
if (!RegionsOrErr)
return RegionsOrErr.takeError();
else
@@ -231,8 +231,8 @@ class AsmAnalysisRegionGenerator final : public AnalysisRegionGenerator,
}
Expected<const CodeRegions &>
- parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
- return AsmCodeRegionGenerator::parseCodeRegions(IP);
+ parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) override {
+ return AsmCodeRegionGenerator::parseCodeRegions(IP, SkipUnsupportedInstructions);
}
};
@@ -254,8 +254,8 @@ class AsmInstrumentRegionGenerator final : public InstrumentRegionGenerator,
MCStreamerWrapper *getMCStreamer() override { return &Streamer; }
Expected<const InstrumentRegions &>
- parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
- Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP);
+ parseInstrumentRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) override {
+ Expected<const CodeRegions &> RegionsOrErr = parseCodeRegions(IP, SkipUnsupportedInstructions);
if (!RegionsOrErr)
return RegionsOrErr.takeError();
else
@@ -263,8 +263,8 @@ class AsmInstrumentRegionGenerator final : public InstrumentRegionGenerator,
}
Expected<const CodeRegions &>
- parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP) override {
- return AsmCodeRegionGenerator::parseCodeRegions(IP);
+ parseCodeRegions(const std::unique_ptr<MCInstPrinter> &IP, bool SkipUnsupportedInstructions) override {
+ return AsmCodeRegionGenerator::parseCodeRegions(IP, SkipUnsupportedInstructions);
}
};
diff --git a/llvm/tools/llvm-mca/llvm-mca.cpp b/llvm/tools/llvm-mca/llvm-mca.cpp
index e037c06b12a35d..e7b8edc5f1268a 100644
--- a/llvm/tools/llvm-mca/llvm-mca.cpp
+++ b/llvm/tools/llvm-mca/llvm-mca.cpp
@@ -440,7 +440,7 @@ int main(int argc, char **argv) {
mca::AsmAnalysisRegionGenerator CRG(*TheTarget, SrcMgr, ACtx, *MAI, *STI,
*MCII);
Expected<const mca::AnalysisRegions &> RegionsOrErr =
- CRG.parseAnalysisRegions(std::move(IPtemp));
+ CRG.parseAnalysisRegions(std::move(IPtemp), SkipUnsupportedInstructions);
if (!RegionsOrErr) {
if (auto Err =
handleErrors(RegionsOrErr.takeError(), [](const StringError &E) {
@@ -482,7 +482,7 @@ int main(int argc, char **argv) {
mca::AsmInstrumentRegionGenerator IRG(*TheTarget, SrcMgr, ICtx, *MAI, *STI,
*MCII, *IM);
Expected<const mca::InstrumentRegions &> InstrumentRegionsOrErr =
- IRG.parseInstrumentRegions(std::move(IPtemp));
+ IRG.parseInstrumentRegions(std::move(IPtemp), SkipUnsupportedInstructions);
if (!InstrumentRegionsOrErr) {
if (auto Err = handleErrors(InstrumentRegionsOrErr.takeError(),
[](const StringError &E) {
More information about the llvm-commits
mailing list