Optimise some adders in CPU_Fetch_C#29
Open
ivanmgribeiro wants to merge 3 commits intobluespec:masterfrom
Open
Optimise some adders in CPU_Fetch_C#29ivanmgribeiro wants to merge 3 commits intobluespec:masterfrom
ivanmgribeiro wants to merge 3 commits intobluespec:masterfrom
Conversation
This removes an adder that is introduced in compilation and is not optimised away by synthesis (tested in Quartus Prime Standard Edition 18.1).
This commit replaces the check in CPU_Fetch_C for imem32.pc = rg_pc + 2 with a Bool register which when imem32.valid is True tells you whether the check returns True or False.
jrtc27
reviewed
Sep 14, 2020
src_Core/CPU/CPU_Fetch_C.bsv
Outdated
Comment on lines
243
to
259
| // Here, we know that addr[0:1] == 2'b10 (from is_addr_odd16). | ||
| // We also know that addr_of_b32 = addr with the bottom 2 bits set to 2'b0 | ||
| // ie addr_of_b32 = addr - 2 (1) | ||
| // Since after this if statement we make a request to imem32.req with the | ||
| // address addr_of_b32, the next time imem32.valid is True we will have | ||
| // imem32.pc = addr_of_b32 + 4 (2) | ||
| // so we will have imem32.pc = addr + 2 using (1) and (2) | ||
| // since we set rg_pc = addr above, we then get | ||
| // imem32.pc = rg_pc + 2 | ||
| rg_imem_pc_is_rg_pc_plus_2 <= True; | ||
| end | ||
| else begin | ||
| // This is similar to the first branch of this if statement, but we get either | ||
| // imem32.pc = rg_pc or imem32.pc = rg_pc - 2 | ||
| // In either case we can assert this to be False | ||
| rg_imem_pc_is_rg_pc_plus_2 <= False; | ||
| end |
Contributor
There was a problem hiding this comment.
These additions all need the first 8 spaces turning into a tab.
Author
There was a problem hiding this comment.
Thanks for the pointer, and sorry it took so long to fix this. It should be fixed now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request has two commits to reduce the number of adders in CPU_Fetch_C and reduce the area footprint of the design.
There is one change that replaces addr_of_b32 with imem32.pc when they are equal - I would expect either the Bluespec compiler to see this optimisation or the synthesis tool to optimise it away but it looks like neither of these things happen and the design ends up using an extra adder for no reason.
The other change replaces the check for
imem32.pc == rg_pc + 2with a Bool register that is set each time rg_pc is updated or a request is made toimem32.There are comments in the code that explain my reasoning for this.
I'm not sure if these comments should remain in the final design or not.
Let me know what you think.
Thanks,
Ivan