I have implemented an 8 bit serial-in parallel-out register in SystemVerilog and I'm trying to test it. I'm using Icarus Verilog as simulator.

In the test bench, I send 8 bits and wait for the rising edge of a signal, then check the obtained parallel buffer. The problem is that, after waiting for the rising edge, the parallel buffer has not the expected value. However, if I add a #0 delay to the assert it does work.

The signal on which I'm waiting the rising edge, and the buffer that should contain the expected value are assigned as:

assign rdy = (i == 7) & was_enabled;
assign out_data = {8{rdy}} & buff;

I know buff contains the right value, then, how is that on the rising edge of rdy, rdy is effectively 1 but out_data is still 0?

Wave dump

Note: See how when rdy goes high out_data is 0xaa.

Code

Serial-in parallel-out register

module sipo_reg(
    input   wire        in_data,
    output  wire [7:0]  out_data,
    output  wire        rdy,
    input   wire        en,
    input   wire        rst,
    input   wire        clk
);
    reg [7:0] buff;
    reg [2:0] i;
    reg was_enabled;

    wire _clk;

    always @(posedge _clk, posedge rst) begin
        if (rst) begin
            buff <= 0;
            i <= 7;
            was_enabled <= 0;
        end else begin
            was_enabled <= 1;
            buff[i] <= in_data;
            i <= i == 0 ? 7 : (i - 1);
        end
    end

    assign _clk = en | clk;  // I know this is a very bad practice, I'm on it...
    assign rdy = (i == 7) & was_enabled;
    assign out_data = {8{rdy}} & buff;
endmodule

Test bench:

module utils_sipo_reg_tb;
    reg clk = 1'b1;
    wire _clk;

    always #2 clk = ~clk;
    assign _clk = clk | en;


    reg in_data = 1'b0, rst = 1'b0, en = 1'b0;
    wire [7:0] out_data;
    wire rdy;

    sipo_reg dut(in_data, out_data, rdy, en, rst, clk);


    integer i = 0;

    initial begin
        $dumpfile(`VCD);
        $dumpvars(1, utils_sipo_reg_tb);
            en = 1;
        #4  rst = 1;
        #4  rst = 0;
            assert(out_data === 8'b0);
            assert(rdy === 1'b0);

        //
        // read 8 bits works
        //
        #4 en = 0;
        for (i = 0; i < 8; i = i   1) begin
            @(negedge _clk) in_data = ~in_data;
        end
        en = 1;
        @(posedge rdy);
        assert(rdy === 1'b1);
        assert(out_data === 8'haa);  // <-- This fails, but works if I add a '#0' delay.
        #20;
        $finish;
    end
endmodule

I have tried to replace these lines

assign rdy = (i == 7) & was_enabled;
assign out_data = {8{rdy}} & buff;

by these

assign rdy = (i == 7) & was_enabled;
assign out_data = {8{((i == 7) & was_enabled)}} & buff;

because I suspected the simulator was 'calculating' out_data after rdy because the former depends on the latter. However, they are still continuous assignments with 0 delay, I would expect them to get their value at the exact same time (unless a delay is added).

Would it be a good design practice to add a few picoseconds of delay after each @(posedge signal) to make sure everything is settled by the simulator?

CodePudding user response：

You have a race condition in your testbench because you are trying to sample a signal at a time where it is changing. All digital systems have inherent race conditions, and the way to deal with them is to only sample your signals when you know they are stable.

In your case, you could use a small numeric delay as you have suggested. However, since you have a clock signal, if you know that changes to signals only occur on the posedge of the clock, you could sample signals at the negedge:

    @(posedge rdy);
    @(negedge clk);
    assert(rdy === 1'b1);
    assert(out_data === 8'haa);

This is a more robust approach than using a numeric delay since it scales better (no need to worry about picking the best numeric delay value).

CodePudding user response：

This is a synchronous design and your assertion should synchronous as well. That means only using one edge, the (positive) clock edge. Once you start using other signal edges, you run into race conditions between statements waiting for the signal to change, which includes both the @(posedge rdy) procedural delay and the assign out_data = {8{rdy}} & buff; continuous assignment.

There are two approaches to fixing this in your testbench:

Do not use @(posedge rdy) in your prodedural code. Use

    @(posedge clk iff (rdy === 1'b1));
    assert(out_data === 8'haa);

Since i and was_enabled are both updated with nonblocking assignments, rdy gets sampled with its old value, as well as out_data in the assertion that follows.

Another option is using a concurrent assertion which is outside of any procedural code

assert property (@(posedge clk) $rose(rdy) |-> out_data === 8'haa);

This reads "When rdy has risen, this implies on the same cycle that out_data must be 8'haa"