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

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Thanks for the feedback.

I am going to upload an updated patch soon.

In https://reviews.llvm.org/D44488#1039016, @courbet wrote:

> 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 ?


I like "microarchitectural register". I will use it for now.



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


================
Comment at: tools/llvm-mca/Dispatch.h:43
+    // an unbound number of temporaries.
+    unsigned TotalMappings;
+    // Number of temporary registers that are currently in use.
----------------
courbet wrote:
> const ?
It can be changed by 'RegisterFile::isAvailable'.

Users can override the register file size using flag -register-file-size. If the number of "microarchitectural registers" is too small, there is a risk of deadlock in the dispatch logic. Method 'isAvailable' spots if there is a problems with the total number of mappings, and artificially increases to avoid problems. When the total number of mappings is created, a warning is generated to standard error to notify the user.

If you want, I can remove that check, and make this field const. We still want to identify potential deadlocks and abort with a fatal_error. What do you think?



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


https://reviews.llvm.org/D44488