starting from version 10.0 when signals are set from VPI they updated but their combination is not

[vvavrychuk]

I have found that starting from v10.0 iverilog/vvp can delay updating signals combination when signals are set from VPI. This issue is not present in v0.9.7.

This is happening only in certain case. I prepared minimal reproducible example below. It is also available in https://gitlab.com/vvavrychuk/vpi-issue.

This issue makes some testbenches, for example https://github.com/ultraembedded/cores/tree/master/uart, not working on newer Icarus Verilog (ultraembedded/cores#4).

I use following Verilog code for test

module tb_top;
reg clk;
reg a, b;
wire c;
assign c = a & b;

always @ (posedge clk)
  $display("a = ", a, ", ",
           "b = ", b, ", ",
           "c = ", c);
endmodule

In VPI code on simulation start I set initial values and schedule timer

static int simulation_started_cb(p_cb_data cb_data)
{
    set_value("tb_top.clk", 0);
    set_value("tb_top.a", 0);
    set_value("tb_top.b", 0);
    start_clock_timer();
}

static void startup_routine(void)
{
    s_cb_data cb_data;
    cb_data.user_data = NULL;
    cb_data.reason    = cbStartOfSimulation;
    cb_data.cb_rtn    = simulation_started_cb;
    cb_data.time      = NULL;
    cb_data.value     = NULL;
    vpi_register_cb(&cb_data);
}

void (*vlog_startup_routines[])() = 
{
    startup_routine,
    0
};

In timer handler I set other values

static int clock_timer_cb(p_cb_data cb_data)
{
    set_value("tb_top.clk", 1);
    set_value("tb_top.a", 1);
    set_value("tb_top.b", 1);
}

As result I get

a = 1, b = 1, c = 0

when running simulation.

Helper functions used in VPI are:

static int get_value(const char* name)
{
    vpiHandle handle = vpi_handle_by_name(name, NULL);
    assert(handle != NULL);

    s_vpi_value value;
    value.format = vpiIntVal;
    vpi_get_value(handle, &value);

    return value.value.integer;
}

static void set_value(const char* name, int value)
{
    vpiHandle handle = vpi_handle_by_name(name, NULL);
    assert(handle != NULL);

    s_vpi_value value_s;
    value_s.format = vpiIntVal;
    value_s.value.integer = value;

    s_vpi_time time;
    time.type = vpiSimTime;
    time.high = 0;
    time.low  = 0;

    vpi_put_value(handle, &value_s, &time, vpiInertialDelay);
}

static void start_clock_timer()
{
    s_vpi_time time;
    time.type = vpiSimTime;
    time.high = 0;
    time.low  = 1;

    s_cb_data cb_data;
    cb_data.user_data = NULL;
    cb_data.reason    = cbAfterDelay;
    cb_data.cb_rtn    = clock_timer_cb;
    cb_data.time      = &time;
    cb_data.value     = NULL;
    vpi_register_cb(&cb_data);
}

[martinwhitaker]

I don't see that this is a bug. Setting the values of a and b adds an update event for c to the stratified event queue. Setting the value of clk triggers the @(posedge clk), which adds an evaluation event for that process to the stratified event queue. The simulator is allowed to execute those events in any order.

[vvavrychuk]

@martinwhitaker sorry, I made a mistake and my above minimal reproducible example does not work both on v0.9.7 and v10.0.

I have updated my example in https://gitlab.com/vvavrychuk/vpi-issue and now on v10.0 it gives

vvp -m issue -M . issue.vvp
a = 1, b = 1, c = 0

and on v0.9.7 it gives

vvp -m issue -M . issue.vvp
a = 0, b = 0, c = 0

Maybe this changes something?

Let me explain how my minimal reproducible example is done now.

Now my clock is coming from Verilog so that tb_top.v looks like

module tb_top;
reg clk = 0;
reg a = 0, b = 0;
wire c;
assign c = a & b;

initial #1 clk = 1;

always @ (posedge clk)
  $display("a = ", a, ", ",
           "b = ", b, ", ",
           "c = ", c);
endmodule

Helper functions get_value and set_value are unchanged. In vlog_startup_routines I register for cbStartOfSimulation as before. In simulation started even handler I subscribe for clock change event

s_cb_data cb_data = {0};
cb_data.reason    = cbValueChange;
cb_data.obj       = vpi_handle_by_name("tb_top.clk", NULL);
cb_data.cb_rtn    = clock_value_changed_cb;

s_vpi_time time   = {0};
time.type         = vpiSimTime;
cb_data.time      = &time;

s_vpi_value value = {0};
value.format      = vpiIntVal;
cb_data.value     = &value;

