[llvm] r350820 - [MCA] Fix wrong definition of ResourceUnitMask in DefaultResourceStrategy.

Andrea Di Biagio via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 10 05:59:13 PST 2019


Author: adibiagio
Date: Thu Jan 10 05:59:13 2019
New Revision: 350820

URL: http://llvm.org/viewvc/llvm-project?rev=350820&view=rev
Log:
[MCA] Fix wrong definition of ResourceUnitMask in DefaultResourceStrategy.

Field ResourceUnitMask was incorrectly defined as a 'const unsigned' mask. It
should have been a 64 bit quantity instead. That means, ResourceUnitMask was
always implicitly truncated to a 32 bit quantity.
This issue has been found by inspection. Surprisingly, that bug was latent, and
it never negatively affected any existing upstream targets.

This patch fixes  the wrong definition of ResourceUnitMask, and adds a bunch of
extra debug prints to help debugging potential issues related to invalid
processor resource masks.

Modified:
    llvm/trunk/include/llvm/MCA/HardwareUnits/ResourceManager.h
    llvm/trunk/include/llvm/MCA/Instruction.h
    llvm/trunk/include/llvm/MCA/Stages/ExecuteStage.h
    llvm/trunk/include/llvm/MCA/Stages/InstructionTables.h
    llvm/trunk/include/llvm/MCA/Support.h
    llvm/trunk/lib/MCA/HardwareUnits/ResourceManager.cpp
    llvm/trunk/lib/MCA/InstrBuilder.cpp
    llvm/trunk/lib/MCA/Pipeline.cpp
    llvm/trunk/lib/MCA/Stages/ExecuteStage.cpp
    llvm/trunk/lib/MCA/Support.cpp
    llvm/trunk/tools/llvm-mca/Views/SummaryView.cpp

Modified: llvm/trunk/include/llvm/MCA/HardwareUnits/ResourceManager.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MCA/HardwareUnits/ResourceManager.h?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MCA/HardwareUnits/ResourceManager.h (original)
+++ llvm/trunk/include/llvm/MCA/HardwareUnits/ResourceManager.h Thu Jan 10 05:59:13 2019
@@ -72,7 +72,7 @@ class DefaultResourceStrategy final : pu
   ///
   /// There is one bit set for every available resource unit.
   /// It defaults to the value of field ResourceSizeMask in ResourceState.
-  const unsigned ResourceUnitMask;
+  const uint64_t ResourceUnitMask;
 
   /// A simple round-robin selector for processor resource units.
   /// Each bit of this mask identifies a sub resource within a group.
@@ -335,13 +335,13 @@ class ResourceManager {
   // Used to quickly identify groups that own a particular resource unit.
   std::vector<uint64_t> Resource2Groups;
 
+  // A table to map processor resource IDs to processor resource masks.
+  SmallVector<uint64_t, 8> ProcResID2Mask;
+
   // Keeps track of which resources are busy, and how many cycles are left
   // before those become usable again.
   SmallDenseMap<ResourceRef, unsigned> BusyResources;
 
-  // A table to map processor resource IDs to processor resource masks.
-  SmallVector<uint64_t, 8> ProcResID2Mask;
-
   // Returns the actual resource unit that will be used.
   ResourceRef selectPipe(uint64_t ResourceID);
 

Modified: llvm/trunk/include/llvm/MCA/Instruction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MCA/Instruction.h?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MCA/Instruction.h (original)
+++ llvm/trunk/include/llvm/MCA/Instruction.h Thu Jan 10 05:59:13 2019
@@ -138,8 +138,8 @@ class WriteState {
 public:
   WriteState(const WriteDescriptor &Desc, unsigned RegID,
              bool clearsSuperRegs = false, bool writesZero = false)
-      : WD(&Desc), CyclesLeft(UNKNOWN_CYCLES), RegisterID(RegID),
-        PRFID(0), ClearsSuperRegs(clearsSuperRegs), WritesZero(writesZero),
+      : WD(&Desc), CyclesLeft(UNKNOWN_CYCLES), RegisterID(RegID), PRFID(0),
+        ClearsSuperRegs(clearsSuperRegs), WritesZero(writesZero),
         IsEliminated(false), DependentWrite(nullptr), PartialWrite(nullptr),
         DependentWriteCyclesLeft(0) {}
 
@@ -155,7 +155,9 @@ public:
   void addUser(ReadState *Use, int ReadAdvance);
   void addUser(WriteState *Use);
 
-  unsigned getDependentWriteCyclesLeft() const { return DependentWriteCyclesLeft; }
+  unsigned getDependentWriteCyclesLeft() const {
+    return DependentWriteCyclesLeft;
+  }
 
   unsigned getNumUsers() const {
     unsigned NumUsers = Users.size();
@@ -348,11 +350,6 @@ struct InstrDesc {
   InstrDesc() = default;
   InstrDesc(const InstrDesc &Other) = delete;
   InstrDesc &operator=(const InstrDesc &Other) = delete;
-
-#ifndef NDEBUG
-  // Original instruction name for debugging purposes.
-  StringRef Name;
-#endif
 };
 
 /// Base class for instructions consumed by the simulation pipeline.
@@ -551,4 +548,4 @@ public:
 } // namespace mca
 } // namespace llvm
 
-#endif  // LLVM_MCA_INSTRUCTION_H
+#endif // LLVM_MCA_INSTRUCTION_H

Modified: llvm/trunk/include/llvm/MCA/Stages/ExecuteStage.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MCA/Stages/ExecuteStage.h?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MCA/Stages/ExecuteStage.h (original)
+++ llvm/trunk/include/llvm/MCA/Stages/ExecuteStage.h Thu Jan 10 05:59:13 2019
@@ -65,7 +65,7 @@ public:
 
   void notifyInstructionIssued(
       const InstRef &IR,
-      ArrayRef<std::pair<ResourceRef, ResourceCycles>> Used) const;
+      MutableArrayRef<std::pair<ResourceRef, ResourceCycles>> Used) const;
   void notifyInstructionExecuted(const InstRef &IR) const;
   void notifyInstructionReady(const InstRef &IR) const;
   void notifyResourceAvailable(const ResourceRef &RR) const;

