Author Name: Ted Campbell
Original Redmine Issue: 634 from https://www.veripool.org
Original Date: 2013-03-18
Original Assignee: Wilson Snyder (@wsnyder)

I'm noticing a complex issue with incorrect simulation result in my design. I've managed to reproduce in a simpler testcase as follows.

Tested against verilator 3.846 and a few older 3.8xx versions.

Test Case Requirements

• Multiple top-level clocks
• Dual-port Memory
  ** MULTIDRIVEN with different clocks on each port
  ** Latency 1 read ports
• Clocks packed into / unpacked from vectors

Attached Verilog

The verilog file contains a simple memory writer module that writes data to the memory, and a simple reader that waits a number of cycles and then verifies that data. There is a 'MULTI_CLK' define in the verilog that demonstrates the mismatch. If it is defined, the design fails, if disabled the design behaves as expected.

Observation 1

When using a single top-level clock with two local wires, the design behaves correctly. The concat / bitsel of the clock alone does not cause issues. If I have two top-level clocks, Verilator reports IMPERFECTSCH on them.

Observation 2

If I use a latency 0 memory read, Verilator still reports IMPERFECTSCH, so I believe the simulation scheduler may have issues.

Observation 3

In my original design, it is actually the memory write that is scrambled, although in this reduced case, the memory read is what is incorrect.

Observation 4

When I examine the generated VFoo.cpp, I see the clocks are mapped to VinpClk variables. These are update in the final block, meaning the clock loads see the correct clock only during the second pass of _eval. The problem is that the chosen scheduler is as follows:

  // Partial and simplified

  if ( posedge clk_rd ) sequent_1( );
  if ( posedge clk_wr ) sequent_2( );

  combo_4( );
  if ( posedge clk_rd ) sequent_6( );
  combo_7( );

  if ( posedge VinpClk_a_clk ) sequent_8( );
  if ( posedge VinpClk_b_clk ) sequent_9( );

  if ( posedge clk_rd ) sequent_11( );
  if ( posedge clk_wr ) multiclk_12( );

  VinpClk_a_clk = ... ;
  VinpClk_b_clk = ... ;

This causes sequent_9 to run after multiclk_12 (during the second iteration of _eval), even though there should be a dependency. This causes address to update before rdata is filled in.

Workaround

If I move the VinpClk_a_clk / VinpClk_b_clk assignments after combo_7, the design works correctly. A similar transform fixes my original large design as well.

In theory if this was added to the schedule graph instead of always at the end, the problem could be solved. Unfortunately it isn't entirely clear to me what could be done if a cycle was detected in that region, which doesn't make this a particularly general solution.

Difficulties

The use of VinpClk suggests the clock is following into similar problems as [[issue613]], where the conclusion seems to be there is sizeable amount of overhaul needed in Verilator.

A simpler approach (at least for my needs) may be to figure out why the clock was confused by the concat / bitsel, and possibly have a simple enhancement for this.

Unfortunately I don't have time immediately to tackle this problem, so I'm hoping to at least start some discussion.