Fix emitting `this` (#5849)

Changes

@@ -24,6 +24,7 @@ Verilator 5.035 devel
 * Add used language to `--preproc-resolve` output (#5795). [Kamil Rakoczy, Antmicro Ltd.]
 * Add empty veriuser.h for legacy compatibility.
 * Optimize automatic splitting of some packed variables (#5843). [Geza Lore]
+* Fix foreach of associative array inside a constraint block (#5727) (#5841). [Yilou Wang]
 * Fix reset of automatic function variables (#5747). [Augustin Fabre]
 * Fix invalid code motion over branches (#5811) (#5814). [Geza Lore]
 * Fix sorting of wide SenItems (#5816). [Geza Lore]
@@ -37,6 +38,8 @@ Verilator 5.035 devel
 * Fix assignment pattern as function argument (#5839)
 * Fix checking built-in method arguments (#5839)
 * Fix splitting of packed ports with non-zero based ranges (#5842). [Geza Lore]
+* Fix NBA shared flag reuse (#5848). [Geza Lore]
+* Fix removal of callbacks no longer in current list (#5851) (#5852). [Gilberto Abram]
 
 
 Verilator 5.034 2025-02-24

docs/CONTRIBUTORS

@@ -65,6 +65,7 @@ Garrett Smith
 Geza Lore
 Gianfranco Costamagna
 Gijs Burghoorn
+Gilberto Abram
 Glen Gibb
 Gökçe Aydos
 Graham Rushton

docs/guide/contributing.rst

@@ -77,8 +77,12 @@ reduce a SystemVerilog design to the smallest possible reproducer.
 It can be used to automatically reduce a design with hundreds of thousands of
 lines to a minimal test case while preserving the bug-inducing behavior.
 
-Please refer to the `README
-<https://github.com/antmicro/sv-bugpoint/blob/main/README.md>`_ file for more
+With :vlopt:`--debug`, Verilator will write a *{prefix}*\ __inputs\ .vpp
+file which has all of the individual input files combined and
+pre-processed, this is often useful as the input design into `sv-bugpoint`.
+
+Please refer to `sv-bugpoint README
+<https://github.com/antmicro/sv-bugpoint/blob/main/README.md>`_ for more
 information on how to use `sv-bugpoint`.
 
 .. Contributing

include/verilated.cpp

@@ -56,6 +56,7 @@
 #include <cctype>
 #include <cerrno>
 #include <cstdlib>
+#include <iostream>
 #include <limits>
 #include <list>
 #include <sstream>
@@ -916,6 +917,20 @@ void _vl_vsformat(std::string& output, const std::string& format, va_list ap) VL
                 }
                 break;
             }
+            case 'p': {  // 'x' but parameter is string
+                const int lbits = va_arg(ap, int);
+                const std::string* const cstr = va_arg(ap, const std::string*);
+                std::ostringstream oss;
+                for (unsigned char c : *cstr) { oss << std::hex << static_cast<int>(c); }
+                std::string hex_str = oss.str();
+                if (width > 0 && widthSet) {
+                    hex_str = hex_str.size() > width
+                                  ? hex_str.substr(0, width)
+                                  : std::string(width - hex_str.size(), '0') + hex_str;
+                    output += hex_str;
+                }
+                break;
+            }
             default: {
                 // Deal with all read-and-print somethings
                 const int lbits = va_arg(ap, int);

include/verilated_vpi.cpp

@@ -873,6 +873,7 @@ class VerilatedVpiImp final {
     // All only medium-speed, so use singleton function
     // Callbacks that are past or at current timestamp
     std::array<VpioCbList, CB_ENUM_MAX_VALUE> m_cbCurrentLists;
+    VpioCbList m_cbCallList;  // List of callbacks currently being called by callCbs
     VpioFutureCbs m_futureCbs;  // Time based callbacks for future timestamps
     VpioFutureCbs m_nextCbs;  // cbNextSimTime callbacks
     std::list<VerilatedVpiPutHolder> m_inertialPuts;  // Pending vpi puts due to vpiInertialDelay
@@ -925,7 +926,13 @@ public:
         for (auto& ir : s().m_cbCurrentLists[reason]) {
             if (ir.id() == id) {
                 ir.invalidate();
-                return;  // Once found, it won't also be in m_futureCbs
+                return;  // Once found, it won't also be in m_cbCallList, m_futureCbs, or m_nextCbs
+            }
+        }
+        for (auto& ir : s().m_cbCallList) {
+            if (ir.id() == id) {
+                ir.invalidate();
+                return;  // Once found, it won't also be in m_futureCbs or m_nextCbs
             }
         }
         {  // Remove from cbFuture queue
@@ -985,10 +992,9 @@ public:
         moveFutureCbs();
         if (s().m_cbCurrentLists[reason].empty()) return false;
         // Iterate on old list, making new list empty, to prevent looping over newly added elements
-        VpioCbList cbObjList;
-        std::swap(s().m_cbCurrentLists[reason], cbObjList);
+        std::swap(s().m_cbCurrentLists[reason], s().m_cbCallList);
         bool called = false;
-        for (VerilatedVpiCbHolder& ihor : cbObjList) {
+        for (VerilatedVpiCbHolder& ihor : s().m_cbCallList) {
             // cbReasonRemove sets to nullptr, so we know on removal the old end() will still exist
             if (VL_LIKELY(!ihor.invalid())) {  // Not deleted earlier
                 VL_DEBUG_IF_PLI(VL_DBG_MSGF("- vpi: reason_callback reason=%d id=%" PRId64 "\n",
@@ -998,6 +1004,7 @@ public:
                 called = true;
             }
         }
+        s().m_cbCallList.clear();
         return called;
     }
     static bool callValueCbs() VL_MT_UNSAFE_ONE {

src/V3EmitCFunc.cpp

@@ -220,7 +220,7 @@ void EmitCFunc::displayEmit(AstNode* nodep, bool isScan) {
                 if (func != "") {
                     puts(func);
                 } else if (argp) {
-                    const bool addrof = isScan || (fmt == '@');
+                    const bool addrof = isScan || (fmt == '@') || (fmt == 'p');
                     if (addrof) puts("&(");
                     iterateConst(argp);
                     if (!addrof) emitDatap(argp);
@@ -371,6 +371,7 @@ void EmitCFunc::displayNode(AstNode* nodep, AstScopeName* scopenamep, const stri
             case 'o': displayArg(nodep, &elistp, isScan, vfmt, ignore, 'o'); break;
             case 'h':  // FALLTHRU
             case 'x': displayArg(nodep, &elistp, isScan, vfmt, ignore, 'x'); break;
+            case 'p': displayArg(nodep, &elistp, isScan, vfmt, ignore, 'p'); break;
             case 's': displayArg(nodep, &elistp, isScan, vfmt, ignore, 's'); break;
             case 'e': displayArg(nodep, &elistp, isScan, vfmt, ignore, 'e'); break;
             case 'f': displayArg(nodep, &elistp, isScan, vfmt, ignore, 'f'); break;

src/V3EmitV.cpp

@@ -634,6 +634,7 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst {
         iterateAndNextConstNull(nodep->fromp());
         puts(cvtToStr(nodep->declRange()));
     }
+    void visit(AstThisRef* nodep) override { puts("this"); }
     void visit(AstTypedef* nodep) override {
         putfs(nodep, "typedef ");
         iterateConstNull(nodep->subDTypep());

src/V3Randomize.cpp

@@ -737,7 +737,14 @@ class ConstraintExprVisitor final : public VNVisitor {
     void visit(AstAssocSel* nodep) override {
         if (editFormat(nodep)) return;
         FileLine* const fl = nodep->fileline();
-        if (VN_IS(nodep->bitp(), CvtPackString) && VN_IS(nodep->bitp()->dtypep(), BasicDType)) {
+        if (VN_IS(nodep->bitp(), VarRef) && VN_AS(nodep->bitp(), VarRef)->isString()) {
+            VNRelinker handle;
+            AstNodeExpr* const idxp
+                = new AstSFormatF{fl, "#x%32p", false, nodep->bitp()->unlinkFrBack(&handle)};
+            handle.relink(idxp);
+            editSMT(nodep, nodep->fromp(), idxp);
+        } else if (VN_IS(nodep->bitp(), CvtPackString)
+                   && VN_IS(nodep->bitp()->dtypep(), BasicDType)) {
             AstCvtPackString* const stringp = VN_AS(nodep->bitp(), CvtPackString);
             const size_t stringSize = VN_AS(stringp->lhsp(), Const)->width();
             if (stringSize > 128) {

test_regress/t/t_constraint_assoc_arr_basic.v

@@ -8,6 +8,7 @@ class constrained_associative_array_basic;
 
     rand int int_index_arr [int];
     rand int string_index_arr [string];
+    rand int string_index_arr_2 [string];
     /* verilator lint_off SIDEEFFECT */
     // Constraints for both arrays
     constraint int_index_constraints {
@@ -18,11 +19,15 @@ class constrained_associative_array_basic;
         string_index_arr["Bob"] inside {50, 60};
         string_index_arr["Charlie"] > 25;
     }
+    constraint string_index_2_constraints {
+        foreach (string_index_arr_2[i]) string_index_arr_2[i] > 10; // nodep->bitp() would be VARREF, instead of CVTPACKSTRING
+    }
 
     // Constructor to initialize arrays
     function new();
         int_index_arr = '{1: 0, 8: 0, 7: 0};
         string_index_arr = '{"Alice": 25, "Bob": 50, "Charlie": 45};
+        string_index_arr_2 = '{"key1": 15, "key2": 20, "key3": 30};
     endfunction
 
     // Function to check and display the arrays
@@ -35,6 +40,9 @@ class constrained_associative_array_basic;
                 (name == "Bob" && !(string_index_arr[name] inside {50, 60})) ||
                 (name == "Charlie" && string_index_arr[name] <= 25)) $stop;
         end
+        foreach (string_index_arr_2[i]) begin
+            if (string_index_arr_2[i] <= 10) $stop;
+        end
     endfunction
 
 endclass

test_regress/t/t_debug_emitv.out

@@ -331,6 +331,9 @@ package Vt_debug_emitv___024unit;
         int signed member;
         member = 'sh1;
         task method;
+            if ((this != this)) begin
+                $stop;
+            end
         endtask
         function new;
         endfunction

test_regress/t/t_debug_emitv.v

@@ -22,6 +22,7 @@ endpackage
 class Cls;
    int member = 1;
    function void method;
+      if (this != this) $stop;
    endfunction
 endclass
 

test_regress/t/t_vpi_time_cb_c.cpp

@@ -94,13 +94,27 @@ static int _time_cb2(p_cb_data cb_data) {
     return 0;
 }
 
+static vpiHandle callback_handles3[2] = {NULL, NULL};
+
+static int _time_cb3(p_cb_data cb_data) {
+    size_t cb_id = (size_t)cb_data->user_data;
+    size_t cb_id_other = cb_id ? 0 : 1;
+    TEST_VERBOSE_PRINTF("time_cb_3: %d\n", (int)cb_id);
+    TEST_CHECK_NZ(callback_handles3[cb_id]);
+    TEST_CHECK_NZ(callback_handles3[cb_id_other]);
+    vpi_remove_cb(callback_handles3[cb_id_other]);
+    callback_handles3[0] = callback_handles3[1] = NULL;
+    return 0;
+}
+
 extern "C" void dpii_init() {
     TEST_VERBOSE_PRINTF("-dpii_init()\n");
 
-    t_cb_data cb_data_n1, cb_data_n2;
+    t_cb_data cb_data_n1, cb_data_n2, cb_data_n3;
     bzero(&cb_data_n1, sizeof(cb_data_n1));
     bzero(&cb_data_n2, sizeof(cb_data_n2));
-    s_vpi_time t1, t2;
+    bzero(&cb_data_n2, sizeof(cb_data_n3));
+    s_vpi_time t1, t2, t3;
 
     cb_data_n1.reason = cbAfterDelay;
     t1.type = vpiSimTime;
@@ -118,6 +132,17 @@ extern "C" void dpii_init() {
     cb_data_n2.time = &t2;
     cb_data_n2.cb_rtn = _time_cb2;
     TestVpiHandle cb_data_n2_h = vpi_register_cb(&cb_data_n2);
+
+    cb_data_n3.reason = cbAfterDelay;
+    t3.type = vpiSimTime;
+    t3.high = 0;
+    t3.low = 5;
+    cb_data_n3.time = &t3;
+    cb_data_n3.cb_rtn = _time_cb3;
+    cb_data_n3.user_data = (PLI_BYTE8*)0;
+    callback_handles3[0] = vpi_register_cb(&cb_data_n3);
+    cb_data_n3.user_data = (PLI_BYTE8*)1;
+    callback_handles3[1] = vpi_register_cb(&cb_data_n3);
 }
 
 extern "C" void dpii_final() {