Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange initialisation behaviour with "VinpClk" cloned clock variables #1327

Closed
veripoolbot opened this issue Aug 9, 2018 · 4 comments
Closed

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Aug 9, 2018


Author Name: Rupert Swarbrick (@rswarbrick)
Original Redmine Issue: 1327 from https://www.veripool.org

Original Assignee: Wilson Snyder (@wsnyder)


We're seeing a strange behaviour in our system testbench. I've tracked
it down to a problem with (seemingly) spurious edges on a clock
variable. I think that Verilator is misbehaving, but if this is
correct and we're just Doing It Wrong (TM), it would be great to
understand why.

Our system testbench includes a DUT and a memory block connected
together. We generate clock and (asynchronous) reset signals for these
two blocks by gating the clock and reset signals of the testbench
using an FSM which steps through the test.

As a result of this logic, I think Verilator decides that the
testbench's "memory_rst_n" variable (which is wired to the reset pin
of the memory) has a cycle and generates a "VinpClk" variable for it.

In the memory model, there is a block of the form

always @(posedge clk or negedge rst_n) begin
$display("%0d %0d", clk, rst_n);
if (rst_n) begin
...
end else begin
...
end
end

The logic to decide when to schedule the always block generates the
following C++ code:

if ((((IData)(vlTOPp->clk) & (~ (IData)(vlTOPp->__Vclklast__TOP__clk)))
| ((~ (IData)(vlTOPp->__VinpClk__TOP__top__DOT__tb_i__DOT__memory_rst_n))
& (IData)(vlTOPp->__Vclklast__TOP____VinpClk__TOP__top__DOT__tb_i__DOT__memory_rst_n)))) {
vlTOPp->_sequent__TOP__5(vlSymsp);
vlTOPp->__Vm_traceActivity = (8U | vlTOPp->__Vm_traceActivity);
vlTOPp->_sequent__TOP__6(vlSymsp);
}

This makes sense to me: the block executes on a posedge of the
toplevel clock or a negedge of the renamed memory_rst_n variable.

Reads from the memory_rst_n variable in the block get turned into code
like this:

VL_WRITEF("%0# %0#\n",1,vlTOPp->clk,1,(IData)(vlTOPp->top__DOT__tb_i__DOT__memory_rst_n));
if (VL_UNLIKELY(vlTOPp->top__DOT__tb_i__DOT__memory_rst_n)) {
...
}

