- User Since
- May 9 2013, 11:10 AM (267 w, 3 d)
Fri, Jun 22
Thanks for doing this!
Wed, Jun 20
Tue, Jun 19
Thanks Matt and Simon.
Mon, Jun 18
Abandoning this patch as there was no concrete plan to push this change Upstrea. It was mainly to help the review process for the RFC patches.
Fri, Jun 15
Well, I guess it doesn't hurt to have these new tests.
We already have tests for all the views and all the stats. But you are right, we don't have tests _only_ for those two flags.
Given that Clement (and presumably Greg) are okay with this change, then I won't oppose it. Sorry for being pedantic.
Not sure what other reviewers think about this.
However, my opinion is that the summary view should never be optional in llvm-mca.
It is only a few lines, and it gives a nice overview of the run.
Wed, Jun 13
Tue, Jun 5
It looks good to me.
I agree that this affects a lot of tests. However it looks like most of the diffs are for znver1, which apparently assigns an arbitrary 100cy latency , 1 uOp and no processor resource cycles to all microcoded instructions. So I am not particularly worried about it.
In the absence of extra/accurate schedule information, I think that computing an optimistic reciprocal throughput based on the number of opcodes and IssueWidth is the best that we can do.
Mon, Jun 4
Address Simon's review comment.
Patch updated. This time with the correct diff...
Address review comments.
Fri, Jun 1
Patch updated (sorry for the spam).
Wed, May 30
Sun, May 27
May 25 2018
Abandoning this patch in favor of D47374.
May 24 2018
May 23 2018
What I am trying to say is that the stage should only be responsible for the process (i.e. orchestrating the execution).
The logic that lives in Scheduler::cycleEvent() is essentially where the "process" logic is implemented. That logic (plus the event notification logic) should be moved to the stage class. The rest of the logic should still be part of the Scheduler class. Basically, the stage is only where we control the execution. It helps decoupling the actual feature from the process (i.e. how we use the feature/interact with the hardware components).
On a second thought,
wouldn't it be better if we still keep the RetireControlUnit abstraction?
We can have that the RetireStage acts as a proxy for the RetireControlUnit.
A couple of minor nits. Otherwise it LGTM too.
May 22 2018
May 21 2018
Address review comment.
Addressed review comments.
May 18 2018
Abandoning in favor of D47077.
Addressed review comment.
May 17 2018
May 16 2018
If the goal is to make the current DispatchUnit a DispatchStage, then this makes sense.
May 15 2018
May 14 2018
Addressed review comments.
May 11 2018
May 10 2018
Patch updated. Note that this patch still needs a test case.
May 7 2018
A few minor nits. Otherwise LGTM.
May 6 2018
May 5 2018
I'd say this regression is an improvement, since IPC increased in that case?