Search code examples
testingsystem-verilogdigital-design

Continuous assignment with 0 delay not getting the expected value after a signal positive edge


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

enter image description here

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?


Solution

  • 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).