[PATCH] D45259: [MC][Tablegen] Allow models to describe the retire control unit for llvm-mca.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

Why did the tests change ? I thought that this change would not impact tests.



================
Comment at: tools/llvm-mca/Dispatch.cpp:262
+      AvailableSlots = EPI.ReorderBufferSize;
+    if (!MaxRetirePerCycle)
+      MaxRetirePerCycle = EPI.MaxRetirePerCycle;
----------------
andreadb wrote:
> courbet wrote:
> > Can we get rid of the MaxRetirePerCycle ctor parameter ? Is there any reason why the user would override this ?
> The original idea was to let users override the retire-per-cycle.
> However, with this patch, the max-retire-per-cycle can now be set via tablegen.
> 
> That being said, most processors don't provide a max-retire-per-cycle. This patch would only fix the BtVer2 model.
> So, I am tempted to keep the flag for now (although, I don't have a strong opinion about it). Alternatively, I can commit a separate patch that removes the flag, and the rebase this patch. What do you think?
> 
I'm pretty sure that I would stick to tablegen-specified models as a user. And if you're developing you own processor, I hope that you have your own tools :) So the flag does not sound that useful to me given that it makes a bit harder to understand this code.
I don't have a strong opinion though, so feel free to keep the flag if i'ts useful to you.


https://reviews.llvm.org/D45259