[llvm] r343766 - [llvm-mca] Check for inconsistencies when constructing instruction descriptors.

[Andrea Di Biagio via llvm-commits]

Author: adibiagio
Date: Thu Oct  4 03:36:49 2018
New Revision: 343766

URL: http://llvm.org/viewvc/llvm-project?rev=343766&view=rev
Log:
[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.

Modified:
    llvm/trunk/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h
    llvm/trunk/tools/llvm-mca/include/InstrBuilder.h
    llvm/trunk/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp
    llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp

Modified: llvm/trunk/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h?rev=343766&r1=343765&r2=343766&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h (original)
+++ llvm/trunk/tools/llvm-mca/include/HardwareUnits/RetireControlUnit.h Thu Oct  4 03:36:49 2018
@@ -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;
   }
 

Modified: llvm/trunk/tools/llvm-mca/include/InstrBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/include/InstrBuilder.h?rev=343766&r1=343765&r2=343766&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/include/InstrBuilder.h (original)
+++ llvm/trunk/tools/llvm-mca/include/InstrBuilder.h Thu Oct  4 03:36:49 2018
@@ -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,

Modified: llvm/trunk/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp?rev=343766&r1=343765&r2=343766&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp (original)
+++ llvm/trunk/tools/llvm-mca/lib/HardwareUnits/RetireControlUnit.cpp Thu Oct  4 03:36:49 2018
@@ -41,10 +41,10 @@ RetireControlUnit::RetireControlUnit(con
 // 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);

Modified: llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp?rev=343766&r1=343765&r2=343766&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp (original)
+++ llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp Thu Oct  4 03:36:49 2018
@@ -321,6 +321,36 @@ Error InstrBuilder::populateReads(InstrD
   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
   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()) {