diff --git a/include/verilated_vpi.cpp b/include/verilated_vpi.cpp index 6d3a3eaf7..795f367ae 100644 --- a/include/verilated_vpi.cpp +++ b/include/verilated_vpi.cpp @@ -2582,6 +2582,19 @@ vpiHandle vpi_put_value(vpiHandle object, p_vpi_value valuep, p_vpi_time /*time_ } PLI_INT32 delay_mode = flags & 0xfff; if (const VerilatedVpioVar* const vop = VerilatedVpioVar::castp(object)) { + VL_DEBUG_IF_PLI( + VL_DBG_MSGF("- vpi: vpi_put_value name=%s fmt=%d vali=%d\n", vop->fullname(), + valuep->format, valuep->value.integer); + VL_DBG_MSGF("- vpi: varp=%p putatp=%p\n", vop->varp()->datap(), vop->varDatap());); + + if (VL_UNLIKELY(!vop->varp()->isPublicRW())) { + VL_VPI_ERROR_(__FILE__, __LINE__, + "vpi_put_value was used on signal marked read-only," + " use public_flat_rw instead for %s : %s", + vop->fullname(), vop->scopep()->defname()); + return nullptr; + } + if (!vl_check_format(vop->varp(), valuep, vop->fullname(), false)) return nullptr; if (delay_mode == vpiInertialDelay) { if (!VerilatedVpiPutHolder::canInertialDelay(valuep)) { VL_VPI_WARNING_( @@ -2593,20 +2606,7 @@ vpiHandle vpi_put_value(vpiHandle object, p_vpi_value valuep, p_vpi_time /*time_ VerilatedVpiImp::inertialDelay(vop, valuep); return object; } - VL_DEBUG_IF_PLI( - VL_DBG_MSGF("- vpi: vpi_put_value name=%s fmt=%d vali=%d\n", vop->fullname(), - valuep->format, valuep->value.integer); - VL_DBG_MSGF("- vpi: varp=%p putatp=%p\n", vop->varp()->datap(), vop->varDatap());); VerilatedVpiImp::evalNeeded(true); - - if (VL_UNLIKELY(!vop->varp()->isPublicRW())) { - VL_VPI_WARNING_(__FILE__, __LINE__, - "Ignoring vpi_put_value to signal marked read-only," - " use public_flat_rw instead: %s", - vop->fullname()); - return nullptr; - } - if (!vl_check_format(vop->varp(), valuep, vop->fullname(), false)) return nullptr; if (valuep->format == vpiVectorVal) { if (VL_UNLIKELY(!valuep->value.vector)) return nullptr; if (vop->varp()->vltype() == VLVT_UINT8) { diff --git a/test_regress/t/t_vpi_const_type.cpp b/test_regress/t/t_vpi_const_type.cpp index ac8ebd2db..5efd97175 100644 --- a/test_regress/t/t_vpi_const_type.cpp +++ b/test_regress/t/t_vpi_const_type.cpp @@ -62,13 +62,32 @@ int mon_check() { const char* strConstTypeStr = vpi_get_str(vpiConstType, strHandle); CHECK_RESULT_CSTR(strConstTypeStr, "vpiStringConst") - TestVpiHandle sigHandle = vpi_handle_by_name((PLI_BYTE8*)"t.signal", NULL); + // t.signal_rd is not constant, and should error on a write + TestVpiHandle sigHandle = vpi_handle_by_name((PLI_BYTE8*)"t.signal_rd", NULL); CHECK_RESULT_NZ(sigHandle) PLI_INT32 sigConstType = vpi_get(vpiConstType, sigHandle); - // t.signal is not constant CHECK_RESULT(sigConstType, vpiUndefined) const char* sigConstTypeStr = vpi_get_str(vpiConstType, sigHandle); CHECK_RESULT_CSTR(sigConstTypeStr, "*undefined*") + // and should error on a write + s_vpi_value vpi_value; + vpi_value.format = vpiIntVal; + vpi_value.value.integer = 1; + vpi_put_value(sigHandle, &vpi_value, NULL, vpiNoDelay); + CHECK_RESULT(vpi_chk_error(nullptr), vpiError); + // and an intertial write + vpi_put_value(sigHandle, &vpi_value, NULL, vpiInertialDelay); + CHECK_RESULT(vpi_chk_error(nullptr), vpiError); + + // t.signal_rw is not constant + sigHandle = vpi_handle_by_name((PLI_BYTE8*)"t.signal_rw", NULL); + CHECK_RESULT_NZ(sigHandle) + sigConstType = vpi_get(vpiConstType, sigHandle); + CHECK_RESULT(sigConstType, vpiUndefined) + sigConstTypeStr = vpi_get_str(vpiConstType, sigHandle); + CHECK_RESULT_CSTR(sigConstTypeStr, "*undefined*") + + // left range of t.signal_rw TestVpiHandle leftHandle = vpi_handle(vpiLeftRange, sigHandle); CHECK_RESULT_NZ(leftHandle) PLI_INT32 leftConstType = vpi_get(vpiConstType, leftHandle); diff --git a/test_regress/t/t_vpi_const_type.v b/test_regress/t/t_vpi_const_type.v index ef76c8f35..2ae4e2f59 100644 --- a/test_regress/t/t_vpi_const_type.v +++ b/test_regress/t/t_vpi_const_type.v @@ -16,7 +16,8 @@ module t (/*AUTOARG*/ parameter time timeParam /*verilator public_flat_rd*/ = 0; parameter string strParam /*verilator public_flat_rd*/ = "abc"; - logic [31:0] signal /*verilator public_flat_rw*/; + logic [31:0] signal_rw /*verilator public_flat_rw*/; + logic [31:0] signal_rd /*verilator public_flat_rd*/; int status; diff --git a/test_regress/t/t_vpi_var.cpp b/test_regress/t/t_vpi_var.cpp index db721c306..99ab60f9f 100644 --- a/test_regress/t/t_vpi_var.cpp +++ b/test_regress/t/t_vpi_var.cpp @@ -702,6 +702,7 @@ int _mon_check_delayed() { CHECK_RESULT_Z(vpi_chk_error(nullptr)); // test unsupported vpiInertialDelay cases + // - should these also throw vpi errors? v.format = vpiStringVal; v.value.str = nullptr; vpi_put_value(vh, &v, &t, vpiInertialDelay); @@ -712,9 +713,11 @@ int _mon_check_delayed() { vpi_put_value(vh, &v, &t, vpiInertialDelay); CHECK_RESULT_NZ(vpi_chk_error(nullptr)); + // This format throws an error now + Verilated::fatalOnVpiError(false); v.format = vpiObjTypeVal; vpi_put_value(vh, &v, &t, vpiInertialDelay); - CHECK_RESULT_NZ(vpi_chk_error(nullptr)); + Verilated::fatalOnVpiError(true); return 0; } diff --git a/test_regress/t/t_vpi_var.v b/test_regress/t/t_vpi_var.v index d93196cc9..240694b86 100644 --- a/test_regress/t/t_vpi_var.v +++ b/test_regress/t/t_vpi_var.v @@ -39,7 +39,7 @@ extern "C" int mon_check(); reg [0:61] quads[2:3] /*verilator public_flat_rw @(posedge clk) */; // verilator lint_on ASCRANGE - reg [31:0] count /*verilator public_flat_rd */; + reg [31:0] count /*verilator public_flat */; reg [31:0] half_count /*verilator public_flat_rd */; reg [31:0] delayed /*verilator public_flat_rw */; reg [31:0] delayed_mem [16] /*verilator public_flat_rw */; diff --git a/test_regress/t/t_vpi_var2.v b/test_regress/t/t_vpi_var2.v index 0c0019ddf..93b720f3d 100644 --- a/test_regress/t/t_vpi_var2.v +++ b/test_regress/t/t_vpi_var2.v @@ -51,7 +51,7 @@ extern "C" int mon_check(); reg invisible1; // verilator lint_on ASCRANGE -/*verilator public_flat_rd_on*/ +/*verilator public_flat_on*/ reg [31:0] count; reg [31:0] half_count; /*verilator public_off*/