steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.81k stars 522 forks source link

data_out in rtl outputs at same clock cycle as data_out_reg (Flip-Flop issue) #1126

Closed abdulhameed-akram closed 4 months ago

abdulhameed-akram commented 4 months ago

The rtl design is as follows:


module design172 #(parameter WIDTH=32) (data_in, data_out, clk, rst);
    input clk;
    input rst;
    input [WIDTH-1:0] data_in; 
    output [WIDTH-1:0] data_out; 

    wire [WIDTH-1:0] wire_d0_0,wire_d0_2;

    d_latch_top #(.WIDTH(WIDTH)) d_latch_instance100(.data_in(data_in),.data_out(wire_d0_0),.clk(clk),.rst(rst));            
    large_mux #(.WIDTH(WIDTH)) large_mux_instance102(.data_in(wire_d0_0),.data_out(wire_d0_2),.clk(clk),.rst(rst));

    assign data_out = wire_d0_2;
endmodule
module d_latch_top #(parameter WIDTH=32) (clk,rst,data_in,data_out);
    input clk;
    input rst;
    input [WIDTH-1:0] data_in;
    output [WIDTH-1:0] data_out;

    reg enable;
    wire [WIDTH-1:0] d_out;

    always @ (posedge clk) begin
        if (rst)
            enable <= 0;
        else
            enable <= 1;
    end

    d_latch #(.WIDTH(WIDTH)) d_latch_inst (.clk(clk),.rst(rst),.data_in(data_in),.data_out(d_out),.en(enable));

    assign data_out=d_out;
endmodule

module d_latch #(parameter WIDTH=32) (clk,rst,data_in,data_out,en);
    input [WIDTH-1:0] data_in;
    input en;
    input clk;
    input rst;
    output reg [WIDTH-1:0] data_out;

    always @ (en or rst or data_in)
        if (rst)
            data_out <= 0;
        else if (!en)
            data_out <= 0;
        else    
            data_out <= data_in;
endmodule

module large_mux 
#(parameter WIDTH=32)
(
    input clk,
    input rst,
    input [WIDTH-1:0] data_in,
    output reg [WIDTH-1:0] data_out
);

reg [WIDTH-1:0] data_out_reg;

// Output logic: either reset or update data_out from data_out_reg
always @(posedge clk or posedge rst) begin
    if (rst)
        data_out <= 0;
    else
        data_out <= data_out_reg;
end

// Handle data_in translation into data_out_reg
always @(posedge clk) begin
    case(data_in[3:0])  // Focus on the lower 4 bits
        4'b0000: data_out_reg = {24'b0, data_in[7:0]};
        4'b0001: data_out_reg = {16'b0, data_in[15:8], 8'b0};
        4'b0010: data_out_reg = {8'b0, data_in[23:16], 16'b0};
        4'b0011: data_out_reg = {data_in[31:24], 24'b0};

        4'b0100: data_out_reg = {16'b0, data_in[15:8], 8'b0};
        4'b0101: data_out_reg = {8'b0, data_in[23:16], 16'b0};
        4'b0110: data_out_reg = {data_in[31:24], 24'b0};
        4'b0111: data_out_reg = {24'b0, data_in[7:0]};

        4'b1000: data_out_reg = {8'b0, data_in[23:16], 16'b0};
        4'b1001: data_out_reg = {data_in[31:24], 24'b0};
        4'b1010: data_out_reg = {24'b0, data_in[7:0]};
        4'b1011: data_out_reg = {16'b0, data_in[15:8], 8'b0};

        4'b1100: data_out_reg = {data_in[31:24], 24'b0};
        4'b1101: data_out_reg = {24'b0, data_in[7:0]};
        4'b1110: data_out_reg = {16'b0, data_in[15:8], 8'b0};
        4'b1111: data_out_reg = {8'b0, data_in[23:16], 16'b0};

        default: data_out_reg = 32'd0;
    endcase
end

endmodule

When simulated with following testbench :

module large_mux_tb;

    reg clk;
    reg rst;
    reg [31:0] data_in;
    wire [31:0] data_out;

design172 dut (.*);

always #1 clk=~clk;

initial begin
  rst = 'b1;
  clk = 'b0;
  data_in = 'd0;
  repeat(5) @(negedge clk);
  rst = 'b0;
  repeat(5) @(negedge clk);
  data_in = 'b1111_1111_1111_1111_1111_1111_1111_0001;
  repeat(4) @(negedge clk);
  data_in = 'b1111_1111_1111_1111_1111_1111_1111_1010;
  repeat(4) @(negedge clk);
  repeat(4) @(negedge clk);
  $finish;
end

initial begin
    $dumpfile("tb.vcd");
    $dumpvars;
end
endmodule

Result in following waveform: image

Here we see data_out and data_out_reg are on same cycle, even though data_out should be on next cycle.

if we only instantiate large mux, then we get correct result i.e data_out on next cycle after data_out_reg.

NickOveracker commented 4 months ago

NOTE: I am not an iverilog contributor.

It's been a spell since I've written synchronous logic, so forgive me if I'm mistaken:

You are assigning data_out <= data_out_reg on posedge clk, and you are also updating data_out_reg on the same posedge. Since there are no delays in the modules, I think that this probably is the expected behavior.

Can you assign data_out <= data_out_reg on negedge clk instead?

martinwhitaker commented 4 months ago

It is in fact non-deterministic. Either result is possible, depending on the order in which the simulator processes events. Always use non-blocking assignments when modelling flip-flops to avoid this.

martinwhitaker commented 4 months ago

Closing as this is not an Icarus Verilog issue.