#681 $monitor, $strobe, etc. can not use complex expressions

[Xingzhi Wen]

Compilation passed, but when run vvp I got
vvp: vthread.cc:144: vvp_bit4_t thr_get_bit(vthread_s*, unsigned int): Assertion `addr < thr->bits4.size()' failed.
Aborted

The file is too big to upload, so I loaded to a temporary server.
http://76.103.58.87:8080/bug_icarus/assert.tar.gz
tar ball of the directory where vvp a.out is executed.

The same design was successfully simulated in Cadence ncverilog.

The compilation command I used is
iverilog -f xyz

the file xyz is as below.

+define+NON_SYNTHE_MEM_CTRL
+define+AVOID_CDS_INTERNAL_ERROR
+define+NOT_USE_DRAM_XMT
+incdir+.
-y .
+incdir+$(XMTV_PATH)/synthe
+incdir+$(DW_LIB_PATH)
-y $(XMTV_PATH)/synthe/asic
-y $(XMTV_PATH)/synthe
-y $(DW_LIB_PATH)
+libext+.v
$(XMTV_PATH)/synthe/run_xmt.v

[Cary R.]

I was able to reproduce the problem here with the provided a.out file. It appears that either the compiler or a run time operator is asking to get the value of an out of range thread bit. The thread space grows dynamically when it is assigned to, but it asserts that the getting of bits is in range. We will need to find some way to track down what is causing the problem and create a more manageable test.

[Cary R.]

I was able to track this down a bit more. It is the $monitor in .../repos/synthe/run_xmt.v on line 175 that is the root of the problem. I would like to see that statement to confirm what is going wrong, but this appears to be a bit select of an array select e.g.(RegFile[7][4]). If that is the case we have a conceptual solution for this problem that we have not had time to implement. Given that we now have failing user code that raises the priority significantly.

Development note:

If my assumptions are correct then we need to implement &APV<> to fix this problem. We should also think about adding a compile time check to $monitor and friends to make sure they are not called with a &T<> since this will often/always? cause problems like this. As a minimum the value from &T<> will be incorrect.

[Nobody/Anonymous]

The monitor statement in my Verilog code is

$monitor($time,,"counter/256 = %h, rd_cnt/256 = %h parallel: %b low/16: %h high/16: %h",counter>>8,rd_cnt>>8,xmt_i.parallel_n,xmt_i.mcluster_i.GR_i.RegFile[6]>>4,xmt_i.mcluster_i.GR_i.RegFile[7]>>4);

It looks like releavant to the example you gave, RegFile[7][4]

[Cary R.]

My example was only slightly relevant. It appears that the compiler is creating invalid code for the >> operator, possibly only for VPI arguments. It is creating a bit select not a part select. The other issue is that to make this work correctly with $monitor we need to add the code I mentioned earlier.

You can work around the problem for now by creating the appropriate continuous assign and then monitoring that variable. For example:

wire [31:0] temp_high = ...RegFile[7]>>4;

$monitor(..., temp_high);

Adding the &APV<> code will change the compiler/vvp interface so that portion of the patch will likely only be available in the development branch (0.10.devel). We may do an exception for this particular case, but do not count on it being available in V0.9.

Both Steve and I are busy working on other issues and since you should be able to work around this with a continuous assign I am not raising the priority. We will discuss the V0.10.devel vs V0.9 issue once we have a fix available.

Please let us know if the temporary continuous assign works for you.

[Cary R.]

I have submitted a patch that fixes the width calculation problem (bit select) since that was a critical compiler problem. The implementation of &APV<> will be a bit longer.

[Xingzhi Wen]

Thanks for quick response.

I also tried the work-around you suggested.
After rewriting $monitor statement, the simulation finished correctly.
My design is pretty big and I am so glad that I can use Icarus for simulation.
One other problem I found is: (it may be a known issue).
reg [3:0] x;
$monitor("%h",x[3:2]);
it prints out even when lower 2 bits are changing, but not the upper 2 bits.

[Cary R.]

That doesn't surprise me. I believe Icarus does not currently support value change callbacks on pieces of a vector, so any change triggers the callback. It was likely thought that this was a better choice than not supporting them at all. I'll look at this and add an appropriate report or maybe a fix when I have some free time.

[Cary R.]

I have submitted pr2785294 as the report for the value change of a bit/part select problem. It will need to be fixed before this (adding &APV<>) will work correctly. The example in pr2785294 has code that should test &APV<> when it is implemented.

[Cary R.]

There was a duplicate bug report (2915691) that while on the surface looks different is really the same. It was trying to monitor the expression ((res === 1'b0) ? "0" : "1") which is not one of the things we currently support. I expect to add support for this some time next year. The work around for this is always to assign the expression to a dummy variable in a continuous assignment and then monitor the dummy variable. I am going to make the summary line a little more generic.

Just so everyone knows Icarus currently only support monitoring, strobing a variable, a bit/part select of a variable or an array word.

I'm also switching this to development since that is where it will be fixed.

[Cary R.]

Steve has submitted patches in development that fix the remaining problems originally reported in this report, missing support for &APV<> and correct sensitivity to bit selects by the value change callback . The expression rework is still needed to fix the duplicate report pr2915691. I'm going to leave this report open as a place holder for the fact that VPI routines cannot handle generic expressions as expected.

[Cary R.]

br916 was closed as a duplicate of this report. As a stop gap measure checks were added to the compiletf routines for $strobe and $monitor to prevent the user from accessing expressions from thread space. An appropriate warning is now printed.