From 68a01cb0f87b5c175bfd2fd5af56211dfaacc2d6 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 11 Apr 2023 16:59:11 -0700 Subject: [PATCH 01/10] Exclude (FlushStage & SetValidWay) condition for RO caches Spent a long time trying to find a way to see if this condition was possible, only to become relativly convinced that it isn't. Basically, since RO cache writes only happen after a long period of stall for the bus access, there's no way a flushD can be active at the same time as a RO cache write. TrapM causes a FlushD, but interrupts are gated by the "commited" logic and the exception pipeline stalls. I feel like its worth keeping the logic to be safe so I've chosen to exclude it rather than explicitely remove it. --- src/cache/cacheway.sv | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index 174b82c59..568e626e5 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -101,14 +101,21 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, if (!READ_ONLY_CACHE) begin assign SetDirtyWay = SetDirty & SelData; assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage; + assign SetValidEN = SetValidWay & ~FlushStage; end else begin + // Don't cover FlushStage assertion during SetValidWay. + // it's not explicitely gated anywhere, but for read-only caches, + // there's no way that a FlushD can happen during the write stage + // of a fetch. + // coverage off -item e 1 -fecexprrow 4 assign SelectedWriteWordEn = SetValidWay & ~FlushStage; + // coverage off -item e 1 -fecexprrow 4 + assign SetValidEN = SetValidWay & ~FlushStage; end // If writing the whole line set all write enables to 1, else only set the correct word. assign FinalByteMask = SetValidWay ? '1 : LineByteMask; // OR - assign SetValidEN = SetValidWay & ~FlushStage; ///////////////////////////////////////////////////////////////////////////////////////////// // Tag Array From 7c9f68e9843e5ba70415aef55381c21295442dd3 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 11 Apr 2023 17:10:09 -0700 Subject: [PATCH 02/10] Remove FlushStage Logic from CacheLRU For coverage. LRUWriteEn is gated by FlushStage in cache.sv, so removing the signal completely avoids future confusion. Update cache.sv to reflect cacheLRU edit. --- src/cache/cache.sv | 2 +- src/cache/cacheLRU.sv | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/cache/cache.sv b/src/cache/cache.sv index c01c714b1..9854152e2 100644 --- a/src/cache/cache.sv +++ b/src/cache/cache.sv @@ -122,7 +122,7 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE // Select victim way for associative caches if(NUMWAYS > 1) begin:vict cacheLRU #(NUMWAYS, SETLEN, OFFSETLEN, NUMLINES) cacheLRU( - .clk, .reset, .CacheEn, .FlushStage, .HitWay, .ValidWay, .VictimWay, .CacheSet, .LRUWriteEn(LRUWriteEn & ~FlushStage), + .clk, .reset, .CacheEn, .HitWay, .ValidWay, .VictimWay, .CacheSet, .LRUWriteEn(LRUWriteEn & ~FlushStage), .SetValid, .PAdr(PAdr[SETTOP-1:OFFSETLEN]), .InvalidateCache, .FlushCache); end else assign VictimWay = 1'b1; // one hot. diff --git a/src/cache/cacheLRU.sv b/src/cache/cacheLRU.sv index 1e7101365..d8de983f7 100644 --- a/src/cache/cacheLRU.sv +++ b/src/cache/cacheLRU.sv @@ -32,8 +32,7 @@ module cacheLRU #(parameter NUMWAYS = 4, SETLEN = 9, OFFSETLEN = 5, NUMLINES = 128) ( input logic clk, - input logic reset, - input logic FlushStage, // Pipeline flush of second stage (prevent writes and bus operations) + input logic reset, input logic CacheEn, // Enable the cache memory arrays. Disable hold read data constant input logic [NUMWAYS-1:0] HitWay, // Which way is valid and matches PAdr's tag input logic [NUMWAYS-1:0] ValidWay, // Which ways for a particular set are valid, ignores tag @@ -134,11 +133,9 @@ module cacheLRU always_ff @(posedge clk) begin if (reset) for (int set = 0; set < NUMLINES; set++) LRUMemory[set] <= '0; if(CacheEn) begin - // if((InvalidateCache | FlushCache) & ~FlushStage) for (int set = 0; set < NUMLINES; set++) LRUMemory[set] <= '0; - if (LRUWriteEn & ~FlushStage) begin + if(LRUWriteEn) LRUMemory[PAdr] <= NextLRU; - end - if(LRUWriteEn & ~FlushStage & (PAdr == CacheSet)) + if(LRUWriteEn & (PAdr == CacheSet)) CurrLRU <= #1 NextLRU; else CurrLRU <= #1 LRUMemory[CacheSet]; From 5b8c6f070e0eef2f7494c48dfba1932779185bde Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 11 Apr 2023 23:05:04 -0700 Subject: [PATCH 03/10] Make AdrSelMux and CacheBusAdrMux mux2 if READ_ONLY_CACHE Some address options are only used in the D$ case. --- src/cache/cache.sv | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/cache/cache.sv b/src/cache/cache.sv index 9854152e2..19cc84c51 100644 --- a/src/cache/cache.sv +++ b/src/cache/cache.sv @@ -73,7 +73,6 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE logic SelAdr; - logic [1:0] AdrSelMuxSel; logic [SETLEN-1:0] CacheSet; logic [LINELEN-1:0] LineWriteData; logic ClearDirty, SetDirty, SetValid; @@ -109,10 +108,18 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE // and FlushAdr when handling D$ flushes // The icache must update to the newest PCNextF on flush as it is probably a trap. Trap // sets PCNextF to XTVEC and the icache must start reading the instruction. - assign AdrSelMuxSel = {SelFlush, ((SelAdr | SelHPTW) & ~((READ_ONLY_CACHE == 1) & FlushStage))}; - mux3 #(SETLEN) AdrSelMux(NextSet[SETTOP-1:OFFSETLEN], PAdr[SETTOP-1:OFFSETLEN], FlushAdr, + if (!READ_ONLY_CACHE) begin + logic [1:0] AdrSelMuxSel; + assign AdrSelMuxSel = {SelFlush, SelAdr | SelHPTW}; + mux3 #(SETLEN) AdrSelMux(NextSet[SETTOP-1:OFFSETLEN], PAdr[SETTOP-1:OFFSETLEN], FlushAdr, AdrSelMuxSel, CacheSet); - + end + else begin + logic AdrSelMuxSel; + assign AdrSelMuxSel = ((SelAdr | SelHPTW) & ~FlushStage); + mux2 #(SETLEN) AdrSelMux(NextSet[SETTOP-1:OFFSETLEN], PAdr[SETTOP-1:OFFSETLEN], + AdrSelMuxSel, CacheSet); + end // Array of cache ways, along with victim, hit, dirty, and read merging logic cacheway #(NUMLINES, LINELEN, TAGLEN, OFFSETLEN, SETLEN, READ_ONLY_CACHE) CacheWays[NUMWAYS-1:0]( .clk, .reset, .CacheEn, .CacheSet, .PAdr, .LineWriteData, .LineByteMask, @@ -152,11 +159,14 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE .PAdr(WordOffsetAddr), .ReadDataLine, .ReadDataWord); // Bus address for fetch, writeback, or flush writeback - mux3 #(`PA_BITS) CacheBusAdrMux(.d0({PAdr[`PA_BITS-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), - .d1({Tag, PAdr[SETTOP-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), - .d2({Tag, FlushAdr, {OFFSETLEN{1'b0}}}), - .s({SelFlush, SelWriteback}), .y(CacheBusAdr)); - + if (!READ_ONLY_CACHE) + mux3 #(`PA_BITS) CacheBusAdrMux(.d0({PAdr[`PA_BITS-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), + .d1({Tag, PAdr[SETTOP-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), + .d2({Tag, FlushAdr, {OFFSETLEN{1'b0}}}), + .s({SelFlush, SelWriteback}), .y(CacheBusAdr)); + else + assign CacheBusAdr = {PAdr[`PA_BITS-1:OFFSETLEN], {OFFSETLEN{1'b0}}}; + ///////////////////////////////////////////////////////////////////////////////////////////// // Write Path ///////////////////////////////////////////////////////////////////////////////////////////// From a1bbcd5e8a67f835486d57592d85743b04f72ee5 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 11 Apr 2023 23:05:56 -0700 Subject: [PATCH 04/10] Coverage and readability improvements to LRUUpdate logic The genvar stuff was switched to readable names to make it easier to understand for the first time. In the LRUUpdate logic for loop, a special case was added for simpler logic in the case of the root node, to hit coverage. --- src/cache/cacheLRU.sv | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/cache/cacheLRU.sv b/src/cache/cacheLRU.sv index d8de983f7..5f1c199a9 100644 --- a/src/cache/cacheLRU.sv +++ b/src/cache/cacheLRU.sv @@ -89,16 +89,26 @@ module cacheLRU assign WayExpanded[StartIndex : EndIndex] = {{DuplicationFactor}{WayEncoded[row]}}; end - genvar r, a, s; + genvar node; assign LRUUpdate[NUMWAYS-2] = '1; - for(s = NUMWAYS-2; s >= NUMWAYS/2; s--) begin : enables - localparam p = NUMWAYS - s - 1; - localparam g = log2(p); - localparam t0 = s - p; - localparam t1 = t0 - 1; - localparam r = LOGNUMWAYS - g; - assign LRUUpdate[t0] = LRUUpdate[s] & ~WayEncoded[r]; - assign LRUUpdate[t1] = LRUUpdate[s] & WayEncoded[r]; + for(node = NUMWAYS-2; node >= NUMWAYS/2; node--) begin : enables + localparam ctr = NUMWAYS - node - 1; + localparam ctr_depth = log2(ctr); + localparam lchild = node - ctr; + localparam rchild = lchild - 1; + localparam r = LOGNUMWAYS - ctr_depth; + + // the child node will be updated if its parent was updated and + // the WayEncoded bit was the correct value. + // The if statement is only there for coverage since LRUUpdate[root] is always 1. + if (node == NUMWAYS-2) begin + assign LRUUpdate[lchild] = ~WayEncoded[r]; + assign LRUUpdate[rchild] = WayEncoded[r]; + end + else begin + assign LRUUpdate[lchild] = LRUUpdate[node] & ~WayEncoded[r]; + assign LRUUpdate[rchild] = LRUUpdate[node] & WayEncoded[r]; + end end // The root node of the LRU tree will always be selected in LRUUpdate. No mux needed. @@ -106,15 +116,15 @@ module cacheLRU mux2 #(1) LRUMuxes[NUMWAYS-3:0](CurrLRU[NUMWAYS-3:0], ~WayExpanded[NUMWAYS-3:0], LRUUpdate[NUMWAYS-3:0], NextLRU[NUMWAYS-3:0]); // Compute next victim way. - for(s = NUMWAYS-2; s >= NUMWAYS/2; s--) begin - localparam t0 = 2*s - NUMWAYS; + for(node = NUMWAYS-2; node >= NUMWAYS/2; node--) begin + localparam t0 = 2*node - NUMWAYS; localparam t1 = t0 + 1; - assign Intermediate[s] = CurrLRU[s] ? Intermediate[t0] : Intermediate[t1]; + assign Intermediate[node] = CurrLRU[node] ? Intermediate[t0] : Intermediate[t1]; end - for(s = NUMWAYS/2-1; s >= 0; s--) begin - localparam int0 = (NUMWAYS/2-1-s)*2; + for(node = NUMWAYS/2-1; node >= 0; node--) begin + localparam int0 = (NUMWAYS/2-1-node)*2; localparam int1 = int0 + 1; - assign Intermediate[s] = CurrLRU[s] ? int1[LOGNUMWAYS-1:0] : int0[LOGNUMWAYS-1:0]; + assign Intermediate[node] = CurrLRU[node] ? int1[LOGNUMWAYS-1:0] : int0[LOGNUMWAYS-1:0]; end logic [NUMWAYS-1:0] FirstZero; From 4cbb9bcec686e9dafd377ed2efe0386097fd5523 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 12 Apr 2023 00:48:06 -0700 Subject: [PATCH 05/10] refactor cachefsm to get full coverage I had to exclude i$ states in coverage-exclusions-rv64gc.do, but it's referred to by scope, which should be pretty robust --- sim/coverage-exclusions-rv64gc.do | 6 + src/cache/cachefsm.sv | 195 +++++++++++++++++++----------- 2 files changed, 128 insertions(+), 73 deletions(-) diff --git a/sim/coverage-exclusions-rv64gc.do b/sim/coverage-exclusions-rv64gc.do index d58e4c514..3e880b502 100644 --- a/sim/coverage-exclusions-rv64gc.do +++ b/sim/coverage-exclusions-rv64gc.do @@ -32,6 +32,12 @@ # This is ugly to exlcude the whole file - is there a better option? // coverage off isn't working coverage exclude -srcfile lzc.sv +# Exclude D$ states from coverage in the I$ instance of cachefsm. +# This is cleaner than trying to set an I$-specific pragma in cachefsm.sv +# Also exclude the write line to ready transition for the I$ since we can't get a flush +# during this operation. +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -fstate CurrState STATE_FLUSH STATE_FLUSH_WRITEBACK STATE_FLUSH_WRITEBACK +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -ftrans CurrState STATE_WRITE_LINE->STATE_READY ###################### # Toggle exclusions diff --git a/src/cache/cachefsm.sv b/src/cache/cachefsm.sv index d1d54097e..a42176325 100644 --- a/src/cache/cachefsm.sv +++ b/src/cache/cachefsm.sv @@ -86,18 +86,25 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( statetype CurrState, NextState; - assign AMO = CacheAtomic[1] & (&CacheRW); - assign StoreAMO = AMO | CacheRW[0]; + // no atomic operations on i$ + if (!READ_ONLY_CACHE) begin + assign AMO = CacheAtomic[1] & (&CacheRW); + assign StoreAMO = AMO | CacheRW[0]; + assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; + assign AnyUpdateHit = StoreAMO & CacheHit; + assign AnyHit = AnyUpdateHit | (CacheRW[1] & CacheHit); + assign CacheAccess = (AMO | CacheRW[1] | CacheRW[0]) & CurrState == STATE_READY; // for performance counter + end + else begin + assign AnyMiss = CacheRW[1] & ~CacheHit & ~InvalidateCache; + assign AnyUpdateHit = 0; // todo clear all RO cache of usage of this logic + assign AnyHit = CacheRW[1] & CacheHit; + assign CacheAccess = CacheRW[1] & CurrState == STATE_READY; // for performance counter + end - assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; - assign AnyUpdateHit = (StoreAMO) & CacheHit; - assign AnyHit = AnyUpdateHit | (CacheRW[1] & CacheHit); + assign CacheMiss = CacheAccess & ~CacheHit; // for performance counter assign FlushFlag = FlushAdrFlag & FlushWayFlag; - // outputs for the performance counters. - assign CacheAccess = (AMO | CacheRW[1] | CacheRW[0]) & CurrState == STATE_READY; - assign CacheMiss = CacheAccess & ~CacheHit; - // special case on reset. When the fsm first exists reset the // PCNextF will no longer be pointing to the correct address. // But PCF will be the reset vector. @@ -106,77 +113,119 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( always_ff @(posedge clk) if (reset | FlushStage) CurrState <= #1 STATE_READY; else CurrState <= #1 NextState; - - always_comb begin - NextState = STATE_READY; - case (CurrState) - STATE_READY: if(InvalidateCache) NextState = STATE_READY; - else if(FlushCache & ~READ_ONLY_CACHE) NextState = STATE_FLUSH; - else if(AnyMiss & (READ_ONLY_CACHE | ~LineDirty)) NextState = STATE_FETCH; - else if(AnyMiss & LineDirty) NextState = STATE_WRITEBACK; - else NextState = STATE_READY; - STATE_FETCH: if(CacheBusAck) NextState = STATE_WRITE_LINE; - else NextState = STATE_FETCH; - STATE_WRITE_LINE: NextState = STATE_READ_HOLD; - STATE_READ_HOLD: if(Stall) NextState = STATE_READ_HOLD; - else NextState = STATE_READY; - STATE_WRITEBACK: if(CacheBusAck) NextState = STATE_FETCH; - else NextState = STATE_WRITEBACK; - // eviction needs a delay as the bus fsm does not correctly handle sending the write command at the same time as getting back the bus ack. - STATE_FLUSH: if(LineDirty) NextState = STATE_FLUSH_WRITEBACK; - else if (FlushFlag) NextState = STATE_READ_HOLD; - else NextState = STATE_FLUSH; - STATE_FLUSH_WRITEBACK: if(CacheBusAck & ~FlushFlag) NextState = STATE_FLUSH; - else if(CacheBusAck) NextState = STATE_READ_HOLD; - else NextState = STATE_FLUSH_WRITEBACK; - default: NextState = STATE_READY; - endcase - end + + // seperating NextState logic by ro vs rw cache results in code duplication but this is needed to hit coverage. + if (!READ_ONLY_CACHE) + always_comb begin + NextState = STATE_READY; + case (CurrState) + STATE_READY: if(InvalidateCache) NextState = STATE_READY; + else if(FlushCache) NextState = STATE_FLUSH; + else if(AnyMiss & ~LineDirty) NextState = STATE_FETCH; + else if(AnyMiss & LineDirty) NextState = STATE_WRITEBACK; + else NextState = STATE_READY; + STATE_FETCH: if(CacheBusAck) NextState = STATE_WRITE_LINE; + else NextState = STATE_FETCH; + STATE_WRITE_LINE: NextState = STATE_READ_HOLD; + STATE_READ_HOLD: if(Stall) NextState = STATE_READ_HOLD; + else NextState = STATE_READY; + STATE_WRITEBACK: if(CacheBusAck) NextState = STATE_FETCH; + else NextState = STATE_WRITEBACK; + // eviction needs a delay as the bus fsm does not correctly handle sending the write command at the same + // time as getting back the bus ack. + STATE_FLUSH: if(LineDirty) NextState = STATE_FLUSH_WRITEBACK; + else if (FlushFlag) NextState = STATE_READ_HOLD; + else NextState = STATE_FLUSH; + STATE_FLUSH_WRITEBACK: if(CacheBusAck & ~FlushFlag) NextState = STATE_FLUSH; + else if(CacheBusAck) NextState = STATE_READ_HOLD; + else NextState = STATE_FLUSH_WRITEBACK; + default: NextState = STATE_READY; + endcase + end // always_comb + else // READ_ONLY_CACHE + always_comb begin + NextState = STATE_READY; + case (CurrState) + STATE_READY: if(InvalidateCache) NextState = STATE_READY; + else if(AnyMiss) NextState = STATE_FETCH; + else NextState = STATE_READY; + STATE_FETCH: if(CacheBusAck) NextState = STATE_WRITE_LINE; + else NextState = STATE_FETCH; + STATE_WRITE_LINE: NextState = STATE_READ_HOLD; + STATE_READ_HOLD: if(Stall) NextState = STATE_READ_HOLD; + else NextState = STATE_READY; + default: NextState = STATE_READY; + endcase // case (CurrState) + end // always_comb // com back to CPU - assign CacheCommitted = (CurrState != STATE_READY) & ~(READ_ONLY_CACHE & CurrState == STATE_READ_HOLD); - assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | - (CurrState == STATE_FETCH) | - (CurrState == STATE_WRITEBACK) | - (CurrState == STATE_WRITE_LINE) | // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write. - (CurrState == STATE_FLUSH) | + if (!READ_ONLY_CACHE) begin + assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | + (CurrState == STATE_FETCH) | + (CurrState == STATE_WRITEBACK) | + (CurrState == STATE_WRITE_LINE) | // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write. + (CurrState == STATE_FLUSH) | + (CurrState == STATE_FLUSH_WRITEBACK); + assign CacheCommitted = CurrState != STATE_READY; + assign SelAdr = (CurrState == STATE_READY & (StoreAMO | AnyMiss)) | // changes if store delay hazard removed + (CurrState == STATE_FETCH) | + (CurrState == STATE_WRITEBACK) | + (CurrState == STATE_WRITE_LINE) | + resetDelay; + assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) | + (CurrState == STATE_WRITE_LINE & (StoreAMO)); + assign ClearDirty = (CurrState == STATE_WRITE_LINE & ~(StoreAMO)) | + (CurrState == STATE_FLUSH & LineDirty); // This is wrong in a multicore snoop cache protocal. Dirty must be cleared concurrently and atomically with writeback. For single core cannot clear after writeback on bus ack and change flushadr. Clears the wrong set. + + // Flush and eviction controls + assign SelWriteback = (CurrState == STATE_WRITEBACK & ~CacheBusAck) | + (CurrState == STATE_READY & AnyMiss & LineDirty); + + assign SelFlush = (CurrState == STATE_READY & FlushCache) | + (CurrState == STATE_FLUSH) | (CurrState == STATE_FLUSH_WRITEBACK); + assign FlushAdrCntEn = (CurrState == STATE_FLUSH_WRITEBACK & FlushWayFlag & CacheBusAck) | + (CurrState == STATE_FLUSH & FlushWayFlag & ~LineDirty); + assign FlushWayCntEn = (CurrState == STATE_FLUSH & ~LineDirty) | + (CurrState == STATE_FLUSH_WRITEBACK & CacheBusAck); + assign FlushCntRst = (CurrState == STATE_FLUSH & FlushFlag & ~LineDirty) | + (CurrState == STATE_FLUSH_WRITEBACK & FlushFlag & CacheBusAck); + // Bus interface controls + assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss & ~LineDirty) | + (CurrState == STATE_FETCH & ~CacheBusAck) | + (CurrState == STATE_WRITEBACK & CacheBusAck); + assign CacheBusRW[0] = (CurrState == STATE_READY & AnyMiss & LineDirty) | + (CurrState == STATE_WRITEBACK & ~CacheBusAck) | + (CurrState == STATE_FLUSH_WRITEBACK & ~CacheBusAck); + // write enable internal to cache + assign CacheEn = (~Stall | FlushCache | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; + end + else begin + assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | + (CurrState == STATE_FETCH) | + (CurrState == STATE_WRITE_LINE); // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write. + assign CacheCommitted = (CurrState != STATE_READY) & ~(CurrState == STATE_READ_HOLD); + assign SelAdr = (CurrState == STATE_READY & AnyMiss) | // changes if store delay hazard removed + (CurrState == STATE_FETCH) | + (CurrState == STATE_WRITE_LINE) | + resetDelay; + assign SetDirty = 0; + assign ClearDirty = 0; + assign SelWriteback = 0; + assign SelFlush = 0; + assign FlushAdrCntEn = 0; + assign FlushWayCntEn = 0; + assign FlushCntRst = 0; + assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss) | + (CurrState == STATE_FETCH & ~CacheBusAck); + assign CacheBusRW[0] = 0; + assign CacheEn = (~Stall | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; + end // else: (READ_ONLY_CACHE) + // write enables internal to cache assign SetValid = CurrState == STATE_WRITE_LINE; - assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) | - (CurrState == STATE_WRITE_LINE & (StoreAMO)); - assign ClearDirty = (CurrState == STATE_WRITE_LINE & ~(StoreAMO)) | - (CurrState == STATE_FLUSH & LineDirty); // This is wrong in a multicore snoop cache protocal. Dirty must be cleared concurrently and atomically with writeback. For single core cannot clear after writeback on bus ack and change flushadr. Clears the wrong set. assign LRUWriteEn = (CurrState == STATE_READY & AnyHit) | (CurrState == STATE_WRITE_LINE); - // Flush and eviction controls - assign SelWriteback = (CurrState == STATE_WRITEBACK & ~CacheBusAck) | - (CurrState == STATE_READY & AnyMiss & LineDirty); - - assign SelFlush = (CurrState == STATE_READY & FlushCache) | - (CurrState == STATE_FLUSH) | - (CurrState == STATE_FLUSH_WRITEBACK); - assign FlushAdrCntEn = (CurrState == STATE_FLUSH_WRITEBACK & FlushWayFlag & CacheBusAck) | - (CurrState == STATE_FLUSH & FlushWayFlag & ~LineDirty); - assign FlushWayCntEn = (CurrState == STATE_FLUSH & ~LineDirty) | - (CurrState == STATE_FLUSH_WRITEBACK & CacheBusAck); - assign FlushCntRst = (CurrState == STATE_FLUSH & FlushFlag & ~LineDirty) | - (CurrState == STATE_FLUSH_WRITEBACK & FlushFlag & CacheBusAck); - // Bus interface controls - assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss & ~LineDirty) | - (CurrState == STATE_FETCH & ~CacheBusAck) | - (CurrState == STATE_WRITEBACK & CacheBusAck); - assign CacheBusRW[0] = (CurrState == STATE_READY & AnyMiss & LineDirty) | - (CurrState == STATE_WRITEBACK & ~CacheBusAck) | - (CurrState == STATE_FLUSH_WRITEBACK & ~CacheBusAck); - - assign SelAdr = (CurrState == STATE_READY & (StoreAMO | AnyMiss)) | // changes if store delay hazard removed - (CurrState == STATE_FETCH) | - (CurrState == STATE_WRITEBACK) | - (CurrState == STATE_WRITE_LINE) | - resetDelay; - assign SelFetchBuffer = CurrState == STATE_WRITE_LINE | CurrState == STATE_READ_HOLD; - assign CacheEn = (~Stall | FlushCache | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; endmodule // cachefsm From 0ed3e80ee0e9872bfa2cb6570d48cdab07109b7b Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 12 Apr 2023 00:53:22 -0700 Subject: [PATCH 06/10] only assign ClearDirtyWay for read-write caches --- src/cache/cacheway.sv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index 568e626e5..106905939 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -97,9 +97,9 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, ///////////////////////////////////////////////////////////////////////////////////////////// assign SetValidWay = SetValid & SelData; - assign ClearDirtyWay = ClearDirty & SelData; if (!READ_ONLY_CACHE) begin assign SetDirtyWay = SetDirty & SelData; + assign ClearDirtyWay = ClearDirty & SelData; assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage; assign SetValidEN = SetValidWay & ~FlushStage; end From cc3b2bf4351a62465f1ef9bd3dea8ead2cf8ca48 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 12 Apr 2023 13:32:36 -0700 Subject: [PATCH 07/10] Cachefsm gate LRUWriteEn with ~FlushStage --- src/cache/cache.sv | 2 +- src/cache/cachefsm.sv | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cache/cache.sv b/src/cache/cache.sv index 2d90e98e8..ea7504f23 100644 --- a/src/cache/cache.sv +++ b/src/cache/cache.sv @@ -129,7 +129,7 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE // Select victim way for associative caches if(NUMWAYS > 1) begin:vict cacheLRU #(NUMWAYS, SETLEN, OFFSETLEN, NUMLINES) cacheLRU( - .clk, .reset, .CacheEn, .FlushStage, .HitWay, .ValidWay, .VictimWay, .CacheSet, .LRUWriteEn, + .clk, .reset, .CacheEn, .HitWay, .ValidWay, .VictimWay, .CacheSet, .LRUWriteEn, .SetValid, .PAdr(PAdr[SETTOP-1:OFFSETLEN]), .InvalidateCache, .FlushCache); end else assign VictimWay = 1'b1; // one hot. diff --git a/src/cache/cachefsm.sv b/src/cache/cachefsm.sv index a42176325..c5e261f27 100644 --- a/src/cache/cachefsm.sv +++ b/src/cache/cachefsm.sv @@ -224,8 +224,9 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( // write enables internal to cache assign SetValid = CurrState == STATE_WRITE_LINE; + // coverage off -item e 1 -fecexprrow 8 assign LRUWriteEn = (CurrState == STATE_READY & AnyHit) | - (CurrState == STATE_WRITE_LINE); + (CurrState == STATE_WRITE_LINE) & ~FlushStage; assign SelFetchBuffer = CurrState == STATE_WRITE_LINE | CurrState == STATE_READ_HOLD; endmodule // cachefsm From 01f241752429312caa9ef34817a5a1702cf4a385 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 12 Apr 2023 15:57:45 -0700 Subject: [PATCH 08/10] cachefsm exclude icache logic without code reuse --- sim/coverage-exclusions-rv64gc.do | 34 ++++- src/cache/cachefsm.sv | 198 ++++++++++++------------------ 2 files changed, 105 insertions(+), 127 deletions(-) diff --git a/sim/coverage-exclusions-rv64gc.do b/sim/coverage-exclusions-rv64gc.do index 3e880b502..eb10505eb 100644 --- a/sim/coverage-exclusions-rv64gc.do +++ b/sim/coverage-exclusions-rv64gc.do @@ -27,17 +27,41 @@ # This file should be a last resort. It's preferable to put # // coverage off # statements inline with the code whenever possible. +# a hack to describe coverage exclusions without hardcoding linenumbers: +do GetLineNum.do # LZA (i<64) statement confuses coverage tool # This is ugly to exlcude the whole file - is there a better option? // coverage off isn't working coverage exclude -srcfile lzc.sv -# Exclude D$ states from coverage in the I$ instance of cachefsm. -# This is cleaner than trying to set an I$-specific pragma in cachefsm.sv -# Also exclude the write line to ready transition for the I$ since we can't get a flush -# during this operation. -coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -fstate CurrState STATE_FLUSH STATE_FLUSH_WRITEBACK STATE_FLUSH_WRITEBACK +### Exclude D$ states and logic for the I$ instance +# This is cleaner than trying to set an I$-specific pragma in cachefsm.sv (which would exclude it for the D$ instance too) +# Also exclude the write line to ready transition for the I$ since we can't get a flush during this operation. +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -fstate CurrState STATE_FLUSH STATE_FLUSH_WRITEBACK STATE_FLUSH_WRITEBACK STATE_WRITEBACK coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -ftrans CurrState STATE_WRITE_LINE->STATE_READY +# exclude unused transitions from case statement. Unfortunately the whole branch needs to be excluded I think. Expression coverage should still work. +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache state-case"] -item b 1 +# exclude branch/condition coverage: LineDirty if statement +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache FETCHStatement"] -item bc 1 +# exclude the unreachable logic +set start [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-start: icache case"] +set end [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-end: icache case"] +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange $start-$end +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache WRITEBACKStatement"] +# exclude Atomic Operation logic +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache storeAMO"] -item e 1 -fecexprrow 6 +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache storeAMO1"] -item e 1 -fecexprrow 2-4 +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache AnyUpdateHit"] -item e 1 -fecexprrow 2 +# cache write logic +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheW"] -item e 1 -fecexprrow 4 +# output signal logic +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache StallStates"] -item e 1 -fecexprrow 8 12 14 +set start [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-start: icache flushdirtycontrols"] +set end [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-end: icache flushdirtycontrols"] +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange $start-$end +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheBusW"] +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache SelAdrCauses"] -item e 1 -fecexprrow 4 10 +coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheBusRCauses"] -item e 1 -fecexprrow 1-2 12 ###################### # Toggle exclusions diff --git a/src/cache/cachefsm.sv b/src/cache/cachefsm.sv index c5e261f27..fc31aec4b 100644 --- a/src/cache/cachefsm.sv +++ b/src/cache/cachefsm.sv @@ -86,25 +86,18 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( statetype CurrState, NextState; - // no atomic operations on i$ - if (!READ_ONLY_CACHE) begin - assign AMO = CacheAtomic[1] & (&CacheRW); - assign StoreAMO = AMO | CacheRW[0]; - assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; - assign AnyUpdateHit = StoreAMO & CacheHit; - assign AnyHit = AnyUpdateHit | (CacheRW[1] & CacheHit); - assign CacheAccess = (AMO | CacheRW[1] | CacheRW[0]) & CurrState == STATE_READY; // for performance counter - end - else begin - assign AnyMiss = CacheRW[1] & ~CacheHit & ~InvalidateCache; - assign AnyUpdateHit = 0; // todo clear all RO cache of usage of this logic - assign AnyHit = CacheRW[1] & CacheHit; - assign CacheAccess = CacheRW[1] & CurrState == STATE_READY; // for performance counter - end + assign AMO = CacheAtomic[1] & (&CacheRW); + assign StoreAMO = AMO | CacheRW[0]; - assign CacheMiss = CacheAccess & ~CacheHit; // for performance counter + assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; // exclusion-tag: icache storeAMO + assign AnyUpdateHit = (StoreAMO) & CacheHit; // exclusion-tag: icache storeAMO1 + assign AnyHit = AnyUpdateHit | (CacheRW[1] & CacheHit); // exclusion-tag: icache AnyUpdateHit assign FlushFlag = FlushAdrFlag & FlushWayFlag; + // outputs for the performance counters. + assign CacheAccess = (AMO | CacheRW[1] | CacheRW[0]) & CurrState == STATE_READY; // exclusion-tag: icache CacheW + assign CacheMiss = CacheAccess & ~CacheHit; + // special case on reset. When the fsm first exists reset the // PCNextF will no longer be pointing to the correct address. // But PCF will be the reset vector. @@ -113,120 +106,81 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( always_ff @(posedge clk) if (reset | FlushStage) CurrState <= #1 STATE_READY; else CurrState <= #1 NextState; - - // seperating NextState logic by ro vs rw cache results in code duplication but this is needed to hit coverage. - if (!READ_ONLY_CACHE) - always_comb begin - NextState = STATE_READY; - case (CurrState) - STATE_READY: if(InvalidateCache) NextState = STATE_READY; - else if(FlushCache) NextState = STATE_FLUSH; - else if(AnyMiss & ~LineDirty) NextState = STATE_FETCH; - else if(AnyMiss & LineDirty) NextState = STATE_WRITEBACK; - else NextState = STATE_READY; - STATE_FETCH: if(CacheBusAck) NextState = STATE_WRITE_LINE; - else NextState = STATE_FETCH; - STATE_WRITE_LINE: NextState = STATE_READ_HOLD; - STATE_READ_HOLD: if(Stall) NextState = STATE_READ_HOLD; - else NextState = STATE_READY; - STATE_WRITEBACK: if(CacheBusAck) NextState = STATE_FETCH; - else NextState = STATE_WRITEBACK; - // eviction needs a delay as the bus fsm does not correctly handle sending the write command at the same - // time as getting back the bus ack. - STATE_FLUSH: if(LineDirty) NextState = STATE_FLUSH_WRITEBACK; - else if (FlushFlag) NextState = STATE_READ_HOLD; - else NextState = STATE_FLUSH; - STATE_FLUSH_WRITEBACK: if(CacheBusAck & ~FlushFlag) NextState = STATE_FLUSH; - else if(CacheBusAck) NextState = STATE_READ_HOLD; - else NextState = STATE_FLUSH_WRITEBACK; - default: NextState = STATE_READY; - endcase - end // always_comb - else // READ_ONLY_CACHE - always_comb begin - NextState = STATE_READY; - case (CurrState) - STATE_READY: if(InvalidateCache) NextState = STATE_READY; - else if(AnyMiss) NextState = STATE_FETCH; - else NextState = STATE_READY; - STATE_FETCH: if(CacheBusAck) NextState = STATE_WRITE_LINE; - else NextState = STATE_FETCH; - STATE_WRITE_LINE: NextState = STATE_READ_HOLD; - STATE_READ_HOLD: if(Stall) NextState = STATE_READ_HOLD; - else NextState = STATE_READY; - default: NextState = STATE_READY; - endcase // case (CurrState) - end // always_comb + + always_comb begin + NextState = STATE_READY; + case (CurrState) // exclusion-tag: icache state-case + STATE_READY: if(InvalidateCache) NextState = STATE_READY; + else if(FlushCache & ~READ_ONLY_CACHE) NextState = STATE_FLUSH; + else if(AnyMiss & (READ_ONLY_CACHE | ~LineDirty)) NextState = STATE_FETCH; // exclusion-tag: icache FETCHStatement + else if(AnyMiss & LineDirty) NextState = STATE_WRITEBACK; // exclusion-tag: icache WRITEBACKStatement + else NextState = STATE_READY; + STATE_FETCH: if(CacheBusAck) NextState = STATE_WRITE_LINE; + else NextState = STATE_FETCH; + STATE_WRITE_LINE: NextState = STATE_READ_HOLD; + STATE_READ_HOLD: if(Stall) NextState = STATE_READ_HOLD; + else NextState = STATE_READY; + // exclusion-tag-start: icache case + STATE_WRITEBACK: if(CacheBusAck) NextState = STATE_FETCH; + else NextState = STATE_WRITEBACK; + // eviction needs a delay as the bus fsm does not correctly handle sending the write command at the same time as getting back the bus ack. + STATE_FLUSH: if(LineDirty) NextState = STATE_FLUSH_WRITEBACK; + else if (FlushFlag) NextState = STATE_READ_HOLD; + else NextState = STATE_FLUSH; + STATE_FLUSH_WRITEBACK: if(CacheBusAck & ~FlushFlag) NextState = STATE_FLUSH; + else if(CacheBusAck) NextState = STATE_READ_HOLD; + else NextState = STATE_FLUSH_WRITEBACK; + // exclusion-tag-end: icache case + default: NextState = STATE_READY; + endcase + end // com back to CPU - if (!READ_ONLY_CACHE) begin - assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | - (CurrState == STATE_FETCH) | - (CurrState == STATE_WRITEBACK) | - (CurrState == STATE_WRITE_LINE) | // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write. - (CurrState == STATE_FLUSH) | - (CurrState == STATE_FLUSH_WRITEBACK); - assign CacheCommitted = CurrState != STATE_READY; - assign SelAdr = (CurrState == STATE_READY & (StoreAMO | AnyMiss)) | // changes if store delay hazard removed - (CurrState == STATE_FETCH) | - (CurrState == STATE_WRITEBACK) | - (CurrState == STATE_WRITE_LINE) | - resetDelay; - assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) | - (CurrState == STATE_WRITE_LINE & (StoreAMO)); - assign ClearDirty = (CurrState == STATE_WRITE_LINE & ~(StoreAMO)) | - (CurrState == STATE_FLUSH & LineDirty); // This is wrong in a multicore snoop cache protocal. Dirty must be cleared concurrently and atomically with writeback. For single core cannot clear after writeback on bus ack and change flushadr. Clears the wrong set. - - // Flush and eviction controls - assign SelWriteback = (CurrState == STATE_WRITEBACK & ~CacheBusAck) | - (CurrState == STATE_READY & AnyMiss & LineDirty); - - assign SelFlush = (CurrState == STATE_READY & FlushCache) | - (CurrState == STATE_FLUSH) | + assign CacheCommitted = (CurrState != STATE_READY) & ~(READ_ONLY_CACHE & CurrState == STATE_READ_HOLD); + assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | // exclusion-tag: icache StallStates + (CurrState == STATE_FETCH) | + (CurrState == STATE_WRITEBACK) | + (CurrState == STATE_WRITE_LINE) | // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write. + (CurrState == STATE_FLUSH) | (CurrState == STATE_FLUSH_WRITEBACK); - assign FlushAdrCntEn = (CurrState == STATE_FLUSH_WRITEBACK & FlushWayFlag & CacheBusAck) | - (CurrState == STATE_FLUSH & FlushWayFlag & ~LineDirty); - assign FlushWayCntEn = (CurrState == STATE_FLUSH & ~LineDirty) | - (CurrState == STATE_FLUSH_WRITEBACK & CacheBusAck); - assign FlushCntRst = (CurrState == STATE_FLUSH & FlushFlag & ~LineDirty) | - (CurrState == STATE_FLUSH_WRITEBACK & FlushFlag & CacheBusAck); - // Bus interface controls - assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss & ~LineDirty) | - (CurrState == STATE_FETCH & ~CacheBusAck) | - (CurrState == STATE_WRITEBACK & CacheBusAck); - assign CacheBusRW[0] = (CurrState == STATE_READY & AnyMiss & LineDirty) | - (CurrState == STATE_WRITEBACK & ~CacheBusAck) | - (CurrState == STATE_FLUSH_WRITEBACK & ~CacheBusAck); - // write enable internal to cache - assign CacheEn = (~Stall | FlushCache | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; - end - else begin - assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | - (CurrState == STATE_FETCH) | - (CurrState == STATE_WRITE_LINE); // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write. - assign CacheCommitted = (CurrState != STATE_READY) & ~(CurrState == STATE_READ_HOLD); - assign SelAdr = (CurrState == STATE_READY & AnyMiss) | // changes if store delay hazard removed - (CurrState == STATE_FETCH) | - (CurrState == STATE_WRITE_LINE) | - resetDelay; - assign SetDirty = 0; - assign ClearDirty = 0; - assign SelWriteback = 0; - assign SelFlush = 0; - assign FlushAdrCntEn = 0; - assign FlushWayCntEn = 0; - assign FlushCntRst = 0; - assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss) | - (CurrState == STATE_FETCH & ~CacheBusAck); - assign CacheBusRW[0] = 0; - assign CacheEn = (~Stall | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; - end // else: (READ_ONLY_CACHE) - // write enables internal to cache assign SetValid = CurrState == STATE_WRITE_LINE; // coverage off -item e 1 -fecexprrow 8 assign LRUWriteEn = (CurrState == STATE_READY & AnyHit) | (CurrState == STATE_WRITE_LINE) & ~FlushStage; + // exclusion-tag-start: icache flushdirtycontrols + assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) | // exclusion-tag: icache SetDirty + (CurrState == STATE_WRITE_LINE & (StoreAMO)); + assign ClearDirty = (CurrState == STATE_WRITE_LINE & ~(StoreAMO)) | // exclusion-tag: icache ClearDirty + (CurrState == STATE_FLUSH & LineDirty); // This is wrong in a multicore snoop cache protocal. Dirty must be cleared concurrently and atomically with writeback. For single core cannot clear after writeback on bus ack and change flushadr. Clears the wrong set. + // Flush and eviction controls + assign SelWriteback = (CurrState == STATE_WRITEBACK & ~CacheBusAck) | + (CurrState == STATE_READY & AnyMiss & LineDirty); + + assign SelFlush = (CurrState == STATE_READY & FlushCache) | + (CurrState == STATE_FLUSH) | + (CurrState == STATE_FLUSH_WRITEBACK); + assign FlushAdrCntEn = (CurrState == STATE_FLUSH_WRITEBACK & FlushWayFlag & CacheBusAck) | + (CurrState == STATE_FLUSH & FlushWayFlag & ~LineDirty); + assign FlushWayCntEn = (CurrState == STATE_FLUSH & ~LineDirty) | + (CurrState == STATE_FLUSH_WRITEBACK & CacheBusAck); + assign FlushCntRst = (CurrState == STATE_FLUSH & FlushFlag & ~LineDirty) | + (CurrState == STATE_FLUSH_WRITEBACK & FlushFlag & CacheBusAck); + // exclusion-tag-end: icache flushdirtycontrols + // Bus interface controls + assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss & ~LineDirty) | // exclusion-tag: icache CacheBusRCauses + (CurrState == STATE_FETCH & ~CacheBusAck) | + (CurrState == STATE_WRITEBACK & CacheBusAck); + assign CacheBusRW[0] = (CurrState == STATE_READY & AnyMiss & LineDirty) | // exclusion-tag: icache CacheBusW + (CurrState == STATE_WRITEBACK & ~CacheBusAck) | + (CurrState == STATE_FLUSH_WRITEBACK & ~CacheBusAck); + + assign SelAdr = (CurrState == STATE_READY & (StoreAMO | AnyMiss)) | // exclusion-tag: icache SelAdrCauses // changes if store delay hazard removed + (CurrState == STATE_FETCH) | + (CurrState == STATE_WRITEBACK) | + (CurrState == STATE_WRITE_LINE) | + resetDelay; assign SelFetchBuffer = CurrState == STATE_WRITE_LINE | CurrState == STATE_READ_HOLD; + assign CacheEn = (~Stall | FlushCache | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; endmodule // cachefsm From ad0e366766bf96939bde271fae29b7a2b1505ccb Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 12 Apr 2023 15:58:38 -0700 Subject: [PATCH 09/10] track GetLinenum.do (tcl procedure to find line numbers to exclude) --- sim/GetLineNum.do | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 sim/GetLineNum.do diff --git a/sim/GetLineNum.do b/sim/GetLineNum.do new file mode 100644 index 000000000..4d38d9121 --- /dev/null +++ b/sim/GetLineNum.do @@ -0,0 +1,18 @@ +# Alec Vercruysse +# 2023-04-12 +# Note that the target string is regex, and needs to be double-escaped. +# e.g. to match a (, you need \\(. +proc GetLineNum {fname target} { + set f [open $fname] + set linectr 1 + while {[gets $f line] != -1} { + if {[regexp $target $line]} { + close $f + return $linectr + } + incr linectr + } + close $f + return -code error \ + "target string not found" +} From 4d9aa728779ceb2f57bce92bdf70de2ab5a60d24 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Fri, 14 Apr 2023 16:54:55 -0700 Subject: [PATCH 10/10] replace instances of code duplication for i$ exclusions w/commands --- sim/coverage-exclusions-rv64gc.do | 11 +++++++++++ src/cache/cache.sv | 28 +++++++++------------------- src/cache/cacheway.sv | 20 ++++---------------- src/generic/mux.sv | 2 +- 4 files changed, 25 insertions(+), 36 deletions(-) diff --git a/sim/coverage-exclusions-rv64gc.do b/sim/coverage-exclusions-rv64gc.do index eb10505eb..374c4b917 100644 --- a/sim/coverage-exclusions-rv64gc.do +++ b/sim/coverage-exclusions-rv64gc.do @@ -62,6 +62,17 @@ coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange $sta coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheBusW"] coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache SelAdrCauses"] -item e 1 -fecexprrow 4 10 coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheBusRCauses"] -item e 1 -fecexprrow 1-2 12 +# cache.sv AdrSelMux and CacheBusAdrMux, excluding unhit Flush branch +coverage exclude -scope /dut/core/ifu/bus/icache/icache/AdrSelMux -linerange [GetLineNum ../src/generic/mux.sv "exclusion-tag: mux3"] -item b 1 +coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheBusAdrMux -linerange [GetLineNum ../src/generic/mux.sv "exclusion-tag: mux3"] -item b 1 3 +# CacheWay Dirty logic. -scope does not accept wildcards. +set numcacheways 4 +for {set i 0} {$i < $numcacheways} {incr i} { + coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: icache SetDirtyWay"] -item e 1 + coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: icache SelectedWiteWordEn"] -item e 1 -fecexprrow 4 6 + # below: flushD can't go high during an icache write b/c of pipeline stall + coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: icache SetValidEN"] -item e 1 -fecexprrow 4 +} ###################### # Toggle exclusions diff --git a/src/cache/cache.sv b/src/cache/cache.sv index ea7504f23..57ff20ab3 100644 --- a/src/cache/cache.sv +++ b/src/cache/cache.sv @@ -73,6 +73,7 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE logic SelAdr; + logic [1:0] AdrSelMuxSel; logic [SETLEN-1:0] CacheSet; logic [LINELEN-1:0] LineWriteData; logic ClearDirty, SetDirty, SetValid; @@ -108,18 +109,10 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE // and FlushAdr when handling D$ flushes // The icache must update to the newest PCNextF on flush as it is probably a trap. Trap // sets PCNextF to XTVEC and the icache must start reading the instruction. - if (!READ_ONLY_CACHE) begin - logic [1:0] AdrSelMuxSel; - assign AdrSelMuxSel = {SelFlush, SelAdr | SelHPTW}; - mux3 #(SETLEN) AdrSelMux(NextSet[SETTOP-1:OFFSETLEN], PAdr[SETTOP-1:OFFSETLEN], FlushAdr, + assign AdrSelMuxSel = {SelFlush, ((SelAdr | SelHPTW) & ~((READ_ONLY_CACHE == 1) & FlushStage))}; + mux3 #(SETLEN) AdrSelMux(NextSet[SETTOP-1:OFFSETLEN], PAdr[SETTOP-1:OFFSETLEN], FlushAdr, AdrSelMuxSel, CacheSet); - end - else begin - logic AdrSelMuxSel; - assign AdrSelMuxSel = ((SelAdr | SelHPTW) & ~FlushStage); - mux2 #(SETLEN) AdrSelMux(NextSet[SETTOP-1:OFFSETLEN], PAdr[SETTOP-1:OFFSETLEN], - AdrSelMuxSel, CacheSet); - end + // Array of cache ways, along with victim, hit, dirty, and read merging logic cacheway #(NUMLINES, LINELEN, TAGLEN, OFFSETLEN, SETLEN, READ_ONLY_CACHE) CacheWays[NUMWAYS-1:0]( .clk, .reset, .CacheEn, .CacheSet, .PAdr, .LineWriteData, .LineByteMask, @@ -159,14 +152,11 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE .PAdr(WordOffsetAddr), .ReadDataLine, .ReadDataWord); // Bus address for fetch, writeback, or flush writeback - if (!READ_ONLY_CACHE) - mux3 #(`PA_BITS) CacheBusAdrMux(.d0({PAdr[`PA_BITS-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), - .d1({Tag, PAdr[SETTOP-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), - .d2({Tag, FlushAdr, {OFFSETLEN{1'b0}}}), - .s({SelFlush, SelWriteback}), .y(CacheBusAdr)); - else - assign CacheBusAdr = {PAdr[`PA_BITS-1:OFFSETLEN], {OFFSETLEN{1'b0}}}; - + mux3 #(`PA_BITS) CacheBusAdrMux(.d0({PAdr[`PA_BITS-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), + .d1({Tag, PAdr[SETTOP-1:OFFSETLEN], {OFFSETLEN{1'b0}}}), + .d2({Tag, FlushAdr, {OFFSETLEN{1'b0}}}), + .s({SelFlush, SelWriteback}), .y(CacheBusAdr)); + ///////////////////////////////////////////////////////////////////////////////////////////// // Write Path ///////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index 106905939..77e844b20 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -97,22 +97,10 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, ///////////////////////////////////////////////////////////////////////////////////////////// assign SetValidWay = SetValid & SelData; - if (!READ_ONLY_CACHE) begin - assign SetDirtyWay = SetDirty & SelData; - assign ClearDirtyWay = ClearDirty & SelData; - assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage; - assign SetValidEN = SetValidWay & ~FlushStage; - end - else begin - // Don't cover FlushStage assertion during SetValidWay. - // it's not explicitely gated anywhere, but for read-only caches, - // there's no way that a FlushD can happen during the write stage - // of a fetch. - // coverage off -item e 1 -fecexprrow 4 - assign SelectedWriteWordEn = SetValidWay & ~FlushStage; - // coverage off -item e 1 -fecexprrow 4 - assign SetValidEN = SetValidWay & ~FlushStage; - end + assign SetDirtyWay = SetDirty & SelData; // exclusion-tag: icache SetDirtyWay + assign ClearDirtyWay = ClearDirty & SelData; + assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage; // exclusion-tag: icache SelectedWiteWordEn + assign SetValidEN = SetValidWay & ~FlushStage; // exclusion-tag: icache SetValidEN // If writing the whole line set all write enables to 1, else only set the correct word. assign FinalByteMask = SetValidWay ? '1 : LineByteMask; // OR diff --git a/src/generic/mux.sv b/src/generic/mux.sv index 636c19c9f..223d41afc 100644 --- a/src/generic/mux.sv +++ b/src/generic/mux.sv @@ -40,7 +40,7 @@ module mux3 #(parameter WIDTH = 8) ( input logic [1:0] s, output logic [WIDTH-1:0] y); - assign y = s[1] ? d2 : (s[0] ? d1 : d0); + assign y = s[1] ? d2 : (s[0] ? d1 : d0); // exclusion-tag: mux3 endmodule module mux4 #(parameter WIDTH = 8) (