🐛 Various bugfixes in store_queue

This commit is contained in:
Florian Zaruba 2017-04-28 15:56:44 +02:00
parent 89042efdba
commit cd56694fb0
5 changed files with 176 additions and 58 deletions

View file

@ -57,7 +57,7 @@ $(tests):
# Optimize top level
vopt${questa_version} ${compile_flag} $@_tb -o $@_tb_optimized +acc -check_synthesis
# vsim${questa_version} $@_tb_optimized
vsim${questa_version} -c +UVM_TESTNAME=$@_test -coverage -do "coverage save -onexit $@.ucdb; run -a; quit -code [coverage attribute -name TESTSTATUS -concise]" $@_tb_optimized
# vsim${questa_version} -c +UVM_TESTNAME=$@_test -coverage -do "coverage save -onexit $@.ucdb; run -a; quit -code [coverage attribute -name TESTSTATUS -concise]" $@_tb_optimized
# User Verilator to lint the target
lint:
verilator ${src} --lint-only \

View file

@ -57,8 +57,6 @@ module store_queue (
// In the current implementation this is represented by a single entry and
// differentiated by the is_speculative flag.
enum logic { IDLE, WAIT_RVALID } CS, NS;
struct packed {
logic [63:0] address;
logic [63:0] data;
@ -68,10 +66,10 @@ module store_queue (
} commit_queue_n, commit_queue_q;
// we can directly output the commit entry since we just have one element in this "queue"
assign paddr_o = commit_queue_q.address;
assign data_o = commit_queue_q.data;
assign be_o = commit_queue_q.be;
assign valid_o = commit_queue_q.valid;
assign paddr_o = commit_queue_q.address;
assign data_o = commit_queue_q.data;
assign be_o = commit_queue_q.be;
assign valid_o = commit_queue_q.valid;
// those signals can directly be output to the memory if
assign address_o = commit_queue_q.address;
@ -81,44 +79,32 @@ module store_queue (
// memory interface
always_comb begin : store_if
// if there is no commit pending and the uncommitted queue is empty as well we can accept the request
automatic logic ready = ~commit_queue_q.valid;
ready_o = ready;
// if we got a grant this implies that the value was not speculative anymore and that we
// do not need to save the values anymore since the memory already processed them
automatic logic ready = ~commit_queue_q.valid | data_gnt_i;
ready_o = ready;
commit_queue_n = commit_queue_q;
data_we_o = 1'b1; // we will always write in the store queue
data_req_o = 1'b0;
NS = CS;
// there should be no commit when we are flushing
if (~flush_i) begin
case (CS)
// we can accept a new request if we were idle
IDLE: begin
if (commit_queue_q.valid) begin
data_req_o = 1'b1;
if (data_gnt_i) begin// advance to the next state if we received the grant
NS = WAIT_RVALID;
end
end
// if the entry in the commit queue is valid and not speculative anymore
// we can issue this instruction
// we can issue it as soon as the commit_i goes high or any number of cycles later
// by looking at the is_speculative flag
if (commit_queue_q.valid & (~commit_queue_q.is_speculative | commit_i)) begin
data_req_o = 1'b1;
if (data_gnt_i) begin// advance to the next state if we received the grant
// NS = WAIT_RVALID;
// we can evict it from the commit buffer
commit_queue_n.valid = 1'b0;
end
WAIT_RVALID: begin
if (data_rvalid_i) begin
// if we got the rvalid we can already perform a new store request
if (commit_queue_q.valid) begin
// we can evict it from the commit buffer
commit_queue_n.valid = 1'b0;
data_req_o = 1'b1;
end else // if do not need to perform another
NS = IDLE;
end else // wait here for the rvalid
NS = WAIT_RVALID;
end
default:;
endcase
end
// we ignore the rvalid signal for now as we assume that the store
// happened
end
// shift the store request from the speculative buffer
@ -129,7 +115,7 @@ module store_queue (
// LSU interface
// we are ready to accept a new entry and the input data is valid
if (ready & valid_i & ~commit_i) begin
if (ready & valid_i) begin
commit_queue_n.address = paddr_i;
commit_queue_n.data = data_i;
commit_queue_n.be = be_i;
@ -137,19 +123,28 @@ module store_queue (
commit_queue_n.is_speculative = 1'b1;
end
// when we flush evict the speculative store
if (flush_i & commit_queue_q.is_speculative) begin
commit_queue_n.valid = 1'b0;
end
end
// registers
always_ff @(posedge clk_i or negedge rst_ni) begin : proc_
if(~rst_ni) begin
commit_queue_q <= '{default: 0};
CS <= IDLE;
end else if (flush_i) begin // just empty the speculative queue
commit_queue_q <= commit_queue_n;
CS <= NS;
end else begin
commit_queue_q <= commit_queue_n;
CS <= NS;
end
end
`ifndef SYNTHESIS
`ifndef verilator
// assert that commit is never set when we are flushing this would be counter intuitive
// as flush and commit is decided in the same stage
assert property (
@(posedge clk_i) rst_ni & flush_i |-> ~commit_i)
else $error ("You are trying to commit and flush in the same cycle");
`endif
`endif
endmodule

View file

@ -41,8 +41,8 @@ interface store_queue_if
wire [DATA_WIDTH/8-1:0] store_be;
clocking mck @(posedge clk);
output flush, commit, valid, store_paddr, store_data, store_be;
input check_paddr, check_data, check_be, ready, store_valid;
output flush, commit, valid, store_paddr, store_data, store_be, store_valid;
input check_paddr, check_data, check_be, ready;
endclocking

View file

@ -20,25 +20,27 @@
module store_queue_tb;
logic rst_ni, clk;
logic store_valid;
logic slave_data_gnt;
mem_if slave(clk);
store_queue_if store_queue(clk);
store_queue dut (
.clk_i ( clk ),
.rst_ni ( rst_ni ),
.flush_i ( store_queue.flush ),
.paddr_o ( store_queue.check_paddr ),
.data_o ( store_queue.check_data ),
.valid_o ( store_queue.valid ),
.be_o ( store_queue.check_be ),
.commit_i ( store_queue.commit ),
.ready_o ( store_queue.ready ),
.valid_i ( store_queue.store_valid ),
.paddr_i ( store_queue.store_paddr ),
.data_i ( store_queue.store_data ),
.be_i ( store_queue.store_be ),
.clk_i ( clk ),
.rst_ni ( rst_ni ),
.flush_i ( store_queue.flush ),
.paddr_o ( store_queue.check_paddr ),
.data_o ( store_queue.check_data ),
.valid_o ( store_queue.valid ),
.be_o ( store_queue.check_be ),
.commit_i ( store_queue.commit ),
.ready_o ( store_queue.ready ),
.valid_i ( store_queue.store_valid ),
.paddr_i ( store_queue.store_paddr ),
.data_i ( store_queue.store_data ),
.be_i ( store_queue.store_be ),
.address_o ( slave.address ),
.data_wdata_o ( slave.data_wdata ),
@ -60,6 +62,11 @@ module store_queue_tb;
#10ns clk = ~clk;
end
// combinatorial signals
assign store_queue.flush = 1'b0;
assign store_queue.store_valid = store_valid & store_queue.ready;
assign slave.data_gnt = slave_data_gnt;
// simulator stopper, this is suboptimal better go for coverage
initial begin
#10000000ns
@ -67,13 +74,85 @@ module store_queue_tb;
end
program testbench (mem_if slave, store_queue_if store_queue);
// data to store
class store_data;
rand logic [63:0] data;
rand logic [7:0] be;
endclass : store_data
store_data data = new();
semaphore sem = new(1);
// ----------
// Driver
// ----------
// make a new store if it is possible
initial begin
// reset assignment
store_valid <= 'b0;
store_queue.mck.store_paddr <= 'b0;
store_queue.mck.store_data <= 'b0;
store_queue.mck.store_be <= 'b0;
store_queue.mck.commit <= 1'b0;
wait(rst_ni);
forever begin
@(store_queue.mck);
store_queue.mck.commit <= 1'b0;
if (store_queue.mck.ready) begin
store_queue.mck.store_paddr <= 64'h1;
store_valid <= 1'b1;
// get some randomized data
void'(data.randomize());
store_queue.mck.store_data <= data.data;
store_queue.mck.store_be <= data.be;
// commit a couple of cycles later
fork
commit_block: begin
sem.get(1);
@(store_queue.mck);
@(store_queue.mck);
store_queue.mck.commit <= 1'b1;
sem.put(1);
end
join_none
end else begin
store_valid <= 1'b0;
end
end
end
// commit part
initial begin
end
// grant process
initial begin
forever begin
slave_data_gnt = 1'b0;
wait (slave.data_req);
// randomize grant delay
// repeat ($urandom_range(0,4)) @(slave.mck);
slave_data_gnt = 1'b1;
wait (~slave.data_req);
end
end
// write memory interface
initial begin
forever begin
@(slave.mck);
if (slave.mck.data_req)
slave.mck.data_rvalid <= 1'b1;
else
slave.mck.data_rvalid <= 1'b0;
end
end
// -------------------
// Monitor && Checker

44
tb/wave_store_queue.do Normal file
View file

@ -0,0 +1,44 @@
onerror {resume}
quietly WaveActivateNextPane {} 0
add wave -noupdate /store_queue_tb/dut/clk_i
add wave -noupdate /store_queue_tb/dut/rst_ni
add wave -noupdate /store_queue_tb/dut/flush_i
add wave -noupdate /store_queue_tb/dut/paddr_o
add wave -noupdate /store_queue_tb/dut/data_o
add wave -noupdate /store_queue_tb/dut/valid_o
add wave -noupdate /store_queue_tb/dut/be_o
add wave -noupdate /store_queue_tb/dut/commit_i
add wave -noupdate /store_queue_tb/dut/ready_o
add wave -noupdate /store_queue_tb/dut/valid_i
add wave -noupdate /store_queue_tb/dut/paddr_i
add wave -noupdate /store_queue_tb/dut/data_i
add wave -noupdate /store_queue_tb/dut/be_i
add wave -noupdate /store_queue_tb/dut/address_o
add wave -noupdate /store_queue_tb/dut/data_wdata_o
add wave -noupdate /store_queue_tb/dut/data_req_o
add wave -noupdate /store_queue_tb/dut/data_we_o
add wave -noupdate /store_queue_tb/dut/data_be_o
add wave -noupdate /store_queue_tb/dut/data_gnt_i
add wave -noupdate /store_queue_tb/dut/data_rvalid_i
add wave -noupdate /store_queue_tb/dut/CS
add wave -noupdate /store_queue_tb/dut/NS
add wave -noupdate /store_queue_tb/dut/commit_queue_n
add wave -noupdate /store_queue_tb/dut/commit_queue_q
TreeUpdate [SetDefaultTree]
WaveRestoreCursors {{Cursor 1} {0 ns} 0}
quietly wave cursor active 0
configure wave -namecolwidth 150
configure wave -valuecolwidth 100
configure wave -justifyvalue left
configure wave -signalnamewidth 1
configure wave -snapdistance 10
configure wave -datasetprefix 0
configure wave -rowmargin 4
configure wave -childrowmargin 2
configure wave -gridoffset 0
configure wave -gridperiod 1
configure wave -griddelta 40
configure wave -timeline 0
configure wave -timelineunits ns
update
WaveRestoreZoom {0 ns} {13479 ns}