[Iverilog-devel] vpi access to arrays

[Jared C. <jar...@gm...>]

Starting to dig into this code and thought I'd do a bit of clean up
while I'm down here.  I noticed a couple things that bring up some
questions...

vpi_get(vpiType, h) on a var array is currently returns vpiMemory.
>From section 36.12.1 in 1800-2012:

"Unpacked unidimensional reg arrays were exclusively characterized as
vpiMemory objects in
IEEE Std 1364-1995, and later deprecated in IEEE Std 1364-2001. This
object type was replaced by
vpiRegArray in IEEE Std 1364-2005, leaving vpiMemory allowed as only a
one-to-many
transition for IEEE Std 1364-2005 and IEEE 1800 standards (see 37.19).
IEEE Std 1364-2001
allowed either vpiMemory or vpiRegArray types to represent unpacked
unidimensional arrays of
vpiReg objects."

Which I read to mean it is now not conforming to return vpiMemory.
There is a similar issue with vpiMemoryWord.

That is easy enough to change, of course, but there isn't currently
the standard approved way to deal with this deprecation (i.e. section
36.12 in 1800-2012).   This would involve adding vpi_compatibility.h
and the corresponding vpi_*_1364v1995, vpi_*_1364v2001, etc.
functions, as well as maybe a command line option to vvp to specify
what to use ala section 36.12.2.2.   Any problems with doing so?  Are
there legal issues with bringing in the vpi_compatibility.h from the
standard?  I noticed vpi_user.h is quite a bit different from that in
the standard, but there are only so many ways to #define the things in
vpi_compatibility.h

---

Another issue: vpi_iterate() currently wants vpiMemoryWord as the type
for all arrays.   This is valid for var arrays (i.e. reg arrays or
memories), but only for backward compatibility so vpiReg should also
be be okay.  For net arrays, the type should be vpiNet. Trying to
iterate over vpiMemoryWord on a net array should result in a null
iterator.  (this is based on my reading of the standard and
experimenting a bit with vcs). Making that change to net arrays would
bring Icarus in conformance with the standard but could be a breaking
change for existing code.  Maybe we want to continue to allow
vpiMemoryWord to iterate over nets in a net array?

This also brings up some style questions I had with this code...
currently there is code like this in vpi_get:

if (property == vpiType) {
   if (ref->get_type_code() == vpiMemory && is_net_array(ref))
     return vpiNetArray;
   else
     return ref->get_type_code();
}

This seems to be a relic from before the change to a class hierarchy
for vpiHandle objects.  Since you want different things for at least
get_type_code and vpi_iterate, it seems like this could benefit from
following the pattern used in vpi_scope.cc and define a struct
vpiVarArray : public __vpiArray, and a similar vpiNetArray, each with
their own get_type_code() and vpi_iterate().

But then you need to make the appropriate data type in the
compile_*_array functions, so vpip_make_array becomes
vpip_make_{net/var}_array... or do you move that code into the
appropriate constructors? Now you have separate classes for net arrays
and var arrays, so you move the nets member into vpiNetArray, the
vals4 in vpiVarArray, possibly make a vpiRealArray for the vals
member.  All the code in arrays.cc that looks like "if I'm a net array
do this, if I'm a var array do this" become virtual functions or ugly
type checks.  Suddenly it's a pretty big change to arrays.cc.  I guess
my question is how much "clean up" would be wanted here, and how much
C++-ness to use?

Sorry for the long email.  Any thoughts would be appreciated.

Jared