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

Single cycle mult #295

Closed
wants to merge 4 commits into from
Closed

Single cycle mult #295

wants to merge 4 commits into from

Conversation

GregAC
Copy link
Contributor

@GregAC GregAC commented Oct 30, 2024

This sits on top of #279 and shouldn't be merged until that has been merged.

Using v2021.1 enabling the single cycle multiplier works fine, no major change in implementation time and implementation results are good. I'd avoided doing it initially as it wasn't working so well under v2023.1/v2024.1 but as has been discovered those versions don't implement Sonata as well for whatever reason (#289)

Need a minimum of 2 (this is what is used in OpenTitan) to enable back
to back requests without stall cycles.
Update code from upstream repository
https://github.com/lowrisc/cheriot-ibex.git to revision
ea2df9db3bcea776f0dc72d6d89c31c73798ecd4

* Feed RV32M through ibexc_top_tracing/ibexc_top (Greg Chadwick)
* Switch to no bitmanip by default (Greg Chadwick)
* Feed RV32B through in ibexc_top (Greg Chadwick)

Signed-off-by: Greg Chadwick <gac@lowrisc.org>
This is effectively a no-op change. Before the latest Ibex was vendored
we had no bitmanip (the RV32BFull parameter was not fully passed
through) and RV32M was the fast multiplier.

Sadly the single cycle multiplier seems to be increasing timing
pressure. It does just meet timing but greatly increases synthesis
times. As it's implemented with in-built FPGA DSP blocks it shouldn't be
a big issue to use it so something to examine here but for now leave
things as they are.
@GregAC
Copy link
Contributor Author

GregAC commented Oct 30, 2024

For reviewing this is the relevant commit 03d918a

@marnovandermaas marnovandermaas marked this pull request as draft October 30, 2024 17:33
@marnovandermaas
Copy link
Contributor

Converting to draft until SRAM PR is done.

@@ -734,7 +734,7 @@ module sonata_system
.HeapBase ( tl_main_pkg::ADDR_SPACE_SRAM ),
.TSMapBase ( tl_main_pkg::ADDR_SPACE_REV_TAG ),
.TSMapSize ( RevTagDepth ),
.RV32M ( ibex_pkg::RV32MFast ),
.RV32M ( ibex_pkg::RV32MSingleCycle ),
Copy link
Contributor

Choose a reason for hiding this comment

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

In principle looks fine to me as long as we can meet timing.

@GregAC
Copy link
Contributor Author

GregAC commented Nov 3, 2024

Closing in favour of #314

@GregAC GregAC closed this Nov 3, 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.

2 participants