[llvm-mca] Updates comment in code, and remove some stale comments. NFC

Also, rename fields `TotalMappings` and `NumUsedMappings` in struct
RegisterMappingTracker into `NumPhysRegs` and `NumUsedPhysRegs`.

llvm-svn: 335219

tools/llvm-mca/DispatchStage.h

@@ -36,25 +36,20 @@ class Backend;
 //
 // This class is responsible for the dispatch stage, in which instructions are
 // dispatched in groups to the Scheduler.  An instruction can be dispatched if
-// functional units are available.
+// the following conditions are met:
-// To be more specific, an instruction can be dispatched to the Scheduler if:
+//  1) There are enough entries in the reorder buffer (see class
-//  1) There are enough entries in the reorder buffer (implemented by class
+//     RetireControlUnit) to write the opcodes associated with the instruction.
-//     RetireControlUnit) to accommodate all opcodes.
 //  2) There are enough temporaries to rename output register operands.
 //  3) There are enough entries available in the used buffered resource(s).
 //
 // The number of micro opcodes that can be dispatched in one cycle is limited by
 // the value of field 'DispatchWidth'. A "dynamic dispatch stall" occurs when
-// processor resources are not available (i.e. at least one of the
+// processor resources are not available. Dispatch stall events are counted
-// aforementioned checks fails). Dispatch stall events are counted during the
+// during the entire execution of the code, and displayed by the performance
-// entire execution of the code, and displayed by the performance report when
+// report when flag '-dispatch-stats' is specified.
-// flag '-verbose' is specified.
 //
