[PATCH] D44488: [llvm-mca] Refactor class RegisterFile to allow the definition of multiple register files.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

In https://reviews.llvm.org/D44488#1038972, @RKSimon wrote:

> <pedantic> I think physical registers is a better term than temporary registers, I don't know what other people think?


I agree in principle, but unfortunately LLVM already uses MCPhysReg to denote RAX/RBX/... so there is a risk of confusion. Agner sometimes uses "microarchitectural registers", which is a nice name, albeit a bit long. We could abbreviate to UarchReg ?

> Jaguar treats 1 ymm logical as (upto) 2 x 128-bit physical entries - real world perf effect is minimal, but it does mean that it performs (upto) 2 read/writes into the PRF. 128-bit physicals that are known zero don't take up an PRF entry and the read/write should be quicker.





================
Comment at: tools/llvm-mca/Dispatch.cpp:41
+      const MCRegisterClass &RC = MRI.getRegClass(RegClassIndex);
+      for (auto I = RC.begin(), E = RC.end(); I < E; ++I)
+        RegisterMappings[*I].second |= 1U << RegisterFileIndex;
----------------
why not use a range ?


================
Comment at: tools/llvm-mca/Dispatch.h:37
+  // Each register file is described by an instance of RegisterFileInfo.
+  // RegisterFileInfo track the number of temporary registers dynamically
+  // allocated during the execution.
----------------
typo: "tracks"


================
Comment at: tools/llvm-mca/Dispatch.h:39
+  // allocated during the execution.
+  struct RegisterFileInfo {
+    // Total number of temporary registers that are available for register
----------------
XXXInfo ususally represents immutable information about XXX. What about calling this RegisterMappingTracker or something along those lines ?


================
Comment at: tools/llvm-mca/Dispatch.h:43
+    // an unbound number of temporaries.
+    unsigned TotalMappings;
+    // Number of temporary registers that are currently in use.
----------------
const ?


================
Comment at: tools/llvm-mca/Dispatch.h:72
+  // A register file mask identifies a set of register files. Each bit of the
+  // mask representation identifies a specif register file.
+  // For example:
----------------
typo: specific


================
Comment at: tools/llvm-mca/Dispatch.h:108
+                       unsigned NumTemps);
+  void reserveTemporaries(unsigned RegisterFileMask);
+  void releaseTemporaries(unsigned RegisterFileMask);
----------------
Please document these.


https://reviews.llvm.org/D44488