vpi memory iterator #655
Comments
|
Original Redmine Comment The bug seems reasonable. The test case wasn't in your branch though so I couldn't test it to make sure. I'm not sure why you need the lhs()/rhs() functions to be in so many classes or virtual. I think this will cause problems later as want to move to supporting more complex data types and a var itself doesn't have a single simple range. BTW I know it was my fault originally for using lsb/msb, but we should use lo/hi/left/right for function terminology match what IEEE says. If it won't mess you up I can do a simple replace in trunk with no functional change to clean that up. |
|
Original Redmine Comment I've added a test case to the branch - test_regress/t_vpi_memory, the other test case I mentioned previously was just a throw away piece of code but can supply if you really want. The only simulator I have access to is icarus, I've been trying to get the vpi test to run on icarus but not done it yet. I'll be happier if it agrees. I haven't updated t_vpi_var testcase as it now fails because it expects vpiMemoryWord to return a range object (the old behaviour). Don't know how I ended up with lhs()/rhs() - probably the way VerilatedVpioRange was constructed. What is required is a range() encapsulation of these (I've changed VerilatedVpioRange so it has a range reference instead). The range() method is still in the same places and virtual, though - follows the same pattern as the other accessors. You will know the scope of the changes better than I for the search & replace. Commit the update & I'll merge it to the this branch. |
|
Original Redmine Comment I've ported the t_vpi_memory test to iverilog, and it passes for both verilator and iverilog which increases my confidence in the vpi changes. Sorry about any bastardisation to driver.pl; I've never really used perl, and didn't have time to really understand any nuances in how that script works. |
|
Original Redmine Comment Great, I'll merge your changes then do the other name cleanup. |
|
Original Redmine Comment Pushed to git towards 3.851. Thanks again for patching! |
|
Original Redmine Comment I've ported the t_vpi_memory test to iverilog, and it passes for both verilator and iverilog which increases my confidence in the vpi changes. Sorry about any bastardisation to driver.pl; I've never really used perl, and didn't have time to really understand any nuances in how that script works. |
|
Original Redmine Comment Oooops. Didn't mean to repost that last comment. Sorry. But while I'm here I'll point out that the t_vpi_var issue wasn't fixed in the patch that you pulled. I don't know if you ran a full regression but if you did you'll have seen it didn't pass? Don't pull what's currently there because I've got lost trying to get t_vpi_var to work for icarus too. Do you want me to patch t_vpi_var? Let me know & I'll tell you when you can pull the patch. |
|
Original Redmine Comment I had just made t_vpI_var match the return values, assuming your fixes were ok. If that's not right a fix would be appreciated. |
|
Original Redmine Comment Fixed a while ago. |
veripoolbot
added
resolution: fixed
…uhdm-tests/serv/serv-e59fe53 Bump uhdm-tests/serv/serv from `f04a510` to `e59fe53`
Author Name: Rich Porter
Original Redmine Issue: 655 from https://www.veripool.org
Original Date: 2013-06-05
Original Assignee: Rich Porter
I think that the iterator returned by vpi_iterate with a type of vpiMemoryWord is incorrect. The diagram in IEEE 1364, 26.6.9 suggests a memory word iterator is returned not a range iterator as in the current implementation.
26.6.7 suggests that a range object iterator is returned when a reg array is iterated on with vpiRange.
I compiled a test case with a 4 state simulator (cver) that returned a memory word iterator with vpiMemoryWord and not a range, but as it does not support reg arrays I was unable to verify the range iterator.
There is a proposed fix on the branch vpi-memory at https://github.com/rporter/verilator, see
rporter/verilator@master...vpi-memory
It also adds some properties e.g. vpiSize, vpiIndex as well as vpiScope and vpiParent to variable objects.
If you think this is correct I'll add a new testcase.
And I think that VerilatedVpioVarIndex be renamed to VerilatedVpioMemoryWord.
The text was updated successfully, but these errors were encountered: