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

Faster Wishbone #505

Merged
merged 10 commits into from
Nov 13, 2023
Merged

Faster Wishbone #505

merged 10 commits into from
Nov 13, 2023

Conversation

tilk
Copy link
Member

@tilk tilk commented Nov 5, 2023

This PR improves the performance of the Wishbone master. It's now possible to achieve full utilization of the Wishbone bus: 2 cycles per read and 1 cycle per (unregistered) write. IPC in benchmarks is improved by 12% without Fmax loss.

Truly, Forwarder is a magic component.

@tilk tilk added the optimization This is *just* an optimization! label Nov 5, 2023
@tilk tilk added this to the Improve the core's performance milestone Nov 5, 2023
@tilk tilk requested a review from piotro888 November 6, 2023 12:18
test/peripherals/test_wishbone.py Show resolved Hide resolved
test/peripherals/test_wishbone.py Show resolved Hide resolved
test/peripherals/test_wishbone.py Show resolved Hide resolved
@@ -325,6 +352,7 @@ def mem_op_process():
yield from self.m.request.call(addr=addr, data=data, we=write, sel=sel)
res = yield from self.m.result.call()
if write:
yield # workaround, memory state will change the next cycle!
Copy link
Contributor

Choose a reason for hiding this comment

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

But this cause that we don't test if request in the first cycle after write works properly.

Copy link
Member Author

Choose a reason for hiding this comment

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

The behavior of the request itself is already tested in TestWishboneMaster.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the test to allow writes without delays, also added random latencies.

Copy link
Member

@piotro888 piotro888 left a comment

Choose a reason for hiding this comment

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

Apart from @lekcyjna123 comment for TestWishboneMemorySlave, looks good to me

@tilk tilk force-pushed the tilk/faster-wishbone branch from d64dcfc to b0d9757 Compare November 13, 2023 11:10
@tilk
Copy link
Member Author

tilk commented Nov 13, 2023

There was an issue in receiving results from the Wishbone master: if there was latency at receive, requests could disappear. Fixed.

@tilk tilk requested a review from lekcyjna123 November 13, 2023 11:17
@tilk tilk merged commit 78f66cc into master Nov 13, 2023
6 checks passed
@tilk tilk deleted the tilk/faster-wishbone branch November 13, 2023 13:31
github-actions bot pushed a commit that referenced this pull request Nov 13, 2023
tilk added a commit to kuznia-rdzeni/transactron that referenced this pull request Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization This is *just* an optimization!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants