From af113c7268bee7030483ac3e18fc460af02d994d Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Mon, 3 Apr 2023 13:44:07 -0700 Subject: [PATCH 01/11] Exclude CacheLRU log2 function from coverage --- src/cache/cacheLRU.sv | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cache/cacheLRU.sv b/src/cache/cacheLRU.sv index 780807943..125a4ae10 100644 --- a/src/cache/cacheLRU.sv +++ b/src/cache/cacheLRU.sv @@ -67,11 +67,14 @@ module cacheLRU assign AllValid = &ValidWay; ///// Update replacement bits. + + // coverage off: Untestable without varying NUMWAYS. function integer log2 (integer value); for (log2=0; value>0; log2=log2+1) value = value>>1; return log2; endfunction // log2 + // coverage on // On a miss we need to ignore HitWay and derive the new replacement bits with the VictimWay. mux2 #(NUMWAYS) WayMux(HitWay, VictimWay, SetValid, Way); From 9df246e5de8ef505925e0d64c40d0d4b16c2379d Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 4 Apr 2023 22:19:21 -0700 Subject: [PATCH 02/11] put cacheLRU coverage explanation on another line the `: explanation` syntax was not working --- src/cache/cacheLRU.sv | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cache/cacheLRU.sv b/src/cache/cacheLRU.sv index 125a4ae10..1e7101365 100644 --- a/src/cache/cacheLRU.sv +++ b/src/cache/cacheLRU.sv @@ -68,7 +68,8 @@ module cacheLRU ///// Update replacement bits. - // coverage off: Untestable without varying NUMWAYS. + // coverage off + // Excluded from coverage b/c it is untestable without varying NUMWAYS. function integer log2 (integer value); for (log2=0; value>0; log2=log2+1) value = value>>1; From 255332115834fe24b68e9ece68270027ef40061e Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 4 Apr 2023 22:20:31 -0700 Subject: [PATCH 03/11] fix typo in cachway setValid input comment --- 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 d7cc0792d..c5660bf49 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -38,7 +38,7 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, input logic [$clog2(NUMLINES)-1:0] CacheSet, // Cache address, the output of the address select mux, NextAdr, PAdr, or FlushAdr input logic [`PA_BITS-1:0] PAdr, // Physical address input logic [LINELEN-1:0] LineWriteData, // Final data written to cache (D$ only) - input logic SetValid, // Set the dirty bit in the selected way and set + input logic SetValid, // Set the valid bit in the selected way and set input logic ClearValid, // Clear the valid bit in the selected way and set input logic SetDirty, // Set the dirty bit in the selected way and set input logic ClearDirty, // Clear the dirty bit in the selected way and set From 8b6b96012d5ba4f67c5985ee3ab56bcaca1d19dc Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Tue, 4 Apr 2023 22:21:06 -0700 Subject: [PATCH 04/11] add ram1p1rwe for read-only cache ways (remove byte-enable) - increases coverage --- src/cache/cacheway.sv | 10 +++- src/generic/mem/ram1p1rwe.sv | 95 ++++++++++++++++++++++++++++++++++++ testbench/testbench.sv | 2 +- 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 src/generic/mem/ram1p1rwe.sv diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index c5660bf49..8f5fe79a5 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -128,10 +128,18 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, localparam LOGNUMSRAM = $clog2(NUMSRAM); for(words = 0; words < NUMSRAM; words++) begin: word - ram1p1rwbe #(.DEPTH(NUMLINES), .WIDTH(SRAMLEN)) CacheDataMem(.clk, .ce(CacheEn), .addr(CacheSet), + if (!READ_ONLY_CACHE) begin:wordram + ram1p1rwbe #(.DEPTH(NUMLINES), .WIDTH(SRAMLEN)) CacheDataMem(.clk, .ce(CacheEn), .addr(CacheSet), .dout(ReadDataLine[SRAMLEN*(words+1)-1:SRAMLEN*words]), .din(LineWriteData[SRAMLEN*(words+1)-1:SRAMLEN*words]), .we(SelectedWriteWordEn), .bwe(FinalByteMask[SRAMLENINBYTES*(words+1)-1:SRAMLENINBYTES*words])); + end + else begin:wordram // no byte-enable needed for i$. + ram1p1rwe #(.DEPTH(NUMLINES), .WIDTH(SRAMLEN)) CacheDataMem(.clk, .ce(CacheEn), .addr(CacheSet), + .dout(ReadDataLine[SRAMLEN*(words+1)-1:SRAMLEN*words]), + .din(LineWriteData[SRAMLEN*(words+1)-1:SRAMLEN*words]), + .we(SelectedWriteWordEn)); + end end // AND portion of distributed read multiplexers diff --git a/src/generic/mem/ram1p1rwe.sv b/src/generic/mem/ram1p1rwe.sv new file mode 100644 index 000000000..587710f40 --- /dev/null +++ b/src/generic/mem/ram1p1rwe.sv @@ -0,0 +1,95 @@ +/////////////////////////////////////////// +// 1 port sram. +// +// Written: avercruysse@hmc.edu (Modified from ram1p1rwbe, by ross1728@gmail.com) +// Created: 04 April 2023 +// +// Purpose: ram1p1wre, but without byte-enable. Used for icache data. +// +// Documentation: +// +// A component of the CORE-V-WALLY configurable RISC-V project. +// +// Copyright (C) 2021-23 Harvey Mudd College & Oklahoma State University +// +// SPDX-License-Identifier: Apache-2.0 WITH SHL-2.1 +// +// Licensed under the Solderpad Hardware License v 2.1 (the “License”); you may not use this file +// except in compliance with the License, or, at your option, the Apache License version 2.0. You +// may obtain a copy of the License at +// +// https://solderpad.org/licenses/SHL-2.1/ +// +// Unless required by applicable law or agreed to in writing, any work distributed under the +// License is distributed on an “AS IS” BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +// either express or implied. See the License for the specific language governing permissions +// and limitations under the License. +//////////////////////////////////////////////////////////////////////////////////////////////// + +// WIDTH is number of bits in one "word" of the memory, DEPTH is number of such words + +`include "wally-config.vh" + +module ram1p1rwe #(parameter DEPTH=64, WIDTH=44) ( + input logic clk, + input logic ce, + input logic [$clog2(DEPTH)-1:0] addr, + input logic [WIDTH-1:0] din, + input logic we, + output logic [WIDTH-1:0] dout +); + + logic [WIDTH-1:0] RAM[DEPTH-1:0]; + + // *************************************************************************** + // TRUE SRAM macro + // *************************************************************************** + if ((`USE_SRAM == 1) & (WIDTH == 128) & (DEPTH == 64)) begin // Cache data subarray + // 64 x 128-bit SRAM + ram1p1rwbe_64x128 sram1A (.CLK(clk), .CEB(~ce), .WEB(~we), + .A(addr), .D(din), + .BWEB('0), .Q(dout)); + + end else if ((`USE_SRAM == 1) & (WIDTH == 44) & (DEPTH == 64)) begin // RV64 cache tag + // 64 x 44-bit SRAM + ram1p1rwbe_64x44 sram1B (.CLK(clk), .CEB(~ce), .WEB(~we), + .A(addr), .D(din), + .BWEB('0), .Q(dout)); + + end else if ((`USE_SRAM == 1) & (WIDTH == 22) & (DEPTH == 64)) begin // RV32 cache tag + // 64 x 22-bit SRAM + ram1p1rwbe_64x22 sram1B (.CLK(clk), .CEB(~ce), .WEB(~we), + .A(addr), .D(din), + .BWEB('0), .Q(dout)); + + // *************************************************************************** + // READ first SRAM model + // *************************************************************************** + end else begin: ram + integer i; + + // Read + logic [$clog2(DEPTH)-1:0] addrd; + flopen #($clog2(DEPTH)) adrreg(clk, ce, addr, addrd); + assign dout = RAM[addrd]; + + /* // Read + always_ff @(posedge clk) + if(ce) dout <= #1 mem[addr]; */ + + // Write divided into part for bytes and part for extra msbs + // Questa sim version 2022.3_2 does not allow multiple drivers for RAM when using always_ff. + // Therefore these always blocks use the older always @(posedge clk) + if(WIDTH >= 8) + always @(posedge clk) + if (ce & we) + for(i = 0; i < WIDTH/8; i++) + RAM[addr][i*8 +: 8] <= #1 din[i*8 +: 8]; + + if (WIDTH%8 != 0) // handle msbs if width not a multiple of 8 + always @(posedge clk) + if (ce & we) + RAM[addr][WIDTH-1:WIDTH-WIDTH%8] <= #1 din[WIDTH-1:WIDTH-WIDTH%8]; + end + +endmodule diff --git a/testbench/testbench.sv b/testbench/testbench.sv index 87718b48b..ac544c50b 100644 --- a/testbench/testbench.sv +++ b/testbench/testbench.sv @@ -713,7 +713,7 @@ module DCacheFlushFSM // these dirty bit selections would be needed if dirty is moved inside the tag array. //.dirty(testbench.dut.core.lsu.bus.dcache.dcache.CacheWays[way].dirty.DirtyMem.RAM[index]), //.dirty(testbench.dut.core.lsu.bus.dcache.dcache.CacheWays[way].CacheTagMem.RAM[index][`PA_BITS+tagstart]), - .data(testbench.dut.core.lsu.bus.dcache.dcache.CacheWays[way].word[cacheWord].CacheDataMem.RAM[index]), + .data(testbench.dut.core.lsu.bus.dcache.dcache.CacheWays[way].word[cacheWord].wordram.CacheDataMem.RAM[index]), .index(index), .cacheWord(cacheWord), .CacheData(CacheData[way][index][cacheWord]), From 782feb6161ee0f6530c579728b5f99327f2405c4 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 5 Apr 2023 11:30:39 -0700 Subject: [PATCH 05/11] turn off ce coverage for ram1p1rwe According to the textbook, the cache memory chip enable, `CacheEn`, is only lowered by the cachefsm with it is in the ready state and a pipeline stall is asserted. For read only caches, cache writes only occur in the state_write_line state. So there is no way that a write would happen while the chip enable is low. Removing the chip-enable check from this memory to increase coverage would be a bad idea since if anyone else uses this ram, the behaviour would be differently than expected. Instead, I opted to turn off coverage for this statement. Since this ram, which does not have a byte enable, is used exclusively by read-only caches right now, this should not mistakenly exclude coverage for other cases, such as D$. --- src/generic/mem/ram1p1rwe.sv | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/generic/mem/ram1p1rwe.sv b/src/generic/mem/ram1p1rwe.sv index 587710f40..8a2f971e9 100644 --- a/src/generic/mem/ram1p1rwe.sv +++ b/src/generic/mem/ram1p1rwe.sv @@ -81,14 +81,23 @@ module ram1p1rwe #(parameter DEPTH=64, WIDTH=44) ( // Questa sim version 2022.3_2 does not allow multiple drivers for RAM when using always_ff. // Therefore these always blocks use the older always @(posedge clk) if(WIDTH >= 8) - always @(posedge clk) - if (ce & we) + always @(posedge clk) + // coverage off + // ce only goes low when cachefsm is in READY state and Flush is asserted. + // for read-only caches, we only goes high in the STATE_WRITE_LINE cachefsm state. + // so we can never get we=1, ce=0 for I$. Note that turning off coverage here + // might miss some cases for D$, however, when we might go high due to a store. + if (ce & we) + // coverage on for(i = 0; i < WIDTH/8; i++) RAM[addr][i*8 +: 8] <= #1 din[i*8 +: 8]; if (WIDTH%8 != 0) // handle msbs if width not a multiple of 8 - always @(posedge clk) + always @(posedge clk) + // coverage off + // (see the above explanation) if (ce & we) + // coverage on RAM[addr][WIDTH-1:WIDTH-WIDTH%8] <= #1 din[WIDTH-1:WIDTH-WIDTH%8]; end From 81125d31808299b5eaaff9edc5094d1845f92a41 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 5 Apr 2023 11:36:02 -0700 Subject: [PATCH 06/11] change i$ cachetagmem from ram1p1rwbe -> ram1p1rwe the byte write-enables were always tied high, so we can use RAM without byte-enable to increase coverage. --- src/cache/cacheway.sv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index 8f5fe79a5..e5df9456a 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -107,8 +107,8 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, // Tag Array ///////////////////////////////////////////////////////////////////////////////////////////// - ram1p1rwbe #(.DEPTH(NUMLINES), .WIDTH(TAGLEN)) CacheTagMem(.clk, .ce(CacheEn), - .addr(CacheSet), .dout(ReadTag), .bwe('1), + ram1p1rwe #(.DEPTH(NUMLINES), .WIDTH(TAGLEN)) CacheTagMem(.clk, .ce(CacheEn), + .addr(CacheSet), .dout(ReadTag), .din(PAdr[`PA_BITS-1:OFFSETLEN+INDEXLEN]), .we(SetValidEN)); // AND portion of distributed tag multiplexer From 3419ef3651b6a7be181b3e0c4decc6002ab02161 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 5 Apr 2023 11:42:57 -0700 Subject: [PATCH 07/11] remove ClearValid from cache The cachefsm hardwired ClearValid logic to zero. This signal might've been added to potentially add extra functionality later. Unless that functionality is added, however, it negatively impacts coverage. If the goal is to maximize coverage, this signal should be removed and only added when it becomes necessary. --- src/cache/cache.sv | 6 +++--- src/cache/cachefsm.sv | 2 -- src/cache/cacheway.sv | 5 +---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/cache/cache.sv b/src/cache/cache.sv index 56044384b..63d882223 100644 --- a/src/cache/cache.sv +++ b/src/cache/cache.sv @@ -76,7 +76,7 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE logic [1:0] AdrSelMuxSel; logic [SETLEN-1:0] CacheSet; logic [LINELEN-1:0] LineWriteData; - logic ClearValid, ClearDirty, SetDirty, SetValid; + logic ClearDirty, SetDirty, SetValid; logic [LINELEN-1:0] ReadDataLineWay [NUMWAYS-1:0]; logic [NUMWAYS-1:0] HitWay, ValidWay; logic CacheHit; @@ -116,7 +116,7 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE // 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, - .SetValid, .ClearValid, .SetDirty, .ClearDirty, .SelWriteback, .VictimWay, + .SetValid, .SetDirty, .ClearDirty, .SelWriteback, .VictimWay, .FlushWay, .SelFlush, .ReadDataLineWay, .HitWay, .ValidWay, .DirtyWay, .TagWay, .FlushStage, .InvalidateCache); // Select victim way for associative caches @@ -209,7 +209,7 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE .FlushStage, .CacheRW, .CacheAtomic, .Stall, .CacheHit, .LineDirty, .CacheStall, .CacheCommitted, .CacheMiss, .CacheAccess, .SelAdr, - .ClearValid, .ClearDirty, .SetDirty, .SetValid, .SelWriteback, .SelFlush, + .ClearDirty, .SetDirty, .SetValid, .SelWriteback, .SelFlush, .FlushAdrCntEn, .FlushWayCntEn, .FlushCntRst, .FlushAdrFlag, .FlushWayFlag, .FlushCache, .SelFetchBuffer, .InvalidateCache, .CacheEn, .LRUWriteEn); diff --git a/src/cache/cachefsm.sv b/src/cache/cachefsm.sv index cd1d43c55..d1d54097e 100644 --- a/src/cache/cachefsm.sv +++ b/src/cache/cachefsm.sv @@ -55,7 +55,6 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( input logic FlushAdrFlag, // On last set of a cache flush input logic FlushWayFlag, // On the last way for any set of a cache flush output logic SelAdr, // [0] SRAM reads from NextAdr, [1] SRAM reads from PAdr - output logic ClearValid, // Clear the valid bit in the selected way and set output logic SetValid, // Set the dirty bit in the selected way and set output logic ClearDirty, // Clear the dirty bit in the selected way and set output logic SetDirty, // Set the dirty bit in the selected way and set @@ -146,7 +145,6 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( assign SetValid = CurrState == STATE_WRITE_LINE; assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) | (CurrState == STATE_WRITE_LINE & (StoreAMO)); - assign ClearValid = '0; 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) | diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index e5df9456a..32b0a6e82 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -39,7 +39,6 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, input logic [`PA_BITS-1:0] PAdr, // Physical address input logic [LINELEN-1:0] LineWriteData, // Final data written to cache (D$ only) input logic SetValid, // Set the valid bit in the selected way and set - input logic ClearValid, // Clear the valid bit in the selected way and set input logic SetDirty, // Set the dirty bit in the selected way and set input logic ClearDirty, // Clear the dirty bit in the selected way and set input logic SelWriteback, // Overrides cached tag check to select a specific way and set for writeback @@ -71,7 +70,6 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, logic [LINELEN/8-1:0] FinalByteMask; logic SetValidEN; logic SetValidWay; - logic ClearValidWay; logic SetDirtyWay; logic ClearDirtyWay; logic SelNonHit; @@ -94,7 +92,6 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, ///////////////////////////////////////////////////////////////////////////////////////////// assign SetValidWay = SetValid & SelData; - assign ClearValidWay = ClearValid & SelData; assign SetDirtyWay = SetDirty & SelData; assign ClearDirtyWay = ClearDirty & SelData; @@ -154,7 +151,7 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, if(CacheEn) begin ValidWay <= #1 ValidBits[CacheSet]; if(InvalidateCache) ValidBits <= #1 '0; - else if (SetValidEN | (ClearValidWay & ~FlushStage)) ValidBits[CacheSet] <= #1 SetValidWay; + else if (SetValidEN) ValidBits[CacheSet] <= #1 SetValidWay; end end From 54df581ce6a73adef5e4fbc3bee289fb7043ae93 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 5 Apr 2023 11:45:26 -0700 Subject: [PATCH 08/11] make Cache Flush Logic dependent on !READ_ONLY_CACHE read-only caches do not have flush logic since they do not have to deal with dirty bits. --- src/cache/cache.sv | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/cache/cache.sv b/src/cache/cache.sv index 63d882223..c01c714b1 100644 --- a/src/cache/cache.sv +++ b/src/cache/cache.sv @@ -188,19 +188,25 @@ module cache #(parameter LINELEN, NUMLINES, NUMWAYS, LOGBWPL, WORDLEN, MUXINTE // Flush logic ///////////////////////////////////////////////////////////////////////////////////////////// - // Flush address (line number) - assign ResetOrFlushCntRst = reset | FlushCntRst; - flopenr #(SETLEN) FlushAdrReg(clk, ResetOrFlushCntRst, FlushAdrCntEn, FlushAdrP1, NextFlushAdr); - mux2 #(SETLEN) FlushAdrMux(NextFlushAdr, FlushAdrP1, FlushAdrCntEn, FlushAdr); - assign FlushAdrP1 = NextFlushAdr + 1'b1; - assign FlushAdrFlag = (NextFlushAdr == FLUSHADRTHRESHOLD[SETLEN-1:0]); - - // Flush way - flopenl #(NUMWAYS) FlushWayReg(clk, FlushWayCntEn, ResetOrFlushCntRst, {{NUMWAYS-1{1'b0}}, 1'b1}, NextFlushWay, FlushWay); - if(NUMWAYS > 1) assign NextFlushWay = {FlushWay[NUMWAYS-2:0], FlushWay[NUMWAYS-1]}; - else assign NextFlushWay = FlushWay[NUMWAYS-1]; - assign FlushWayFlag = FlushWay[NUMWAYS-1]; + if (!READ_ONLY_CACHE) begin:flushlogic + // Flush address (line number) + assign ResetOrFlushCntRst = reset | FlushCntRst; + flopenr #(SETLEN) FlushAdrReg(clk, ResetOrFlushCntRst, FlushAdrCntEn, FlushAdrP1, NextFlushAdr); + mux2 #(SETLEN) FlushAdrMux(NextFlushAdr, FlushAdrP1, FlushAdrCntEn, FlushAdr); + assign FlushAdrP1 = NextFlushAdr + 1'b1; + assign FlushAdrFlag = (NextFlushAdr == FLUSHADRTHRESHOLD[SETLEN-1:0]); + // Flush way + flopenl #(NUMWAYS) FlushWayReg(clk, FlushWayCntEn, ResetOrFlushCntRst, {{NUMWAYS-1{1'b0}}, 1'b1}, NextFlushWay, FlushWay); + if(NUMWAYS > 1) assign NextFlushWay = {FlushWay[NUMWAYS-2:0], FlushWay[NUMWAYS-1]}; + else assign NextFlushWay = FlushWay[NUMWAYS-1]; + assign FlushWayFlag = FlushWay[NUMWAYS-1]; + end // block: flushlogic + else begin:flushlogic + assign FlushWayFlag = 0; + assign FlushAdrFlag = 0; + end + ///////////////////////////////////////////////////////////////////////////////////////////// // Cache FSM ///////////////////////////////////////////////////////////////////////////////////////////// From 570e86afc38f4770dd428c5b4140dbb971ab87fd Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 5 Apr 2023 11:46:28 -0700 Subject: [PATCH 09/11] Make CacheWay flush and dirty logic dependent on !READ_ONLY_CACHE To increase coverage. Read-only caches do not have flushes since they do not have dirty bits. --- src/cache/cacheway.sv | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index 32b0a6e82..174b82c59 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -74,17 +74,22 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, logic ClearDirtyWay; logic SelNonHit; logic SelData; - logic FlushWayEn, VictimWayEn; - // FlushWay and VictimWay are part of a one hot way selection. Must clear them if FlushWay not selected - // or VictimWay not selected. - assign FlushWayEn = FlushWay & SelFlush; - assign VictimWayEn = VictimWay & SelWriteback; - - assign SelNonHit = FlushWayEn | SetValid | SelWriteback; - - mux2 #(1) seltagmux(VictimWay, FlushWay, SelFlush, SelTag); - + + if (!READ_ONLY_CACHE) begin:flushlogic + logic FlushWayEn; + + mux2 #(1) seltagmux(VictimWay, FlushWay, SelFlush, SelTag); + + // FlushWay is part of a one hot way selection. Must clear it if FlushWay not selected. + assign FlushWayEn = FlushWay & SelFlush; + assign SelNonHit = FlushWayEn | SetValid | SelWriteback; + end + else begin:flushlogic // no flush operation for read-only caches. + assign SelTag = VictimWay; + assign SelNonHit = SetValid; + end + mux2 #(1) selectedwaymux(HitWay, SelTag, SelNonHit , SelData); ///////////////////////////////////////////////////////////////////////////////////////////// @@ -92,11 +97,16 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, ///////////////////////////////////////////////////////////////////////////////////////////// assign SetValidWay = SetValid & SelData; - assign SetDirtyWay = SetDirty & SelData; assign ClearDirtyWay = ClearDirty & SelData; - + if (!READ_ONLY_CACHE) begin + assign SetDirtyWay = SetDirty & SelData; + assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage; + end + else begin + assign SelectedWriteWordEn = SetValidWay & ~FlushStage; + end + // If writing the whole line set all write enables to 1, else only set the correct word. - assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage; assign FinalByteMask = SetValidWay ? '1 : LineByteMask; // OR assign SetValidEN = SetValidWay & ~FlushStage; From 9e3d78de8bb84764c78666ec046111c80ebe7c99 Mon Sep 17 00:00:00 2001 From: Sydeny Date: Wed, 5 Apr 2023 14:10:15 -0700 Subject: [PATCH 10/11] Starting to extend fpu conditional coverage, reformating ifu test cases --- tests/coverage/fpu.S | 6 ++++++ tests/coverage/ifu.S | 20 ++++++-------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/tests/coverage/fpu.S b/tests/coverage/fpu.S index a349ac606..250100a68 100644 --- a/tests/coverage/fpu.S +++ b/tests/coverage/fpu.S @@ -67,6 +67,7 @@ main: # fcvt.w.q a0, ft0 # fcvt.q.d ft3, ft0 + # Completing branch coverage in fctrl.sv .word 0x38007553 // Testing the all False case for 119 - funct7 under, op = 101 0011 .word 0x40000053 // Line 145 All False Test case - illegal instruction? .word 0xd0400053 // Line 156 All False Test case - illegal instruction? @@ -74,6 +75,11 @@ main: .word 0xd2400053 // Line 168 All False Test case - illegal instruction? .word 0xc2400053 // Line 174 All False Test case - illegal instruction? + # Increasing conditional coverage in fctrl.sv + .word 0xc5000007 // Attempting to toggle (Op7 != 7) to 0 on line 97 in fctrl, not sure what instruction this works out to + .word 0xe0101053 // toggling (Rs2D == 0) to 0 on line 139 in fctrl. Illegal Intsr (like fclass but incorrect rs2) + .word 0xe0100053 // toggling (Rs2D == 0) to 0 on line 141 in fctrl. Illegal Intsr (like fmv but incorrect rs2) + # Test illegal instructions are detected .word 0x00000007 // illegal floating-point load (bad Funct3) .word 0x00000027 // illegal floating-point store (bad Funct3) diff --git a/tests/coverage/ifu.S b/tests/coverage/ifu.S index 9cde14ce2..662629916 100644 --- a/tests/coverage/ifu.S +++ b/tests/coverage/ifu.S @@ -35,20 +35,12 @@ main: //.hword 0x2000 // CL type compressed floating-point ld-->funct3,imm,rs1',imm,rd',op // binary version 0000 0000 0000 0000 0010 0000 0000 0000 mv s0, sp - c.fld fs0, 0(s0) + c.fld fs0, 0(s0) // Previously uncovered instructions + c.fsd fs0, 0(s0) + .hword 0x2002 // c.fldsp fs0, 0 + .hword 0xA002 // c.fsdsp fs0, 0 + .hword 0x9C41 // line 134 Illegal compressed instruction - c.fsd fs0, 0(s0) - - // c.fldsp fs0, 0 - .hword 0x2002 - - // c.fsdsp fs0, 0 - .hword 0xA002 - - //# Illegal compressed instruction with op = 01, instr[15:10] = 100111, and 0's everywhere else - //.hword 0x9C01 - - # Line Illegal compressed instruction - .hword 0x9C41 + //.hword 0x9C01 //# Illegal compressed instruction with op = 01, instr[15:10] = 100111, and 0's everywhere else j done From ac3569d75ca4c4aebda19df6c236b43614285fae Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 5 Apr 2023 14:54:58 -0700 Subject: [PATCH 11/11] Update ram1p1rwe (ce & we) coverage exlusion explanation --- src/generic/mem/ram1p1rwe.sv | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/generic/mem/ram1p1rwe.sv b/src/generic/mem/ram1p1rwe.sv index 8a2f971e9..480ad3b45 100644 --- a/src/generic/mem/ram1p1rwe.sv +++ b/src/generic/mem/ram1p1rwe.sv @@ -5,6 +5,8 @@ // Created: 04 April 2023 // // Purpose: ram1p1wre, but without byte-enable. Used for icache data. +// Be careful using this module, since coverage is turned off for (ce & we). +// In read-only caches, we never get (we=1, ce=0), so this waiver is needed. // // Documentation: // @@ -85,8 +87,7 @@ module ram1p1rwe #(parameter DEPTH=64, WIDTH=44) ( // coverage off // ce only goes low when cachefsm is in READY state and Flush is asserted. // for read-only caches, we only goes high in the STATE_WRITE_LINE cachefsm state. - // so we can never get we=1, ce=0 for I$. Note that turning off coverage here - // might miss some cases for D$, however, when we might go high due to a store. + // so we can never get we=1, ce=0 for I$. if (ce & we) // coverage on for(i = 0; i < WIDTH/8; i++)