[PATCH] D47244: [llvm-mca] Add the RetireStage.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In https://reviews.llvm.org/D47244#1111881, @mattd wrote:

> I like Andrea's suggestion, and that we still maintain the RCU as a separate component.
>
> This patch maintains that separation, but places ownership in the Backend.  Additionally, the RegisterFile (renamed to RF instead of RAT) is also another simulated hardware component that is owned by the Backend.  Any other MCA component that requires the use of such hardware abstraction will be passed pointers to those items during construction.


Nice patch. Thanks!

> The Owner pointer in the RetireStage (sorry) will hopefully disappear once we move the eventing (notification) logic from solely being owned by the Backend, into the the Stages.

No problem. One step at the time :-).

> This patch moves the notifyInstructionRetired and cycleEvent from the RCU into the RetireStage.

LGTM modulo a few nits (see comments below).

Thanks
-Andrea



================
Comment at: tools/llvm-mca/DispatchStage.cpp:33
 
-void DispatchStage::notifyInstructionRetired(const InstRef &IR) {
-  LLVM_DEBUG(dbgs() << "[E] Instruction Retired: " << IR << '\n');
-  SmallVector<unsigned, 4> FreedRegs(RAT->getNumRegisterFiles());
-  const InstrDesc &Desc = IR.getInstruction()->getDesc();
-
-  for (const std::unique_ptr<WriteState> &WS : IR.getInstruction()->getDefs())
-    RAT->removeRegisterWrite(*WS.get(), FreedRegs, !Desc.isZeroLatency());
-  Owner->notifyInstructionEvent(HWInstructionRetiredEvent(IR, FreedRegs));
-}
-
-bool DispatchStage::checkRAT(const InstRef &IR) {
+bool DispatchStage::checkRF(const InstRef &IR) {
   SmallVector<unsigned, 4> RegDefs;
----------------
s/checkRF/checkPRF


================
Comment at: tools/llvm-mca/DispatchStage.h:62-63
   const llvm::MCSubtargetInfo &STI;
+  RetireControlUnit *RCU;
+  RegisterFile *RF;
 
----------------
These should be references.
Also, could you please rename RF --> PRF (as in physical register file)?


================
Comment at: tools/llvm-mca/DispatchStage.h:92-93
                 const llvm::MCRegisterInfo &MRI, unsigned RegisterFileSize,
-                unsigned MaxDispatchWidth, Scheduler *Sched)
+                unsigned MaxDispatchWidth, RetireControlUnit *R, RegisterFile *F,
+                Scheduler *Sched)
       : DispatchWidth(MaxDispatchWidth), AvailableEntries(MaxDispatchWidth),
----------------
Pass the RCU and PRF by reference to the constructor of DispatchStage.


================
Comment at: tools/llvm-mca/RetireStage.h:31-32
+  Backend *Owner;
+  RetireControlUnit *RCU;
+  RegisterFile *RF;
+
----------------
Make these references.
Please rename RF into PRF.


================
Comment at: tools/llvm-mca/Scheduler.h:405
   const llvm::MCSchedModel &SM;
+  RetireControlUnit *RCU;
 
----------------
Make this a reference.


https://reviews.llvm.org/D47244