From 646597040690d940a8d55267f11fb3e75bb14dd9 Mon Sep 17 00:00:00 2001
From: Andrea Di Biagio <Andrea_DiBiagio@sn.scee.net>
Date: Thu, 4 Oct 2018 10:36:49 +0000
Subject: [PATCH] [llvm-mca] Check for inconsistencies when constructing
 instruction descriptors.

This should help with catching inconsistent definitions of instructions with
zero opcodes, which also declare to consume scheduler/pipeline resources.

llvm-svn: 343766
---
 .../include/HardwareUnits/RetireControlUnit.h |  5 +++
 tools/llvm-mca/include/InstrBuilder.h         |  2 ++
 .../lib/HardwareUnits/RetireControlUnit.cpp   |  4 +--
 tools/llvm-mca/lib/InstrBuilder.cpp           | 34 +++++++++++++++++++
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h b/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h
index 7f2d699e410..552a2094ff1 100644
--- a/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h
+++ b/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h
@@ -70,6 +70,11 @@ public:
     // of the reorder buffer. To avoid problems, cap the amount of slots to
     // the size of the reorder buffer.
     Quantity = std::min(Quantity, static_cast<unsigned>(Queue.size()));
+
+    // Further normalize the number of micro opcodes for instructions that
+    // declare zero opcodes. This should match the behavior of method
+    // reserveSlot().
+    Quantity = std::max(Quantity, 1U);
     return AvailableSlots >= Quantity;
   }
 
diff --git a/tools/llvm-mca/include/InstrBuilder.h b/tools/llvm-mca/include/InstrBuilder.h
index 5888b08b3a3..e3180925dab 100644
--- a/tools/llvm-mca/include/InstrBuilder.h
+++ b/tools/llvm-mca/include/InstrBuilder.h
@@ -62,6 +62,8 @@ class InstrBuilder {
                              unsigned SchedClassID);
   llvm::Error populateReads(InstrDesc &ID, const llvm::MCInst &MCI,
                             unsigned SchedClassID);
+  llvm::Error verifyInstrDesc(const InstrDesc &ID,
+                              const llvm::MCInst &MCI) const;
 
 public:
   InstrBuilder(const llvm::MCSubtargetInfo &sti, const llvm::MCInstrInfo &mcii,
diff --git a/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp b/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp
index ca3cc8cfafb..af1b01f49dc 100644
--- a/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp
+++ b/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp
@@ -41,10 +41,10 @@ RetireControlUnit::RetireControlUnit(const MCSchedModel &SM)
 // Reserves a number of slots, and returns a new token.
 unsigned RetireControlUnit::reserveSlot(const InstRef &IR,
                                         unsigned NumMicroOps) {
-  assert(isAvailable(NumMicroOps));
+  assert(isAvailable(NumMicroOps) && "Reorder Buffer unavailable!");
   unsigned NormalizedQuantity =
       std::min(NumMicroOps, static_cast<unsigned>(Queue.size()));
-  // Zero latency instructions may have zero mOps. Artificially bump this
+  // Zero latency instructions may have zero uOps. Artificially bump this
   // value to 1. Although zero latency instructions don't consume scheduler
   // resources, they still consume one slot in the retire queue.
   NormalizedQuantity = std::max(NormalizedQuantity, 1U);
diff --git a/tools/llvm-mca/lib/InstrBuilder.cpp b/tools/llvm-mca/lib/InstrBuilder.cpp
index 6f776f39668..0a26f40b940 100644
--- a/tools/llvm-mca/lib/InstrBuilder.cpp
+++ b/tools/llvm-mca/lib/InstrBuilder.cpp
@@ -321,6 +321,36 @@ Error InstrBuilder::populateReads(InstrDesc &ID, const MCInst &MCI,
   return ErrorSuccess();
 }
 
+Error InstrBuilder::verifyInstrDesc(const InstrDesc &ID,
+                                    const MCInst &MCI) const {
+  if (ID.NumMicroOps != 0)
+    return ErrorSuccess();
+
+  bool UsesMemory = ID.MayLoad || ID.MayStore;
+  bool UsesBuffers = !ID.Buffers.empty();
+  bool UsesResources = !ID.Resources.empty();
+  if (!UsesMemory && !UsesBuffers && !UsesResources)
+    return ErrorSuccess();
+
+  std::string ToString;
+  raw_string_ostream OS(ToString);
+  if (UsesMemory) {
+    WithColor::error() << "found an inconsistent instruction that decodes "
+                       << "into zero opcodes and that consumes load/store "
+                       << "unit resources.\n";
+  } else {
+    WithColor::error() << "found an inconsistent instruction that decodes"
+                       << " to zero opcodes and that consumes scheduler "
+                       << "resources.\n";
+  }
+
+  MCIP.printInst(&MCI, OS, "", STI);
+  OS.flush();
+  WithColor::note() << "instruction: " << ToString << '\n';
+  return make_error<StringError>("Invalid instruction definition found",
+                                 inconvertibleErrorCode());
+}
+
 Expected<const InstrDesc &>
 InstrBuilder::createInstrDescImpl(const MCInst &MCI) {
   assert(STI.getSchedModel().hasInstrSchedModel() &&
@@ -392,6 +422,10 @@ InstrBuilder::createInstrDescImpl(const MCInst &MCI) {
   LLVM_DEBUG(dbgs() << "\t\tMaxLatency=" << ID->MaxLatency << '\n');
   LLVM_DEBUG(dbgs() << "\t\tNumMicroOps=" << ID->NumMicroOps << '\n');
 
+  // Sanity check on the instruction descriptor.
+  if (Error Err = verifyInstrDesc(*ID, MCI))
+    return std::move(Err);
+
   // Now add the new descriptor.
   SchedClassID = MCDesc.getSchedClass();
   if (!SM.getSchedClassDesc(SchedClassID)->isVariant()) {