When running our testbench, we call Verilated::randReset(2) to ask for
random initialisation. What we see is that the always block shown
above executes at initialisation time but the $display call shows
clk=0 and rst_n=1 (so there shouldn't have been either type of edge).
Shortly afterwards, the simulation falls over because the memory model
doesn't expect this to happen.

Investigating with GDB shows the following values:

(gdb) p vlTOPp->top__DOT__tb_i__DOT__memory_rst_n
$1 = 1 '\001'
(gdb) p vlTOPp->__Vchglast__TOP__top__DOT__tb_i__DOT__memory_rst_n
$2 = 0 '\000'
(gdb) p vlTOPp->__VinpClk__TOP__top__DOT__tb_i__DOT__memory_rst_n
$3 = 0 '\000'
(gdb) p vlTOPp->__Vclklast__TOP____VinpClk__TOP__top__DOT__tb_i__DOT__memory_rst_n
$4 = 1 '\001'

That is, there has actually been a posedge on rst_n, but there is an
apparent negedge on the VinpClk clone.

Since this seemed to be some sort of strange reset behaviour, I did
some further digging to see how these variables were initialised:

$ grep -n 'top__DOT__tb_i__DOT__memory_rst_n.*=' *.cpp | grep -v -- '->'
Vtop__10__Slow.cpp:410: __VinpClk__TOP__top__DOT__tb_i__DOT__memory_rst_n = 0;
Vtop__10__Slow.cpp:412: __Vclklast__TOP____VinpClk__TOP__top__DOT__tb_i__DOT__memory_rst_n = VL_RAND_RESET_I(1);
Vtop__10__Slow.cpp:418: __Vchglast__TOP__top__DOT__tb_i__DOT__memory_rst_n = 0;
Vtop__8__Slow.cpp:255: top__DOT__tb_i__DOT__memory_rst_n = 0;

Note that all the fields except the "last" version of the cloned
variable are initialised to zero. If I hack the generated C++ code to
initialise all four fields to zero, the spurious negedge goes away and
everything works.

The initial values of these fields seem to be controlled by the
"zeroit" flag in V3EmitC.cpp, but I don't know enough about
Verilator's type infrastructure to get further easily.

Is it possible that something that controls the underlying type of the
variable wasn't copied across in ClockVisitor::getCreateLastClk()?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Aug 11, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-08-11T15:37:02Z


I suspect the code should be changed to at init time after rand reset, set all last vars = the normal non-last value.

Does doing that fix your example?

Can you try to make an example in test regress format (see the docs) showing the problem please?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Aug 30, 2018


Original Redmine Comment
Author Name: Rupert Swarbrick (@rswarbrick)
Original Date: 2018-08-30T11:16:04Z


Hi there,

Sorry for the slow reply. I've written a silly test that shows this. I suspect it's a little flaky (because it depends on the random number generator giving different initialisations for the rst_n variables). If it doesn't fail for you, try changing the randReset to a different number.

When I run things, this is what I get:

rjs@rjs-argon:~/tool-source/verilator/test_regress$ ./driver.pl --vlt t_vinpclk_initialisation.pl
======================================================================
vlt/t_vinpclk_initialisation: ==================================================
vlt/t_vinpclk_initialisation: Compile
         perl ../bin/verilator --prefix Vt_vinpclk_initialisation --x-assign unique -cc -Mdir obj_vlt/t_vinpclk_initialisation -OD --debug-check --comp-limit-members 10 -CFLAGS -g3 --exe /home/rjs/tool-source/verilator/test_regress/t/t_vinpclk_initialisation.cpp --clk clk  -f input.vc +define+TEST_OBJ_DIR=obj_vlt/t_vinpclk_initialisation t/t_vinpclk_initialisation.v    > obj_vlt/t_vinpclk_initialisation/vlt_compile.log
vlt/t_vinpclk_initialisation: GCC
         make -C obj_vlt/t_vinpclk_initialisation -f /home/rjs/tool-source/verilator/test_regress/Makefile_obj VM_PREFIX=Vt_vinpclk_initialisation TEST_OBJ_DIR=obj_vlt/t_vinpclk_initialisation CPPFLAGS_DRIVER=-DT_VINPCLK_INITIALISATION  MAKE_MAIN=0  Vt_vinpclk_initialisation    > obj_vlt/t_vinpclk_initialisation/vlt_gcc.log
make: Entering directory '/home/rjs/tool-source/verilator/test_regress/obj_vlt/t_vinpclk_initialisation'
g++  -I.  -MMD -I/home/rjs/tool-source/verilator/test_regress/../include -I/home/rjs/tool-source/verilator/test_regress/../include/vltstd -DVL_PRINTF=printf -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=0 -faligned-new -Wno-bool-operation -Wno-sign-compare -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable -Wno-shadow     -g3  -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_vinpclk_initialisation -DVM_PREFIX=Vt_vinpclk_initialisation -DVM_PREFIX_INCLUDE="<Vt_vinpclk_initialisation.h>" -DT_VINPCLK_INITIALISATION    -c -o t_vinpclk_initialisation.o /home/rjs/tool-source/verilator/test_regress/t/t_vinpclk_initialisation.cpp
/usr/bin/perl /home/rjs/tool-source/verilator/test_regress/../bin/verilator_includer -DVL_INCLUDE_OPT=include Vt_vinpclk_initialisation.cpp > Vt_vinpclk_initialisation__ALLcls.cpp
g++  -I.  -MMD -I/home/rjs/tool-source/verilator/test_regress/../include -I/home/rjs/tool-source/verilator/test_regress/../include/vltstd -DVL_PRINTF=printf -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=0 -faligned-new -Wno-bool-operation -Wno-sign-compare -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable -Wno-shadow     -g3  -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_vinpclk_initialisation -DVM_PREFIX=Vt_vinpclk_initialisation -DVM_PREFIX_INCLUDE="<Vt_vinpclk_initialisation.h>" -DT_VINPCLK_INITIALISATION    -c -o Vt_vinpclk_initialisation__ALLcls.o Vt_vinpclk_initialisation__ALLcls.cpp
/usr/bin/perl /home/rjs/tool-source/verilator/test_regress/../bin/verilator_includer -DVL_INCLUDE_OPT=include Vt_vinpclk_initialisation__Syms.cpp > Vt_vinpclk_initialisation__ALLsup.cpp
g++  -I.  -MMD -I/home/rjs/tool-source/verilator/test_regress/../include -I/home/rjs/tool-source/verilator/test_regress/../include/vltstd -DVL_PRINTF=printf -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=0 -faligned-new -Wno-bool-operation -Wno-sign-compare -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable -Wno-shadow     -g3  -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_vinpclk_initialisation -DVM_PREFIX=Vt_vinpclk_initialisation -DVM_PREFIX_INCLUDE="<Vt_vinpclk_initialisation.h>" -DT_VINPCLK_INITIALISATION    -c -o Vt_vinpclk_initialisation__ALLsup.o Vt_vinpclk_initialisation__ALLsup.cpp
       Archiving Vt_vinpclk_initialisation__ALL.a ...
ar r Vt_vinpclk_initialisation__ALL.a Vt_vinpclk_initialisation__ALLcls.o Vt_vinpclk_initialisation__ALLsup.o
ranlib Vt_vinpclk_initialisation__ALL.a
g++    t_vinpclk_initialisation.o verilated.o Vt_vinpclk_initialisation__ALL.a    -o Vt_vinpclk_initialisation -lm -lstdc++
make: Leaving directory '/home/rjs/tool-source/verilator/test_regress/obj_vlt/t_vinpclk_initialisation'
vlt/t_vinpclk_initialisation: Run
         obj_vlt/t_vinpclk_initialisation/Vt_vinpclk_initialisation    > obj_vlt/t_vinpclk_initialisation/vlt_sim.log
Oh dear! 'always @(posedge clk or negedge rst_n)' block triggered with clk=0, rst_n=1.
%Error: t/t_vinpclk_initialisation.v:22: Verilog $stop
Aborting...
%Warning: vlt/t_vinpclk_initialisation: Exec of obj_vlt/t_vinpclk_initialisation/Vt_vinpclk_initialisation failed: Oh dear! 'always @(posedge clk or negedge rst_n)' block triggered with clk=0, rst_n=1.

vlt/t_vinpclk_initialisation: %Error: Exec of obj_vlt/t_vinpclk_initialisation/Vt_vinpclk_initialisation failed: Oh dear! 'always @(posedge clk or negedge rst_n)' block triggered with clk=0, rst_n=1.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Aug 31, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-08-31T00:06:51Z


Fixed in git towards 4.000.

As suspected randomization of VinpClk confused it. Note BTW that not getting edges at time zero is what this test expected and is the default, but the -x-initial-edge can override this behavior - that had a similar bug when randomized.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 16, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-16T21:28:19Z


In 4.002.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.