-// If the number of micro opcodes of an instruction is bigger than
+// If the number of micro opcodes exceedes DispatchWidth, then the instruction
-// DispatchWidth, then it can only be dispatched at the beginning of one cycle.
+// is dispatched in multiple cycles.
-// The DispatchStage will still have to wait for a number of cycles (depending
-// on the DispatchWidth and the number of micro opcodes) before it can serve
-// other instructions.
 class DispatchStage : public Stage {
   unsigned DispatchWidth;
   unsigned AvailableEntries;

tools/llvm-mca/InstrBuilder.cpp

@@ -159,61 +159,7 @@ static void populateWrites(InstrDesc &ID, const MCInst &MCI,
                            const MCInstrDesc &MCDesc,
                            const MCSchedClassDesc &SCDesc,
                            const MCSubtargetInfo &STI) {
-  // This algorithm currently works under the strong (and potentially incorrect)
+  // These are for now the (strong) assumptions made by this algorithm:
-  // assumption that information related to register def/uses can be obtained
-  // from MCInstrDesc.
-  //
-  // However class MCInstrDesc is used to describe MachineInstr objects and not
-  // MCInst objects. To be more specific, MCInstrDesc objects are opcode
-  // descriptors that are automatically generated via tablegen based on the
-  // instruction set information available from the target .td files.  That
-  // means, the number of (explicit) definitions according to MCInstrDesc always
-  // matches the cardinality of the `(outs)` set in tablegen.
-  //
-  // By constructions, definitions must appear first in the operand sequence of
-  // a MachineInstr. Also, the (outs) sequence is preserved (example: the first
-  // element in the outs set is the first operand in the corresponding
-  // MachineInstr).  That's the reason why MCInstrDesc only needs to declare the
-  // total number of register definitions, and not where those definitions are
-  // in the machine operand sequence.
-  //
-  // Unfortunately, it is not safe to use the information from MCInstrDesc to
-  // also describe MCInst objects. An MCInst object can be obtained from a
-  // MachineInstr through a lowering step which may restructure the operand
-  // sequence (and even remove or introduce new operands). So, there is a high
-  // risk that the lowering step breaks the assumptions that register
-  // definitions are always at the beginning of the machine operand sequence.
-  //
-  // This is a fundamental problem, and it is still an open problem. Essentially
-  // we have to find a way to correlate def/use operands of a MachineInstr to
-  // operands of an MCInst. Otherwise, we cannot correctly reconstruct data
-  // dependencies, nor we can correctly interpret the scheduling model, which
-  // heavily uses machine operand indices to define processor read-advance
-  // information, and to identify processor write resources.  Essentially, we
-  // either need something like a MCInstrDesc, but for MCInst, or a way
-  // to map MCInst operands back to MachineInstr operands.
-  //
-  // Unfortunately, we don't have that information now. So, this prototype
-  // currently work under the strong assumption that we can always safely trust
-  // the content of an MCInstrDesc.  For example, we can query a MCInstrDesc to
-  // obtain the number of explicit and implicit register defintions.  We also
-  // assume that register definitions always come first in the operand sequence.
-  // This last assumption usually makes sense for MachineInstr, where register
-  // definitions always appear at the beginning of the operands sequence. In
-  // reality, these assumptions could be broken by the lowering step, which can
-  // decide to lay out operands in a different order than the original order of
-  // operand as specified by the MachineInstr.
-  //
-  // Things get even more complicated in the presence of "optional" register
-  // definitions. For MachineInstr, optional register definitions are always at
-  // the end of the operand sequence. Some ARM instructions that may update the
-  // status flags specify that register as a optional operand.  Since we don't
-  // have operand descriptors for MCInst, we assume for now that the optional
-  // definition is always the last operand of a MCInst.  Again, this assumption
-  // may be okay for most targets. However, there is no guarantee that targets
-  // would respect that.
-  //
-  // In conclusion: these are for now the strong assumptions made by the tool:
   //  * The number of explicit and implicit register definitions in a MCInst
   //    matches the number of explicit and implicit definitions according to
   //    the opcode descriptor (MCInstrDesc).
@@ -227,8 +173,6 @@ static void populateWrites(InstrDesc &ID, const MCInst &MCI,
   // like x86. This is mainly because the vast majority of instructions is
   // expanded to MCInst using a straightforward lowering logic that preserves
   // the ordering of the operands.
-  //
-  // In the longer term, we need to find a proper solution for this issue.
   unsigned NumExplicitDefs = MCDesc.getNumDefs();
   unsigned NumImplicitDefs = MCDesc.getNumImplicitDefs();
   unsigned NumWriteLatencyEntries = SCDesc.NumWriteLatencyEntries;

tools/llvm-mca/RegisterFile.cpp

@@ -96,12 +96,12 @@ void RegisterFile::allocatePhysRegs(IndexPlusCostPairTy Entry,
   unsigned Cost = Entry.second;
   if (RegisterFileIndex) {
     RegisterMappingTracker &RMT = RegisterFiles[RegisterFileIndex];
-    RMT.NumUsedMappings += Cost;
+    RMT.NumUsedPhysRegs += Cost;
     UsedPhysRegs[RegisterFileIndex] += Cost;
   }
 
   // Now update the default register mapping tracker.
-  RegisterFiles[0].NumUsedMappings += Cost;
+  RegisterFiles[0].NumUsedPhysRegs += Cost;
   UsedPhysRegs[0] += Cost;
 }
 
@@ -111,12 +111,12 @@ void RegisterFile::freePhysRegs(IndexPlusCostPairTy Entry,
   unsigned Cost = Entry.second;
   if (RegisterFileIndex) {
     RegisterMappingTracker &RMT = RegisterFiles[RegisterFileIndex];
-    RMT.NumUsedMappings -= Cost;
+    RMT.NumUsedPhysRegs -= Cost;
     FreedPhysRegs[RegisterFileIndex] += Cost;
   }
 
   // Now update the default register mapping tracker.
-  RegisterFiles[0].NumUsedMappings -= Cost;
+  RegisterFiles[0].NumUsedPhysRegs -= Cost;
   FreedPhysRegs[0] += Cost;
 }
 
@@ -215,13 +215,13 @@ unsigned RegisterFile::isAvailable(ArrayRef<unsigned> Regs) const {
       continue;
 
     const RegisterMappingTracker &RMT = RegisterFiles[I];
-    if (!RMT.TotalMappings) {
+    if (!RMT.NumPhysRegs) {
       // The register file has an unbounded number of microarchitectural
       // registers.
       continue;
     }
 
-    if (RMT.TotalMappings < NumRegs) {
+    if (RMT.NumPhysRegs < NumRegs) {
       // The current register file is too small. This may occur if the number of
       // microarchitectural registers in register file #0 was changed by the
       // users via flag -reg-file-size. Alternatively, the scheduling model
@@ -230,7 +230,7 @@ unsigned RegisterFile::isAvailable(ArrayRef<unsigned> Regs) const {
           "Not enough microarchitectural registers in the register file");
     }
 
-    if (RMT.TotalMappings < (RMT.NumUsedMappings + NumRegs))
+    if (RMT.NumPhysRegs < (RMT.NumUsedPhysRegs + NumRegs))
       Response |= (1U << I);
   }
 
@@ -252,8 +252,8 @@ void RegisterFile::dump() const {
   for (unsigned I = 0, E = getNumRegisterFiles(); I < E; ++I) {
     dbgs() << "Register File #" << I;
     const RegisterMappingTracker &RMT = RegisterFiles[I];
-    dbgs() << "\n  TotalMappings:        " << RMT.TotalMappings
+    dbgs() << "\n  TotalMappings:        " << RMT.NumPhysRegs
-           << "\n  NumUsedMappings:      " << RMT.NumUsedMappings << '\n';
+           << "\n  NumUsedMappings:      " << RMT.NumUsedPhysRegs << '\n';
   }
 }
 #endif

tools/llvm-mca/RegisterFile.h

@@ -26,94 +26,93 @@ namespace mca {
 class ReadState;
 class WriteState;
 
-/// Manages hardware register files, and tracks data dependencies
+/// Manages hardware register files, and tracks register definitions for
-/// between registers.
+/// register renaming purposes.
 class RegisterFile {
   const llvm::MCRegisterInfo &MRI;
 
-  // Each register file is described by an instance of RegisterMappingTracker.
+  // Each register file is associated with an instance of RegisterMappingTracker.
-  // RegisterMappingTracker tracks the number of register mappings dynamically
+  // A RegisterMappingTracker keeps track of the number of physical registers
-  // allocated during the execution.
+  // which have been dynamically allocated by the simulator.
   struct RegisterMappingTracker {
-    // Total number of register mappings that are available for register
+    // The total number of physical registers that are available in this
-    // renaming. A value of zero for this field means: this register file has
+    // register file for register renaming purpouses.  A value of zero for this
-    // an unbounded number of registers.
+    // field means: this register file has an unbounded number of physical
-    const unsigned TotalMappings;
+    // registers.
-    // Number of mappings that are currently in use.
+    const unsigned NumPhysRegs;
-    unsigned NumUsedMappings;
+    // Number of physical registers that are currently in use.
+    unsigned NumUsedPhysRegs;
 
-    RegisterMappingTracker(unsigned NumMappings)
+    RegisterMappingTracker(unsigned NumPhysRegisters)
-        : TotalMappings(NumMappings), NumUsedMappings(0) {}
+        : NumPhysRegs(NumPhysRegisters), NumUsedPhysRegs(0) {}
   };
 
-  // This is where information related to the various register files is kept.
+  // A vector of register file descriptors.  This set always contains at least
-  // This set always contains at least one register file at index #0. That
+  // one entry. Entry at index #0 is reserved.  That entry describes a register
-  // register file "sees" all the physical registers declared by the target, and
+  // file with an unbounded number of physical registers that "sees" all the
-  // (by default) it allows an unbounded number of mappings.
+  // hardware registers declared by the target (i.e. all the register
-  // Users can limit the number of mappings that can be created by register file
+  // definitions in the target specific `XYZRegisterInfo.td` - where `XYZ` is
-  // #0 through the command line flag `-register-file-size`.
+  // the target name).
+  // 
+  // Users can limit the number of physical registers that are available in
+  // regsiter file #0 specifying command line flag `-register-file-size=<uint>`.
   llvm::SmallVector<RegisterMappingTracker, 4> RegisterFiles;
 
-  // This pair is used to identify the owner of a physical register, as well as
+  // This pair is used to identify the owner of a register, as well as
-  // the cost of using that register file.
+  // the "register cost". Register cost is defined as the number of physical
+  // registers required to allocate a user register.
+  // For example: on X86 BtVer2, a YMM register consumes 2 128-bit physical
+  // registers. So, the cost of allocating a YMM register in BtVer2 is 2.
   using IndexPlusCostPairTy = std::pair<unsigned, unsigned>;
 
   // RegisterMapping objects are mainly used to track physical register
-  // definitions. A WriteState object describes a register definition, and it is
+  // definitions. There is a RegisterMapping for every register defined by the
-  // used to track RAW dependencies (see Instruction.h).  A RegisterMapping
+  // Target. For each register, a RegisterMapping pair contains a descriptor of
-  // object also specifies the set of register files.  The mapping between
+  // the last register write (in the form of a WriteState object), as well as a
-  // physreg and register files is done using a "register file mask".
+  // IndexPlusCostPairTy to quickly identify owning register files.
-  //
-  // A register file index identifies a user defined register file.
-  // There is one index per RegisterMappingTracker, and index #0 is reserved to
-  // the default unified register file.
   //
   // This implementation does not allow overlapping register files. The only
   // register file that is allowed to overlap with other register files is
-  // register file #0.
+  // register file #0. If we exclude register #0, every register is "owned" by
+  // at most one register file.
   using RegisterMapping = std::pair<WriteState *, IndexPlusCostPairTy>;
 
-  // This map contains one entry for each physical register defined by the
+  // This map contains one entry for each register defined by the target.
-  // processor scheduling model.
   std::vector<RegisterMapping> RegisterMappings;
 
-  // This method creates a new RegisterMappingTracker for a register file that
+  // This method creates a new register file descriptor.
-  // contains all the physical registers specified by the register classes in
+  // The new register file owns all of the registers declared by register
-  // the 'RegisterClasses' set.
+  // classes in the 'RegisterClasses' set.
   //
-  // The long term goal is to let scheduling models optionally describe register
+  // Processor models allow the definition of RegisterFile(s) via tablegen. For
-  // files via tablegen definitions. This is still a work in progress.
+  // example, this is a tablegen definition for a x86 register file for
-  // For example, here is how a tablegen definition for a x86 FP register file
+  // XMM[0-15] and YMM[0-15], that allows up to 60 renames (each rename costs 1
-  // that features AVX might look like:
+  // physical register).
   //
-  //    def FPRegisterFile : RegisterFile<[VR128RegClass, VR256RegClass], 60>
+  //    def FPRegisterFile : RegisterFile<60, [VR128RegClass, VR256RegClass]>
   //
   // Here FPRegisterFile contains all the registers defined by register class
   // VR128RegClass and VR256RegClass. FPRegisterFile implements 60
   // registers which can be used for register renaming purpose.
-  //
-  // The list of register classes is then converted by the tablegen backend into
-  // a list of register class indices. That list, along with the number of
-  // available mappings, is then used to create a new RegisterMappingTracker.
   void
   addRegisterFile(llvm::ArrayRef<llvm::MCRegisterCostEntry> RegisterClasses,
                   unsigned NumPhysRegs);
 
-  // Allocates register mappings in register file specified by the
+  // Consumes physical registers in each register file specified by the
-  // IndexPlusCostPairTy object. This method is called from addRegisterMapping.
+  // `IndexPlusCostPairTy`. This method is called from `addRegisterMapping()`.
   void allocatePhysRegs(IndexPlusCostPairTy IPC,
                         llvm::MutableArrayRef<unsigned> UsedPhysRegs);
 
-  // Removes a previously allocated mapping from the register file referenced
+  // Releases previously allocated physical registers from the register file(s)
-  // by the IndexPlusCostPairTy object. This method is called from
+  // referenced by the IndexPlusCostPairTy object. This method is called from
-  // invalidateRegisterMapping.
+  // `invalidateRegisterMapping()`.
   void freePhysRegs(IndexPlusCostPairTy IPC,
                     llvm::MutableArrayRef<unsigned> FreedPhysRegs);
 
   // Create an instance of RegisterMappingTracker for every register file
   // specified by the processor model.
-  // If no register file is specified, then this method creates a single
+  // If no register file is specified, then this method creates a default
-  // register file with an unbounded number of registers.
+  // register file with an unbounded number of physical registers.
   void initialize(const llvm::MCSchedModel &SM, unsigned NumRegs);
 
 public:
@@ -123,28 +122,26 @@ public:
     initialize(SM, NumRegs);
   }
 
-  // This method updates the data dependency graph by inserting a new register
+  // This method updates the register mappings inserting a new register
-  // definition. This method is also responsible for updating the number of used
+  // definition. This method is also responsible for updating the number of
-  // physical registers in the register file(s). The number of physical
+  // allocated physical registers in each register file modified by the write.
-  // registers is updated only if flag ShouldAllocatePhysRegs is set.
+  // No physical regiser is allocated when flag ShouldAllocatePhysRegs is set.
   void addRegisterWrite(WriteState &WS,
                         llvm::MutableArrayRef<unsigned> UsedPhysRegs,
                         bool ShouldAllocatePhysRegs = true);
 
-  // Updates the data dependency graph by removing a write. It also updates the
+  // Removes write \param WS from the register mappings.
-  // internal state of the register file(s) by freeing physical registers.
+  // Physical registers may be released to reflect this update.
-  // The number of physical registers is updated only if flag ShouldFreePhysRegs
-  // is set.
   void removeRegisterWrite(const WriteState &WS,
                            llvm::MutableArrayRef<unsigned> FreedPhysRegs,
                            bool ShouldFreePhysRegs = true);
 
-  // Checks if there are enough microarchitectural registers in the register
+  // Checks if there are enough physical registers in the register files.
-  // files.  Returns a "response mask" where each bit is the response from a
+  // Returns a "response mask" where each bit represents the response from a
-  // RegisterMappingTracker.
+  // different register file.  A mask of all zeroes means that all register
-  // For example: if all register files are available, then the response mask
+  // files are available.  Otherwise, the mask can be used to identify which
-  // is a bitmask of all zeroes. If Instead register file #1 is not available,
+  // register file was busy.  This sematic allows us classify dispatch dispatch
-  // then the response mask is 0b10.
+  // stalls caused by the lack of register file resources.
   unsigned isAvailable(llvm::ArrayRef<unsigned> Regs) const;
   void collectWrites(llvm::SmallVectorImpl<WriteState *> &Writes,
                      unsigned RegID) const;