Possible cxxrtl simulation bug

[galibert]

Version

Yosys 0.13+10 (git sha1 36482680d, clang 13.0.0 -fPIC -Os)

On which OS did this happen?

Linux

Reproduction Steps

https://og.kervella.org/sgen.tar.bz2

This code builds a simulation of the atari st shifter from the AtariST_Mister repository. The issue is with the pxCtrEn signal (pcen in the log, obtained by running sftrun) which can be set by three things:

• nReset -> 0, sets to 0
• load_d2 -> 1, sets to 1
• Reload -> 0, sets to the current value of load_d2

nReset is set to 1 near the start. The problematic behaviour is at cycle 189: 186 [LD:0 DE:0 D:0000] cidx=0 load1=0 load2=0 pcnt=e rd=f pcen=1 rdn=1 reload=0 s=0000000000000000 d=0123456789abcdef 187 [LD:0 DE:0 D:0000] cidx=0 load1=0 load2=0 pcnt=f rd=f pcen=1 rdn=1 reload=0 s=0000000000000000 d=0123456789abcdef 188 [LD:0 DE:0 D:0000] cidx=0 load1=0 load2=0 pcnt=0 rd=f pcen=1 rdn=1 reload=1 s=0000000000000000 d=0123456789abcdef 189 [LD:0 DE:0 D:0000] cidx=0 load1=0 load2=0 pcnt=1 rd=0 pcen=1 rdn=0 reload=0 s=0123456789abcdef d=0123456789abcdef 190 [LD:0 DE:0 D:0000] cidx=1 load1=0 load2=0 pcnt=2 rd=0 pcen=1 rdn=1 reload=0 s=123456789abcdef0 d=0123456789abcdef

load_d2 (load2) is 0, reload goes from 1 to 0, yet pcen stays 1.

The verilog code is: reg pxCtrEn; always @(negedge Reload, negedge nReset, posedge load_d2) begin if (!nReset) pxCtrEn <= 1'b0; else if (load_d2) pxCtrEn <= 1'b1; else pxCtrEn <= load_d2; end

Expected Behavior

pcen goes to 0, which will resync the pixel counter for the next video line.

Actual Behavior

pcen stays at 1, and the next line is broken.

[whitequark]

always @(negedge Reload, negedge nReset, posedge load_d2) begin

I don't think this is synthesizable Verilog.

[mwkmwkmwk]

always @(negedge Reload, negedge nReset, posedge load_d2) begin

I don't think this is synthesizable Verilog.

It is. DFF with async reset at !nReset, async set at load_d2, and clock of !Reload (with the usual lovely synth-sim mismatch that happens in Verilog when you have both set and reset).

It also has a completely pointless load from load_d2 in the clock path, which will obviously always be 0 when that path is reached, but that's not the point here.

The problem in this case appears to be dependency tracking between two FFs where one's output drivers the other's clock (the Reload signal in this case, specifically) — cxxrtl emits an eval() function that unconditionally sets converged to be true, and fails to notice that changing the Reload signal may require a recomputation.

Here is a reduced testcase:

module top(input wire clk, output reg q = 0);

reg clk2 = 0;

always @(posedge clk)
        clk2 <= 1;

always @(posedge clk2)
        q <= 1;

endmodule

#include "top.cc"

int main() {
        cxxrtl_design::p_top top;
        top.step();
        for (int i = 0; i < 4; i++) {
                top.p_clk.set(false);
                top.step();
                printf("%u %u\n", top.p_q.curr.data[0], top.p_q.next.data[0]);
                top.p_clk.set(true);
                top.step();
                printf("%u %u\n", top.p_q.curr.data[0], top.p_q.next.data[0]);
        }
}

It will always print all-0, despite the 0-to-1 transition on clk2.

[whitequark]

The problem in this case appears to be dependency tracking between two FFs where one's output drivers the other's clock (the Reload signal in this case, specifically) — cxxrtl emits an eval() function that unconditionally sets converged to be true, and fails to notice that changing the Reload signal may require a recomputation.

Oh yeah, this is a known issue then. This will be addressed in the new dependency tracking algorithm.

[galibert]

Cool, let me know if/when you want me to test a new version.