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

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: tools/llvm-mca/Dispatch.cpp:153
+unsigned RegisterFile::isAvailable(const ArrayRef<unsigned> Regs) {
+  SmallVector<unsigned, 4> NumTemporaries(0U);
+  NumTemporaries.resize(getNumRegisterFiles());
----------------
remove "(0U)" ?


================
Comment at: tools/llvm-mca/Dispatch.h:43
+    // an unbound number of temporaries.
+    unsigned TotalMappings;
+    // Number of temporary registers that are currently in use.
----------------
andreadb wrote:
> 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?
> 
I see. Dying sounds reasonable. It does not really make sense for a microarchitecture to not even have enough microarchitectural registers to handle the registers its ISA defines. The user should know (or be told :) ) that.


https://reviews.llvm.org/D44488