vpi_register_cb(&cb_data);

In clock change event I check that I get high value and change a and b to high value

if (cb_data->value->value.integer != 0) {
    set_value("tb_top.a", 1);
    set_value("tb_top.b", 1);
}

I think behaviour of v10.0 is not correct because I use vpiInertialDelay.

[martinwhitaker]

The IEEE standard doesn't specify the scheduling behaviour for vpi_put_value with a delay. Icarus schedules it as an active update event in the appropriate time slot. If I had implemented it, I would have chosen to make it a nonblocking assign update event, but I can't say the current Icarus behaviour is wrong.

So, your "clk = 1" assignment adds active update events for a and b to the event queue and also adds an active evaluation event for the always @(posedge clk) process to the event queue. The simulator may choose to process these events in any order. If it chooses to process the @(posedge clk) event first, the display output will be "a = 0, b = 0, c = 0". If however it processes the a and b update events first, these will cause an active evaluation event for the "a & b" expression to be added to the event queue. The simulator now has the choice of processing either the @(posedge clk) event or the "a & b" evaluation event. If it chooses to process the @(posedge clk) event first, the display output will be "a = 1, b = 1, c = 0". If however it processes the "a & b" evaluation event, this will add an active update event for c to the event queue. The simulator now has the choice of processing either the @(posedge clk) event or the c update event. If it chooses to process the @(posedge clk) event first, the display output will be "a = 1, b = 1, c = 0". If however it processes the c update event first, the display output will be "a = 1, b = 1, c = 1".

Extending the above to process the a and b update events either side of the @(posedge clk) event, the range of permissible outputs becomes

a = 0, b = 0, c = 0
a = 1, b = 0, c = 0
a = 0, b = 1, c = 0
a = 1, b = 1, c = 0
a = 1, b = 1, c = 1

[vvavrychuk]

@martinwhitaker consider usual case of device under test in Verilog and testbench that includes VPI, or SystemC which interfaces Verilog though VPI (as it is done for example here https://github.com/ultraembedded/cores/tree/master/uart/testbench). I shown it on diagram:

Both parts are synchronized by clock. If we set some data from VPI synchronously to the clock and then DUT synchronosly to the same clock gets some data updated but an combinatorial function of them not update then we can not implement this testbench in VPI.

You say

If I had implemented it, I would have chosen to make it a nonblocking assign update event

That seems exactly what is needed. Could you please suggest if there is a way from VPI side to influence that.

[red0bear]

@vvavrychuk

Can you see this https://github.com/GLADICOS/SPACEWIRESYSTEMC

[steveicarus]

I think this is a conversation best had on the iverilog-devel mailing list.

…

On Tue, Feb 5, 2019 at 10:36 AM Felipe Fernandes da Costa < ***@***.***> wrote: hello. Can you see this https://github.com/GLADICOS/SPACEWIRESYSTEMC — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#223 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAlMlqgUJDpEaJBO0A0uOxVw-DK06D2ks5vKc88gaJpZM4ZnAiL> .

-- Steve Williams "The woods are lovely, dark and deep. [email protected] <[email protected]> But I have promises to keep, http://www.icarus.com and lines to code before I sleep, http://www.picturel.com And lines to code before I sleep."

[vvavrychuk]

I was able to solve my problem by using system task function to advance VPI simulation

always @ (posedge clk)
    $clock_change;

Idea used in @red0bear.

It works (as expected) only when I use vpiInertialDelay (even zero one):

iverilog -o issue.vvp tb_top.v
gcc -shared -fPIC -o issue.vpi -I/usr/include/iverilog/ issue_vpi.c
vvp -m issue -M . issue.vvp
time =                    1, a = 0, b = 0, c = 0
clock_change_calltf, time = 1
clock_change_calltf, time = 3
time =                    3, a = 0, b = 0, c = 0

But if I use vpiNoDelay I get:

gcc -shared -fPIC -o issue.vpi -I/usr/include/iverilog/ issue_vpi.c
vvp -m issue -M . issue.vvp
time =                    1, a = 0, b = 0, c = 0
clock_change_calltf, time = 1
clock_change_calltf, time = 3
time =                    3, a = 1, b = 1, c = 0

Full code is in https://gitlab.com/vvavrychuk/vpi-issue/tree/advance-using-system-tf.

Still not sure that it will work forever and will not break with some Icarus Verilog version. Because as it is was explained by @martinwhitaker Icarus Verilog is free to choose between added vpiInertialDelay with zero delay and already pending events.

So only correct solution for me seems to use non-zero inertial delay when changing values from VPI code.