Modified: llvm/trunk/include/llvm/MCA/Stages/InstructionTables.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MCA/Stages/InstructionTables.h?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MCA/Stages/InstructionTables.h (original)
+++ llvm/trunk/include/llvm/MCA/Stages/InstructionTables.h Thu Jan 10 05:59:13 2019
@@ -32,7 +32,8 @@ class InstructionTables final : public S
   SmallVector<uint64_t, 8> Masks;
 
 public:
-  InstructionTables(const MCSchedModel &Model) : Stage(), SM(Model) {
+  InstructionTables(const MCSchedModel &Model)
+      : Stage(), SM(Model), Masks(Model.getNumProcResourceKinds()) {
     computeProcResourceMasks(Model, Masks);
   }
 

Modified: llvm/trunk/include/llvm/MCA/Support.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MCA/Support.h?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MCA/Support.h (original)
+++ llvm/trunk/include/llvm/MCA/Support.h Thu Jan 10 05:59:13 2019
@@ -104,7 +104,7 @@ public:
 /// Resource masks are used by the ResourceManager to solve set membership
 /// problems with simple bit manipulation operations.
 void computeProcResourceMasks(const MCSchedModel &SM,
-                              SmallVectorImpl<uint64_t> &Masks);
+                              MutableArrayRef<uint64_t> Masks);
 
 /// Compute the reciprocal block throughput from a set of processor resource
 /// cycles. The reciprocal block throughput is computed as the MAX between:

Modified: llvm/trunk/lib/MCA/HardwareUnits/ResourceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MCA/HardwareUnits/ResourceManager.cpp?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/lib/MCA/HardwareUnits/ResourceManager.cpp (original)
+++ llvm/trunk/lib/MCA/HardwareUnits/ResourceManager.cpp Thu Jan 10 05:59:13 2019
@@ -118,7 +118,8 @@ getStrategyFor(const ResourceState &RS)
 ResourceManager::ResourceManager(const MCSchedModel &SM)
     : Resources(SM.getNumProcResourceKinds()),
       Strategies(SM.getNumProcResourceKinds()),
