Conversation
… set for parallel tag lookup and output
…meters Added handling for translating VA to respective PA. Added sequential logic for handling state transitions on clk edge. Removed ports for L2.
…face ports, comb miss logic added ports to interface with the replacement/eviction module. Setup axi ports (subject to change). Setup combination logic for handling 'miss' state.
Anthony cache -> cache_branch
Carter cache -> cache_branch
|
need to still work on AXI interfaces; dcache still requires work for write back / write allocate mru.sv is current replacement policy module |
feat: added prelim logic for write allocate and write back policies
Changed misspelled port definition from "dcahce_tlb_req_valid_o" to ""dcache_tlb_req_valid_o"
Meowcaroni
left a comment
Meowcaroni
left a comment
There was a problem hiding this comment.
Good job overall, dcache and icache are especially well-organized. I also appreciate the header and body comments in general.
dcache.sv and lru.sv have some necessary fixes to-do regarding combinational and sequential logic, but everything else is good/can be fixed later.
|
|
||
| CACHE_LOOKUP: begin | ||
| dcache_tlb_req_valid_o = 1'b1; | ||
| dcache_tlb_resp_ready = 1'b0; |
There was a problem hiding this comment.
Functionality Issue: Since dcache_tlb_resp_ready (what I'm assuming is meant to be "dcache_tlb_resp_ready_o") is treated sequentially with non-blocking statements in the TLB Translation Logic block, this may conflict with the combinational usage here.
Delete this line if that's the case.
|
|
||
| CACHE_ALLOCATE: begin | ||
| dcache_bridge_req_valid_o = 1'b1; | ||
| dcache_bridge_req_addr_o = phys_addr; |
There was a problem hiding this comment.
Functionality Issue: dcache_bridge_req_addr_o also faces the same issue as dcache_tlb_resp_ready_o with the sequential block in Cache Allocate.
Delete this line or its usage in that block.
|
|
||
| CACHE_WRITEBACK: begin | ||
| dcache_bridge_req_valid_o = 1'b1; | ||
| dcache_bridge_req_addr_o = phys_addr; |
There was a problem hiding this comment.
Functionality Issue: dcache_bridge_req_addr_o also faces the same issue as dcache_tlb_resp_ready_o with the sequential block in Cache Writeback.
Delete this line or its usage in that block.
| dcache_resp_valid_o = 1'b0; | ||
| dcache_resp_rdata_o = '0; |
There was a problem hiding this comment.
Functionality Issue: dcache_resp_valid_o and dcache_resp_rdata_o have the same issue with combinational/sequential block driving in the CPU Load block
Alter its usage here and/or in that block.
| if (way_to_evict_o == 1'b0) assign evicted_line_o = way1_line_i; | ||
| else assign evicted_line_o = way2_line_i; |
There was a problem hiding this comment.
Functionality Issue: If-else statements cannot be used in the module's top scope (directly within module) to select individual assign statements. In this case, I recommend using a conditional operator:
| if (way_to_evict_o == 1'b0) assign evicted_line_o = way1_line_i; | |
| else assign evicted_line_o = way2_line_i; | |
| assign evicted_line_o = (way_to_evict_o == 1'b0) ? way1_line_i : way2_line_i; |
There was a problem hiding this comment.
For scalability, any "way" variables can be converted into packed/unpacked arrays with a WAYS parameter. For example: input logic [WAYS-1:0] isWay_valid_i
(Note: This can be added later on, not a top priority)
| end else if (isthere_miss_i) begin | ||
|
|
||
| // if there's a miss, invert bit to track the victim way | ||
| mru_bits[set_index_i] <= way_to_evict_o; |
There was a problem hiding this comment.
Functionality Note: This may cause a race condition with way_to_evict_o = ~mru_bits[set_index_i]; earlier. Consider changing if possible.
3 participants
No description provided.