Bug fix, tests, and assertion for stall condition #117
Conversation
core/Rename.cpp
Outdated
| @@ -356,4 +359,9 @@ namespace olympia | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| void Rename::onStartingTeardown_(){ | |||
| sparta_assert(current_stall_ == StallReason::NOT_STALLED, "Rename is stalled when entering teardown, possible lockup in Rename?") | |||
There was a problem hiding this comment.
@knute-sifive Was thinking either to have an assert like this to ensure that there is no ending stall when teardown begins, or should I do a notification similar to #108
There was a problem hiding this comment.
Yeah, you can't have this assertion. It's possible that Rename is stalled waiting for free registers AND the ROB was told to stop simulation on instruction N. This is a valid case and you don't want to assert on that.
In #108 (still under review at this time), having the ROB broadcast "I stopped simulation" and Rename listening for that and determining if it's a cause in this function is appropriate.
There was a problem hiding this comment.
I'll remove the assertion/teardown, and I'll add in the rename debugging with the teardown once #108 is merged.
ghost
assigned 
ghost
requested review from
| // decrement reference for store instruction to data register | ||
| if(inst_ptr->isStoreInst()) { | ||
| // decrement reference to data register | ||
| if(inst_ptr->isLoadStoreInst()){ |
There was a problem hiding this comment.
@danbone's fix is a good catch: noticing that the renaming and reclamation are not in sync
But there's something nagging me about all of this -- we're renaming x0. Can you open a new issue that fixes that?
Specifically, in renameInstructions_, when iterating over the sources and destinations, you need to skip renaming x0:
if (num == 0 && rf == core_types::RF_INTEGER) { continue; }
Fixing #116