-
Notifications
You must be signed in to change notification settings - Fork 5
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
ICACHE module #768
base: main
Are you sure you want to change the base?
ICACHE module #768
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will continue reviewing tomorrow
hit_index_q0 = i; | ||
end | ||
else begin | ||
hit_array_q0[i] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's review this together and potentially improve the coding style
( | ||
input logic clk, | ||
input logic rst, | ||
input logic [31:0] pcQ100H, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a valid bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a valid bit is located inside structs towards the core and towards the i_mem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's part of the struct.. I think we should have and independent valid bit instead of the valid in the structs..
|
||
`MAFIA_RST_DFF(prev_hit_index_q0, hit_index_q0, clk, rst) | ||
assign new_hit_index_q0 = ((prev_hit_index_q0 != hit_index_q0)); | ||
assign cache_full = (tag_valid_arr == 16'hffff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to parametrize the cache size
module i_mem #(parameter DATA_WIDTH = 128, parameter ADRS_WIDTH = 32) | ||
( | ||
input logic clock , | ||
input logic [ADRS_WIDTH-1:0] address , // TODO - possible to reduce size because we use only 28 bits pc[31:4] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should reduce to 28 bits.
( | ||
input logic clk, | ||
input logic rst, | ||
input logic [31:0] pcQ100H, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's part of the struct.. I think we should have and independent valid bit instead of the valid in the structs..
In that PR, I designed a fully associative cache with 16 cache lines (CLs) and implemented a PLRU (Pseudo-Least Recently Used) replacement mechanism. To improve the design's throughput, I aimed to minimize the number of states in the state machine.
The
i_cache_top.sv
file contains two modules: one for the cache and the other for the PLRU tree algorithm. The testbench,i_cache_top_tb.sv
, contains the test, which is relatively long. I verified the functionality using waveforms.I haven’t connected the
i_mem.sv
itself to the cache yet, but I tested it, and it’s working. I have also created a forced delay of 8 cycles.