[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 07:47:13 PDT 2024


https://github.com/peterwaller-arm updated https://github.com/llvm/llvm-project/pull/90474

>From 6b57520107755e8721b61cd4fcd99461be29f5c9 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.

Add a description of -skip-unsupported-instructions to the llvm-mca
command guide.
---
 llvm/docs/CommandGuide/llvm-mca.rst           |  7 ++++
 .../AArch64/Exynos/float-divide-multiply.s    |  2 +-
 .../llvm-mca/AArch64/Exynos/float-integer.s   |  2 +-
 llvm/test/tools/llvm-mca/bad-input.s          | 11 ++++++
 llvm/tools/llvm-mca/CodeRegionGenerator.cpp   |  7 ++--
 llvm/tools/llvm-mca/CodeRegionGenerator.h     | 36 ++++++++++++-------
 llvm/tools/llvm-mca/llvm-mca.cpp              |  5 +--
 7 files changed, 52 insertions(+), 18 deletions(-)
 create mode 100644 llvm/test/tools/llvm-mca/bad-input.s

diff --git a/llvm/docs/CommandGuide/llvm-mca.rst b/llvm/docs/CommandGuide/llvm-mca.rst
index eae5e1406b899e..effb990a2f36b4 100644
--- a/llvm/docs/CommandGuide/llvm-mca.rst
+++ b/llvm/docs/CommandGuide/llvm-mca.rst
@@ -234,6 +234,13 @@ option specifies "``-``", then the output will also be sent to standard output.
   no extra information, and InstrumentManager never overrides the default
   schedule class for a given instruction.
 
+.. option:: -skip-unsupported-instructions
+
+  Force :program:`llvm-mca` to continue even in the presence of instructions
+  which do not parse or lack key scheduling formation. Note that the resulting
+  analysis is impacted since those unsupported instructions are ignored as-if
+  they are not supplied as a part of the input.
+
 EXIT STATUS
 -----------
 
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.s b/llvm/test/tools/llvm-mca/bad-input.s
new file mode 100644
index 00000000000000..a412f10394fb25
--- /dev/null
+++ b/llvm/test/tools/llvm-mca/bad-input.s
@@ -0,0 +1,11 @@
+# RUN: not llvm-mca %s -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK-ALL,CHECK %s
+# RUN: not llvm-mca -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 for any architecture (by virtue of this text.)
diff --git a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
index 5241b584b74661..77ab6584589e4c 100644
--- a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
+++ b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
@@ -29,7 +29,8 @@ 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 +62,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..8fd988bf97f0a1 100644
--- a/llvm/tools/llvm-mca/CodeRegionGenerator.h
+++ b/llvm/tools/llvm-mca/CodeRegionGenerator.h
@@ -148,7 +148,8 @@ 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 +165,8 @@ 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 +178,8 @@ 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 +205,8 @@ 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 +226,10 @@ 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 +237,10 @@ 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 +262,10 @@ 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 +273,10 @@ 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..674a2da551b2c4 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,8 @@ 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