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

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In https://reviews.llvm.org/D45259#1058226, @courbet wrote:

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


It does have an impact on the retire stage.

On BtVer2, the retire throughput is 2 instructions per cycle. It does affect the timeline at least.



================
Comment at: tools/llvm-mca/Dispatch.cpp:262
+      AvailableSlots = EPI.ReorderBufferSize;
+    if (!MaxRetirePerCycle)
+      MaxRetirePerCycle = EPI.MaxRetirePerCycle;
----------------
courbet wrote:
> 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.
That's a good point (especially if you have your own tools).
It could have been used to experiment/play with the RCU (with SMT processors we might emulate a different retire throughput).
That being said, the argument is a bit weak. SMT introduces other issues related to the partitioning of resources which cannot be fully tweaked via flags.

If okay for you, I am going to commit a change that removes the flag and updates the docs in preparation for this patch. Then I rebase this patch.


https://reviews.llvm.org/D45259