VPI memory access for packed arrays (#2922)

src/V3AstNodes.h

@@ -935,6 +935,7 @@ public:
     const VNumRange& nrange() const { return m.m_nrange; }
     int hi() const { return (rangep() ? rangep()->hiConst() : m.m_nrange.hi()); }
     int lo() const { return (rangep() ? rangep()->loConst() : m.m_nrange.lo()); }
+    int elements() const { return (rangep() ? rangep()->elementsConst() : m.m_nrange.elements()); }
     int left() const { return littleEndian() ? lo() : hi(); }  // How to show a declaration
     int right() const { return littleEndian() ? hi() : lo(); }
     bool littleEndian() const {

src/V3EmitCSyms.cpp

@@ -762,6 +762,7 @@ void EmitCSyms::emitSymImp() {
             AstScope* scopep = it->second.m_scopep;
             AstVar* varp = it->second.m_varp;
             //
+            int pwidth = 1;
             int pdim = 0;
             int udim = 0;
             string bounds;
@@ -773,6 +774,7 @@ void EmitCSyms::emitSymImp() {
                     bounds += ",";
                     bounds += cvtToStr(basicp->lo());
                     pdim++;
+                    pwidth *= basicp->elements();
                 }
                 for (AstNodeDType* dtypep = varp->dtypep(); dtypep;) {
                     dtypep
@@ -784,6 +786,7 @@ void EmitCSyms::emitSymImp() {
                         bounds += cvtToStr(adtypep->right());
                         if (VN_IS(dtypep, PackArrayDType)) {
                             pdim++;
+                            pwidth *= adtypep->elementsConst();
                         } else {
                             udim++;
                         }
@@ -793,8 +796,14 @@ void EmitCSyms::emitSymImp() {
                     }
                 }
             }
-            //
+            // TODO: actually expose packed arrays as vpiRegArray
-            if (udim > 1 && (pdim && udim)) {
+            if (pdim > 1 && udim == 0) {
+                bounds = ", ";
+                bounds += cvtToStr(pwidth - 1);
+                bounds += ",0";
+                pdim = 1;
+            }
+            if (pdim > 1 || udim > 1) {
                 puts("//UNSUP ");  // VerilatedImp can't deal with >2d or packed arrays
             }
             puts(protect("__Vscope_" + it->second.m_scopeName) + ".varInsert(__Vfinal,");

test_regress/t/TestCheck.h

@@ -54,6 +54,15 @@ static const bool verbose = false;
         } \
     } while (0)
 
+#define TEST_CHECK_Z(got) \
+    do { \
+        if ((got)) { \
+            std::cout << std::dec << "%Error: " << __FILE__ << ":" << __LINE__ << std::hex \
+                      << ": GOT!= NULL   EXP=NULL" << std::endl; \
+            ++errors; \
+        } \
+    } while (0)
+
 #define TEST_CHECK_NZ(got) \
     do { \
         if (!(got)) { \

test_regress/t/t_vpi_get.cpp

@@ -173,7 +173,7 @@ int mon_check_props() {
            {"sub.the_intf.bytesig", {8, vpiNoDirection, 0, vpiReg}, {0, 0, 0, 0}},
            {"sub.the_intf.param", {32, vpiNoDirection, 0, vpiParameter}, {0, 0, 0, 0}},
            {"sub.the_intf.lparam", {32, vpiNoDirection, 0, vpiParameter}, {0, 0, 0, 0}},
-           {"twobytwo", {2, vpiNoDirection, 0, vpiMemory}, {2, vpiNoDirection, 0, vpiMemoryWord}},
+           {"twobytwo", {4, vpiNoDirection, 0, vpiReg}, {0, 0, 0, 0}},
            {NULL, {0, 0, 0, 0}, {0, 0, 0, 0}}};
     struct params* value = values;
     while (value->signal) {

test_regress/t/t_vpi_memory.cpp

@@ -33,6 +33,7 @@
 
 #include "TestSimulator.h"
 #include "TestVpi.h"
+#include "TestCheck.h"
 
 // __FILE__ is too long
 #define FILENM "t_vpi_memory.cpp"
@@ -41,116 +42,103 @@
     if (0) printf
 
 unsigned int main_time = 0;
+int errors = 0;
 
 //======================================================================
 
-#define CHECK_RESULT_VH(got, exp) \
+void _mon_check_range(const TestVpiHandle& handle, int size, int left, int right) {
-    if ((got) != (exp)) { \
-        printf("%%Error: %s:%d: GOT = %p   EXP = %p\n", FILENM, __LINE__, (got), (exp)); \
-        return __LINE__; \
-    }
-
-#define CHECK_RESULT_NZ(got) \
-    if (!(got)) { \
-        printf("%%Error: %s:%d: GOT = NULL  EXP = !NULL\n", FILENM, __LINE__); \
-        return __LINE__; \
-    }
-
-// Use cout to avoid issues with %d/%lx etc
-#define CHECK_RESULT(got, exp) \
-    if ((got) != (exp)) { \
-        std::cout << std::dec << "%Error: " << FILENM << ":" << __LINE__ << ": GOT = " << (got) \
-                  << "   EXP = " << (exp) << std::endl; \
-        return __LINE__; \
-    }
-
-#define CHECK_RESULT_HEX(got, exp) \
-    if ((got) != (exp)) { \
-        std::cout << std::dec << "%Error: " << FILENM << ":" << __LINE__ << std::hex \
-                  << ": GOT = " << (got) << "   EXP = " << (exp) << std::endl; \
-        return __LINE__; \
-    }
-
-#define CHECK_RESULT_CSTR(got, exp) \
-    if (strcmp((got), (exp))) { \
-        printf("%%Error: %s:%d: GOT = '%s'   EXP = '%s'\n", FILENM, __LINE__, \
-               (got) ? (got) : "<null>", (exp) ? (exp) : "<null>"); \
-        return __LINE__; \
-    }
-
-#define CHECK_RESULT_CSTR_STRIP(got, exp) CHECK_RESULT_CSTR(got + strspn(got, " "), exp)
-
-int _mon_check_range(const TestVpiHandle& handle, int size, int left, int right) {
     s_vpi_value value;
     value.format = vpiIntVal;
     value.value.integer = 0;
     // check size of object
     {
         int vpisize = vpi_get(vpiSize, handle);
-        CHECK_RESULT(vpisize, size);
+        TEST_CHECK_EQ(vpisize, size);
     }
     int coherency;
     {
         // check left hand side of range
         TestVpiHandle left_h = vpi_handle(vpiLeftRange, handle);
-        CHECK_RESULT_NZ(left_h);
+        TEST_CHECK_NZ(left_h);
         vpi_get_value(left_h, &value);
-        CHECK_RESULT(value.value.integer, left);
+        TEST_CHECK_EQ(value.value.integer, left);
         coherency = value.value.integer;
     }
     {
         // check right hand side of range
         TestVpiHandle right_h = vpi_handle(vpiRightRange, handle);
-        CHECK_RESULT_NZ(right_h);
+        TEST_CHECK_NZ(right_h);
         vpi_get_value(right_h, &value);
-        CHECK_RESULT(value.value.integer, right);
+        TEST_CHECK_EQ(value.value.integer, right);
         coherency -= value.value.integer;
     }
     // calculate size & check
     coherency = abs(coherency) + 1;
-    CHECK_RESULT(coherency, size);
+    TEST_CHECK_EQ(coherency, size);
-    return 0;  // Ok
 }
 
-int _mon_check_memory() {
+void _mem_check(const char* name, int size, int left, int right, int words) {
     s_vpi_value value;
-    value.format = vpiIntVal;
-    value.value.integer = 0;
     s_vpi_error_info e;
 
-    vpi_printf((PLI_BYTE8*)"Check memory vpi ...\n");
+    vpi_printf((PLI_BYTE8*)"Check memory vpi (%s) ...\n", name);
-    TestVpiHandle mem_h = vpi_handle_by_name((PLI_BYTE8*)TestSimulator::rooted("mem0"), NULL);
+    TestVpiHandle mem_h = vpi_handle_by_name((PLI_BYTE8*)TestSimulator::rooted(name), NULL);
-    CHECK_RESULT_NZ(mem_h);
+    TEST_CHECK_NZ(mem_h);
-    {
     // check type
     int vpitype = vpi_get(vpiType, mem_h);
-        CHECK_RESULT(vpitype, vpiMemory);
+    if (vpitype != vpiMemory && vpitype != vpiReg) {
+        printf("%%Error: %s:%d vpiType neither vpiMemory or vpiReg: %d\n", FILENM, __LINE__,
+               vpitype);
+        errors++;
+    }
+    std::string binStr;
+    for (int i = words; i >= 1; i--) {
+        for (int pos = size - 1; pos >= 0; pos--) {
+            int posValue = (i >> pos) & 0x1;
+            binStr += posValue ? "1" : "0";
+        }
     }
-    if (int status = _mon_check_range(mem_h, 16, 16, 1)) return status;
     // iterate and store
-    {
+    if (vpitype == vpiMemory) {
+        _mon_check_range(mem_h, words, words, 1);
         TestVpiHandle iter_h = vpi_iterate(vpiMemoryWord, mem_h);
         int cnt = 0;
         while (TestVpiHandle lcl_h = vpi_scan(iter_h)) {
+            value.format = vpiIntVal;
             value.value.integer = ++cnt;
             vpi_put_value(lcl_h, &value, NULL, vpiNoDelay);
+            TEST_CHECK_Z(vpi_chk_error(&e));
             // check size and range
-            if (int status = _mon_check_range(lcl_h, 32, 31, 0)) return status;
+            _mon_check_range(lcl_h, size, left, right);
         }
         iter_h.freed();  // IEEE 37.2.2 vpi_scan at end does a vpi_release_handle
-        CHECK_RESULT(cnt, 16);  // should be 16 addresses
+        TEST_CHECK_EQ(cnt, words);  // should be words addresses
+    } else {
+        int expSize = size * words;
+        _mon_check_range(mem_h, expSize, expSize - 1, 0);
+        value.format = vpiBinStrVal;
+        value.value.str = const_cast<char*>(binStr.c_str());
+        vpi_put_value(mem_h, &value, NULL, vpiNoDelay);
+        TEST_CHECK_Z(vpi_chk_error(&e));
     }
-    {
+    if (vpitype == vpiMemory) {
         // iterate and accumulate
         TestVpiHandle iter_h = vpi_iterate(vpiMemoryWord, mem_h);
         int cnt = 0;
         while (TestVpiHandle lcl_h = vpi_scan(iter_h)) {
             ++cnt;
+            value.format = vpiIntVal;
             vpi_get_value(lcl_h, &value);
-            CHECK_RESULT(value.value.integer, cnt);
+            TEST_CHECK_Z(vpi_chk_error(&e));
+            TEST_CHECK_EQ(value.value.integer, cnt);
         }
         iter_h.freed();  // IEEE 37.2.2 vpi_scan at end does a vpi_release_handle
-        CHECK_RESULT(cnt, 16);  // should be 16 addresses
+        TEST_CHECK_EQ(cnt, words);  // should be words addresses
+    } else {
+        value.format = vpiBinStrVal;
+        vpi_get_value(mem_h, &value);
+        TEST_CHECK_Z(vpi_chk_error(&e));
+        TEST_CHECK_EQ(std::string(value.value.str), binStr);
     }
 
     // don't care for non verilator
@@ -158,58 +146,77 @@ int _mon_check_memory() {
     if (TestSimulator::is_icarus()) {
         vpi_printf((PLI_BYTE8*)"Skipping property checks for simulator %s\n",
                    TestSimulator::get_info().product);
-        return 0;  // Ok
+        return;  // Ok
     }
     {
         // make sure trying to get properties that don't exist
         // doesn't crash
         TestVpiHandle iter_h = vpi_iterate(vpiMemoryWord, mem_h);
         int should_be_0 = vpi_get(vpiSize, iter_h);
-        CHECK_RESULT(should_be_0, 0);
+        TEST_CHECK_EQ(should_be_0, 0);
         should_be_0 = vpi_get(vpiIndex, iter_h);
-        CHECK_RESULT(should_be_0, 0);
+        TEST_CHECK_EQ(should_be_0, 0);
         vpiHandle should_be_NULL = vpi_handle(vpiLeftRange, iter_h);
-        CHECK_RESULT(should_be_NULL, 0);
+        TEST_CHECK_EQ(should_be_NULL, 0);
         should_be_NULL = vpi_handle(vpiRightRange, iter_h);
-        CHECK_RESULT(should_be_NULL, 0);
+        TEST_CHECK_EQ(should_be_NULL, 0);
         should_be_NULL = vpi_handle(vpiScope, iter_h);
-        CHECK_RESULT(should_be_NULL, 0);
+        TEST_CHECK_EQ(should_be_NULL, 0);
     }
-    {
+    if (vpitype == vpiMemory) {
         // check vpiRange
         TestVpiHandle iter_h = vpi_iterate(vpiRange, mem_h);
-        CHECK_RESULT_NZ(iter_h);
+        TEST_CHECK_NZ(iter_h);
         TestVpiHandle lcl_h = vpi_scan(iter_h);
-        CHECK_RESULT_NZ(lcl_h);
+        TEST_CHECK_NZ(lcl_h);
         {
             TestVpiHandle side_h = vpi_handle(vpiLeftRange, lcl_h);
-            CHECK_RESULT_NZ(side_h);
+            TEST_CHECK_NZ(side_h);
             vpi_get_value(side_h, &value);
-            CHECK_RESULT(value.value.integer, 16);
+            TEST_CHECK_EQ(value.value.integer, 16);
         }
         {
             TestVpiHandle side_h = vpi_handle(vpiRightRange, lcl_h);
-            CHECK_RESULT_NZ(side_h);
+            TEST_CHECK_NZ(side_h);
             vpi_get_value(side_h, &value);
-            CHECK_RESULT(value.value.integer, 1);
+            TEST_CHECK_EQ(value.value.integer, 1);
             // check writing to vpiConstant
             vpi_put_value(side_h, &value, NULL, vpiNoDelay);
-            CHECK_RESULT_NZ(vpi_chk_error(&e));
+            TEST_CHECK_NZ(vpi_chk_error(&e));
         }
         {
             // iterator should exhaust after 1 dimension
             TestVpiHandle zero_h = vpi_scan(iter_h);
             iter_h.freed();  // IEEE 37.2.2 vpi_scan at end does a vpi_release_handle
-            CHECK_RESULT(zero_h, 0);
+            TEST_CHECK_EQ(zero_h, 0);
         }
     }
-    return 0;  // Ok
+}
+
+struct params {
+    const char* name;
+    int size;
+    int left;
+    int right;
+    int words;
+};
+
+void _mon_check_memory() {
+    // See note in t_vpi_get.cpp about static
+    static struct params values[]
+        = {{"mem0", 32, 31, 0, 16},   {"memp32", 32, 31, 0, 16}, {"memp31", 31, 30, 0, 16},
+           {"memp33", 33, 32, 0, 15}, {"memw", 32, 31, 0, 16},   {NULL, 0, 0, 0, 0}};
+    struct params* value = values;
+    while (value->name) {
+        _mem_check(value->name, value->size, value->left, value->right, value->words);
+        value++;
+    }
 }
 
 int mon_check() {
     // Callback from initial block in monitor
-    if (int status = _mon_check_memory()) return status;
+    _mon_check_memory();
-    return 0;  // Ok
+    return errors;
 }
 
 //======================================================================

test_regress/t/t_vpi_memory.v

@@ -25,31 +25,42 @@ extern "C" int mon_check();
 
    input clk;
 
+   typedef logic [31:0] word_t;
    reg [31:0] mem0 [16:1] /*verilator public_flat_rw @(posedge clk) */;
+   reg [16:1] [31:0] memp32 /*verilator public_flat_rw @(posedge clk) */;
+   reg [16:1] [30:0] memp31 /*verilator public_flat_rw @(posedge clk) */;
+   reg [15:1] [32:0] memp33 /*verilator public_flat_rw @(posedge clk) */;
+   word_t [16:1] memw /*verilator public_flat_rw @(posedge clk) */;
    integer 	  i, status;
 
+`define CHECK_MEM(mem, words) \
+      for (i = words; i > 0; i--) \
+	if (integer'(mem[i]) !== i) begin \
+          $write("%%Error: %s[%d] : GOT = %d  EXP = %d\n", `"mem`", i, mem[i], i); \
+	  status = -1; \
+        end
+
    // Test loop
    initial begin
 `ifdef VERILATOR
       status = $c32("mon_check()");
-`endif
+`else
-`ifdef IVERILOG
      status = $mon_check();
 `endif
 `ifndef USE_VPI_NOT_DPI
      status = mon_check();
 `endif
       if (status!=0) begin
-	 $write("%%Error: t_vpi_var.cpp:%0d: C Test failed\n", status);
+	 $write("%%Error: t_vpi_memory.cpp: C Test failed (rc=%0d)\n", status);
 	 $stop;
       end
-      for (i = 16; i > 0; i--)
+      `CHECK_MEM(mem0, 16)
-	if (mem0[i] !== i) begin
+      `CHECK_MEM(memp32, 16)
-          $write("%%Error: %d : GOT = %d  EXP = %d\n", i, mem0[i], i);
+      `CHECK_MEM(memp31, 16)
-	  status = 1;
+      `CHECK_MEM(memp33, 15)
-        end
+      `CHECK_MEM(memw, 16)
       if (status!=0) begin
-	 $write("%%Error: t_vpi_var.cpp:%0d: C Test failed\n", status);
+	 $write("%%Error: Verilog memory checks failed\n");
 	 $stop;
       end
       $write("*-* All Finished *-*\n");