Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement LSU clear #499

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Implement LSU clear #499

merged 2 commits into from
Nov 13, 2023

Conversation

Kristopher38
Copy link
Member

This PR implements a proper clear method in dummyLSU with proper handling in LSUDummyInternals. As with most changes, bulk of the code is tests. Test code is a bit copy-paste'y as I didn't go for writing randomized tests (@lekcyjna123 already went through that route and it turned out to be a nightmare) and went with explicitly writing a sequence of steps that are supposed to test different conditions:

  • calling clear after instruction has been inserted into the LSU but before bus request was sent
  • calling clear after bus request was sent but before response arrived
  • calling clear after bus response arrived but wasn't read with get_result yet

This merges into feature/interrupts since that way I won't have to rebase against it (if it was merged into master).

Copy link
Contributor

@lekcyjna123 lekcyjna123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good and surprisingly simple :) Only one remark, to didn't forget about that in future, but it is an optimisation ;)

@@ -271,9 +276,9 @@ def _(rob_id: Value):

@def_method(m, self.clear)
def _():
# TODO: clearing internal lsu component ;)
m.d.sync += current_instr.eq(0)
m.d.sync += current_instr.valid.eq(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have ready "load" instruction to execute, and you call clear in the same cycle, then load will start regardless of clear because (self.execute | is_load) will be true and instr_ready will be false first in next cycle.

@tilk tilk merged commit 2e1326a into feature/interrupts Nov 13, 2023
4 of 5 checks passed
@tilk tilk deleted the kristopher38/lsu-clear branch November 13, 2023 08:53
Kristopher38 added a commit to Kristopher38/coreblocks that referenced this pull request Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants