myhdl-list

[myhdl-list] debugging Cosimulation with Icarus Verilog

[Günter D. <dan...@we...>]

Hi Jan,

I took some time to look into the error I am getting when doing
cosimulation with Icarus Verilog. The first test that fails in the 
myhdl/test/conversion/toVerilog folder is test1 in the 
test_always_comb.py file.

The design is the following:

def design1(a, b, p):
     def logic():
         p.next = a | b
     return always_comb(logic)


What causes the exception, which stops the simulation is that p returns 
the value 'x'. I traced the problem down and it has to do with the 
change_callback() function in the myhdl.c file. It fires too early, 
already at time step 0. Comparing that with Cver, the change_callback() 
function only fires at time step 2.

Now, the change_callback() function is registered in the 
to_myhdl_calltf() function. To avoid having the change_callback() 
function trigger already in time step 0, wouldn't it make sense to 
register it in the first readonly_callback() function. That way it would 
make sure that it does not fire in time step 0?

Guenter

Re: [myhdl-list] debugging Cosimulation with Icarus Verilog

[Jan D. <ja...@ja...>]

Günter Dannoritzer wrote:
> Hi Jan,
> 
> I took some time to look into the error I am getting when doing
> cosimulation with Icarus Verilog. The first test that fails in the 
> myhdl/test/conversion/toVerilog folder is test1 in the 
> test_always_comb.py file.
> 
> The design is the following:
> 
> def design1(a, b, p):
>      def logic():
>          p.next = a | b
>      return always_comb(logic)
> 
> 
> What causes the exception, which stops the simulation is that p returns 
> the value 'x'. I traced the problem down and it has to do with the 
> change_callback() function in the myhdl.c file. It fires too early, 
> already at time step 0. Comparing that with Cver, the change_callback() 
> function only fires at time step 2.
> 
> Now, the change_callback() function is registered in the 
> to_myhdl_calltf() function. To avoid having the change_callback() 
> function trigger already in time step 0, wouldn't it make sense to 
> register it in the first readonly_callback() function. That way it would 
> make sure that it does not fire in time step 0?

Guenter:

I won't have time in the coming weeks to look at this in much
detail. However, some quick comments:

Based on previous experience, I'm quite reluctant to make big
changes. Given that cver works with the method as implemented,
I'm inclined to think that in principle Icarus should behave the
same it Icarus vpi is done properly.

I do see a difference that may be significant between the two
files:

   // icarus stops on the following line but shouldn't
   // cb_data_s.value = &value_s;
   cb_data_s.value = NULL;

while setting up the callback you mention.

Apparently I had an unexpected problem with Icarus here that
I didn't have with cver. This may explain the issue,
I'm not sure.

As a first attempt, I would try if the original line still
breaks things with recent Icarus releases. If it does, I would
contact the Icarus developer to check whether this qualifies
as a bug.

Jan

-- 
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Kaboutermansstraat 97, B-3000 Leuven, Belgium

Re: [myhdl-list] debugging Cosimulation with Icarus Verilog

[Günter D. <dan...@we...>]

Jan Decaluwe wrote:
...

> 
> Based on previous experience, I'm quite reluctant to make big
> changes. Given that cver works with the method as implemented,
> I'm inclined to think that in principle Icarus should behave the
> same it Icarus vpi is done properly.

I have filed a bug report (#2048463) for the cbValueChange issue on the 
Icarus bug tracker.

http://sourceforge.net/tracker/index.php?func=detail&aid=2048463&group_id=149850&atid=775997

One of the developers already confirmed some issues and also named the 
vpiSuppressTime, which is the reason for the necessary change below:

> 
> I do see a difference that may be significant between the two
> files:
> 
>    // icarus stops on the following line but shouldn't
>    // cb_data_s.value = &value_s;
>    cb_data_s.value = NULL;
> 

In the bug report I also mentioned the time 0 issue, which is, that I 
see a call back happening at time 0, when no signal assignment happens. 
I got a reply that the Icarus developers discussed that before and if I 
think that this is a wrong behavior I should file another bug report.

I compared the Cver behavior with Riviera and both behave the same way. 
If no signal assignment happens at time 0, there is no callback. I will 
investigate that some more and then also file it as bug report.

Guenter

Re: [myhdl-list] debugging Cosimulation with Icarus Verilog

[Jan D. <ja...@ja...>]

Excellent, looks like we're getting somewhere here!

Günter Dannoritzer wrote:
> Jan Decaluwe wrote:
> ...
> 
>> Based on previous experience, I'm quite reluctant to make big
>> changes. Given that cver works with the method as implemented,
>> I'm inclined to think that in principle Icarus should behave the
>> same it Icarus vpi is done properly.
> 
> I have filed a bug report (#2048463) for the cbValueChange issue on the 
> Icarus bug tracker.
> 
> http://sourceforge.net/tracker/index.php?func=detail&aid=2048463&group_id=149850&atid=775997
> 
> One of the developers already confirmed some issues and also named the 
> vpiSuppressTime, which is the reason for the necessary change below:
> 
>> I do see a difference that may be significant between the two
>> files:
>>
>>    // icarus stops on the following line but shouldn't
>>    // cb_data_s.value = &value_s;
>>    cb_data_s.value = NULL;
>>
> 
> In the bug report I also mentioned the time 0 issue, which is, that I 
> see a call back happening at time 0, when no signal assignment happens. 
> I got a reply that the Icarus developers discussed that before and if I 
> think that this is a wrong behavior I should file another bug report.
> 
> I compared the Cver behavior with Riviera and both behave the same way. 
> If no signal assignment happens at time 0, there is no callback. I will 
> investigate that some more and then also file it as bug report.
> 
> Guenter


-- 
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Kaboutermansstraat 97, B-3000 Leuven, Belgium
     From Python to silicon:
     http://myhdl.jandecaluwe.com

Re: [myhdl-list] debugging Cosimulation with Icarus Verilog

[Günter D. <dan...@we...>]

Jan Decaluwe wrote:
> Excellent, looks like we're getting somewhere here!

Some update on the Icarus Verilog bug report #2048463 concerning the
cbValueChange issues. The bug is fixed in the git repository, however,
the difference in the call back behavior at time 0 remains. In the bug
tracker there is some explanation about this behavior:

http://sourceforge.net/tracker/index.php?func=detail&aid=2048463&group_id=149850&atid=775997

I also posted a question about this on comp.lang.verilog and got a reply
from someone at Cadence. The Icarus Verilog developer Steve Williams
replied now to that subject as well. Will see what comes out of that
discussion.

Here is a link to the comp.lang.Verilog discussion:

http://groups.google.com/group/comp.lang.verilog/browse_frm/thread/58628e26e5294c58#

I will look into adjusting the current myhdl.c for Icarus to work with
the t0 behavior.

Guenter

Re: [myhdl-list] debugging Cosimulation with Icarus Verilog

[Jan D. <ja...@ja...>]

Günter Dannoritzer wrote:
> Jan Decaluwe wrote:
>> Excellent, looks like we're getting somewhere here!
> 
> Some update on the Icarus Verilog bug report #2048463 concerning the
> cbValueChange issues. The bug is fixed in the git repository, however,
> the difference in the call back behavior at time 0 remains. In the bug
> tracker there is some explanation about this behavior:
> 
> http://sourceforge.net/tracker/index.php?func=detail&aid=2048463&group_id=149850&atid=775997
> 
> I also posted a question about this on comp.lang.verilog and got a reply
> from someone at Cadence. The Icarus Verilog developer Steve Williams
> replied now to that subject as well. Will see what comes out of that
> discussion.
> 
> Here is a link to the comp.lang.Verilog discussion:
> 
> http://groups.google.com/group/comp.lang.verilog/browse_frm/thread/58628e26e5294c58#
> 
> I will look into adjusting the current myhdl.c for Icarus to work with
> the t0 behavior.

Maybe with the bug fix you can use the assignment that was commented out,
and perhaps that fixes it?

   // icarus stops on the following line but shouldn't
   // cb_data_s.value = &value_s;
   cb_data_s.value = NULL;

-- 
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Kaboutermansstraat 97, B-3000 Leuven, Belgium
     From Python to silicon:
     http://myhdl.jandecaluwe.com

Re: [myhdl-list] debugging Cosimulation with Icarus Verilog

[Günter D. <dan...@we...>]

Jan Decaluwe wrote:
...
> 
> Maybe with the bug fix you can use the assignment that was commented out,
> and perhaps that fixes it?
> 
>    // icarus stops on the following line but shouldn't
>    // cb_data_s.value = &value_s;
>    cb_data_s.value = NULL;
> 

Yes, that part works now. But unfortunately does not fix the toVerilog
test errors.

Some of the toVerilog tests failures are caused by the callback at
simulation time 0. The problem is that the callback at time 0 returns a
value 'x' for the signal. The Cosimulation class does not like that and
due to the exception in the Cosimulation._get(self) function an
intbv(None) instance is returned.

So the solution specific to Icarus would be to register the first
cbValueChange call back at simulation time 1. That could actually be
used for all simulators, as the others do it anyway that way.

BTW, I noticed the change is already part of the latest Icarus Verilog
development snapshot 2008-08-30.

Guenter