-      Resource2Groups(SM.getNumProcResourceKinds(), 0) {
+      Resource2Groups(SM.getNumProcResourceKinds(), 0),
+      ProcResID2Mask(SM.getNumProcResourceKinds()) {
   computeProcResourceMasks(SM, ProcResID2Mask);
 
   for (unsigned I = 0, E = SM.getNumProcResourceKinds(); I < E; ++I) {
@@ -283,9 +284,6 @@ void ResourceManager::issueInstruction(
       ResourceRef Pipe = selectPipe(R.first);
       use(Pipe);
       BusyResources[Pipe] += CS.size();
-      // Replace the resource mask with a valid processor resource index.
-      const ResourceState &RS = *Resources[getResourceStateIndex(Pipe.first)];
-      Pipe.first = RS.getProcResourceID();
       Pipes.emplace_back(std::pair<ResourceRef, ResourceCycles>(
           Pipe, ResourceCycles(CS.size())));
     } else {

Modified: llvm/trunk/lib/MCA/InstrBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MCA/InstrBuilder.cpp?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/lib/MCA/InstrBuilder.cpp (original)
+++ llvm/trunk/lib/MCA/InstrBuilder.cpp Thu Jan 10 05:59:13 2019
@@ -31,6 +31,8 @@ InstrBuilder::InstrBuilder(const llvm::M
                            const llvm::MCInstrAnalysis *mcia)
     : STI(sti), MCII(mcii), MRI(mri), MCIA(mcia), FirstCallInst(true),
       FirstReturnInst(true) {
+  const MCSchedModel &SM = STI.getSchedModel();
+  ProcResourceMasks.resize(SM.getNumProcResourceKinds());
   computeProcResourceMasks(STI.getSchedModel(), ProcResourceMasks);
 }
 
@@ -178,8 +180,8 @@ static void initializeUsedResources(Inst
 
   LLVM_DEBUG({
     for (const std::pair<uint64_t, ResourceUsage> &R : ID.Resources)
-      dbgs() << "\t\tMask=" << format_hex(R.first, 16) << ", " <<
-                "cy=" << R.second.size() << '\n';
+      dbgs() << "\t\tMask=" << format_hex(R.first, 16) << ", "
+             << "cy=" << R.second.size() << '\n';
     for (const uint64_t R : ID.Buffers)
       dbgs() << "\t\tBuffer Mask=" << format_hex(R, 16) << '\n';
   });
@@ -525,6 +527,9 @@ InstrBuilder::createInstrDescImpl(const
         MCI);
   }
 
+  LLVM_DEBUG(dbgs() << "\n\t\tOpcode Name= " << MCII.getName(Opcode) << '\n');
+  LLVM_DEBUG(dbgs() << "\t\tSchedClassID=" << SchedClassID << '\n');
+
   // Create a new empty descriptor.
   std::unique_ptr<InstrDesc> ID = llvm::make_unique<InstrDesc>();
   ID->NumMicroOps = SCDesc.NumMicroOps;
@@ -559,9 +564,6 @@ InstrBuilder::createInstrDescImpl(const
   populateWrites(*ID, MCI, SchedClassID);
   populateReads(*ID, MCI, SchedClassID);
 
-#ifndef NDEBUG
-  ID->Name = MCII.getName(Opcode);
-#endif
   LLVM_DEBUG(dbgs() << "\t\tMaxLatency=" << ID->MaxLatency << '\n');
   LLVM_DEBUG(dbgs() << "\t\tNumMicroOps=" << ID->NumMicroOps << '\n');
 

Modified: llvm/trunk/lib/MCA/Pipeline.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MCA/Pipeline.cpp?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/lib/MCA/Pipeline.cpp (original)
+++ llvm/trunk/lib/MCA/Pipeline.cpp Thu Jan 10 05:59:13 2019
@@ -83,13 +83,13 @@ void Pipeline::appendStage(std::unique_p
 }
 
 void Pipeline::notifyCycleBegin() {
-  LLVM_DEBUG(dbgs() << "[E] Cycle begin: " << Cycles << '\n');
+  LLVM_DEBUG(dbgs() << "\n[E] Cycle begin: " << Cycles << '\n');
   for (HWEventListener *Listener : Listeners)
     Listener->onCycleBegin();
 }
 
 void Pipeline::notifyCycleEnd() {
-  LLVM_DEBUG(dbgs() << "[E] Cycle end: " << Cycles << "\n\n");
+  LLVM_DEBUG(dbgs() << "[E] Cycle end: " << Cycles << "\n");
   for (HWEventListener *Listener : Listeners)
     Listener->onCycleEnd();
 }

Modified: llvm/trunk/lib/MCA/Stages/ExecuteStage.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MCA/Stages/ExecuteStage.cpp?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/lib/MCA/Stages/ExecuteStage.cpp (original)
+++ llvm/trunk/lib/MCA/Stages/ExecuteStage.cpp Thu Jan 10 05:59:13 2019
@@ -57,6 +57,7 @@ Error ExecuteStage::issueInstruction(Ins
   HWS.issueInstruction(IR, Used, Ready);
 
   notifyReservedOrReleasedBuffers(IR, /* Reserved */ false);
+
   notifyInstructionIssued(IR, Used);
   if (IR.getInstruction()->isExecuted()) {
     notifyInstructionExecuted(IR);
@@ -184,7 +185,7 @@ void ExecuteStage::notifyResourceAvailab
 
 void ExecuteStage::notifyInstructionIssued(
     const InstRef &IR,
-    ArrayRef<std::pair<ResourceRef, ResourceCycles>> Used) const {
+    MutableArrayRef<std::pair<ResourceRef, ResourceCycles>> Used) const {
   LLVM_DEBUG({
     dbgs() << "[E] Instruction Issued: #" << IR << '\n';
     for (const std::pair<ResourceRef, ResourceCycles> &Resource : Used) {
@@ -193,6 +194,11 @@ void ExecuteStage::notifyInstructionIssu
       dbgs() << "cycles: " << Resource.second << '\n';
     }
   });
+
+  // Replace resource masks with valid resource processor IDs.
+  for (std::pair<ResourceRef, ResourceCycles> &Use : Used)
+    Use.first.first = HWS.getResourceID(Use.first.first);
+
   notifyEvent<HWInstructionEvent>(HWInstructionIssuedEvent(IR, Used));
 }
 

Modified: llvm/trunk/lib/MCA/Support.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MCA/Support.cpp?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/lib/MCA/Support.cpp (original)
+++ llvm/trunk/lib/MCA/Support.cpp Thu Jan 10 05:59:13 2019
@@ -19,13 +19,18 @@
 namespace llvm {
 namespace mca {
 
+#define DEBUG_TYPE "llvm-mca"
+
 void computeProcResourceMasks(const MCSchedModel &SM,
-                              SmallVectorImpl<uint64_t> &Masks) {
+                              MutableArrayRef<uint64_t> Masks) {
   unsigned ProcResourceID = 0;
 
+  assert(Masks.size() == SM.getNumProcResourceKinds() &&
+         "Invalid number of elements");
+  // Resource at index 0 is the 'InvalidUnit'. Set an invalid mask for it.
+  Masks[0] = 0;
+
   // Create a unique bitmask for every processor resource unit.
-  // Skip resource at index 0, since it always references 'InvalidUnit'.
-  Masks.resize(SM.getNumProcResourceKinds());
   for (unsigned I = 1, E = SM.getNumProcResourceKinds(); I < E; ++I) {
     const MCProcResourceDesc &Desc = *SM.getProcResource(I);
     if (Desc.SubUnitsIdxBegin)
@@ -46,6 +51,16 @@ void computeProcResourceMasks(const MCSc
     }
     ProcResourceID++;
   }
+
+#ifndef NDEBUG
+  LLVM_DEBUG(dbgs() << "\nProcessor resource masks:"
+                    << "\n");
+  for (unsigned I = 0, E = SM.getNumProcResourceKinds(); I < E; ++I) {
+    const MCProcResourceDesc &Desc = *SM.getProcResource(I);
+    LLVM_DEBUG(dbgs() << '[' << I << "] " << Desc.Name << " - " << Masks[I]
+                      << '\n');
+  }
+#endif
 }
 
 double computeBlockRThroughput(const MCSchedModel &SM, unsigned DispatchWidth,

Modified: llvm/trunk/tools/llvm-mca/Views/SummaryView.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/Views/SummaryView.cpp?rev=350820&r1=350819&r2=350820&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/Views/SummaryView.cpp (original)
+++ llvm/trunk/tools/llvm-mca/Views/SummaryView.cpp Thu Jan 10 05:59:13 2019
@@ -27,7 +27,8 @@ SummaryView::SummaryView(const MCSchedMo
                          unsigned Width)
     : SM(Model), Source(S), DispatchWidth(Width), LastInstructionIdx(0),
       TotalCycles(0), NumMicroOps(0),
-      ProcResourceUsage(Model.getNumProcResourceKinds(), 0) {
+      ProcResourceUsage(Model.getNumProcResourceKinds(), 0),
+      ProcResourceMasks(Model.getNumProcResourceKinds()) {
   computeProcResourceMasks(SM, ProcResourceMasks);
 }
 




More information about the llvm-commits mailing list