From 3c4de2884ae144e1b61e08145d368e12f0e9caab Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Thu, 28 Mar 2024 15:00:29 +0000 Subject: [PATCH] Fix some corner cases enabling/disabling VHPI callbacks --- src/vhpi/vhpi-model.c | 171 ++++++++++++++++++++---------------- test/regress/gold/vhpi1.txt | 2 +- test/regress/testlist.txt | 1 + test/regress/vhpi13.vhd | 10 +++ test/vhpi/Makemodule.am | 3 +- test/vhpi/vhpi13.c | 108 +++++++++++++++++++++++ test/vhpi/vhpi_test.c | 1 + test/vhpi/vhpi_test.h | 1 + 8 files changed, 218 insertions(+), 79 deletions(-) create mode 100644 test/regress/vhpi13.vhd create mode 100644 test/vhpi/vhpi13.c diff --git a/src/vhpi/vhpi-model.c b/src/vhpi/vhpi-model.c index cac0260b..610cde52 100644 --- a/src/vhpi/vhpi-model.c +++ b/src/vhpi/vhpi-model.c @@ -395,6 +395,7 @@ DEF_CLASS(forGenerate, vhpiForGenerateK, region.object); typedef struct { c_vhpiObject object; + vhpiHandleT pending; vhpiStateT State; vhpiEnumT Reason; vhpiCbDataT data; @@ -781,10 +782,11 @@ static const char *handle_pp(vhpiHandleT handle) tb_printf(tb, "%p:{", handle); - c_vhpiObject *obj = from_handle(handle); - if (obj == NULL) + handle_slot_t *slot = decode_handle(vhpi_context(), handle); + if (slot == NULL) tb_cat(tb, "INVALID"); else { + c_vhpiObject *obj = slot->obj; tb_cat(tb, vhpi_class_str(obj->kind)); c_abstractDecl *decl = is_abstractDecl(obj); @@ -1381,35 +1383,51 @@ static void vhpi_signal_event_cb(uint64_t now, rt_signal_t *signal, static void vhpi_global_cb(rt_model_t *m, void *user) { vhpiHandleT handle = user; + vhpi_context_t *c = vhpi_context(); - { - c_vhpiObject *obj = from_handle(handle); - if (obj == NULL) - return; + { + handle_slot_t *slot = decode_handle(c, handle); + if (slot == NULL) + return; - c_callback *cb = is_callback(obj); - if (cb == NULL) - return; + c_callback *cb = is_callback(slot->obj); + if (cb == NULL) + return; - if (cb->State != vhpiEnable) - return; + assert(cb->State != vhpiMature); + assert(handle == cb->pending); - (cb->data.cb_rtn)(&(cb->data)); + if (cb->State == vhpiEnable) + (cb->data.cb_rtn)(&(cb->data)); } // The handle may be invalidated by the user call { - c_vhpiObject *obj = from_handle(handle); - if (obj == NULL) + handle_slot_t *slot = decode_handle(c, handle); + if (slot == NULL) return; - c_callback *cb = is_callback(obj); + c_callback *cb = is_callback(slot->obj); assert(cb != NULL); - if (!vhpi_is_repetitive(cb->Reason)) { + switch (cb->Reason) { + case vhpiCbRepEndOfProcesses: + case vhpiCbRepLastKnownDeltaCycle: + case vhpiCbRepNextTimeStep: + case vhpiCbRepEndOfTimeStep: + case vhpiCbRepStartOfNextCycle: + model_set_global_cb(c->model, vhpi_get_rt_event(cb->Reason), + vhpi_global_cb, handle); + break; + + case vhpiCbValueChange: + break; + + default: cb->State = vhpiMature; drop_handle(handle); + break; } } } @@ -1669,9 +1687,16 @@ int vhpi_release_handle(vhpiHandleT handle) return 0; } -static int enable_cb(c_callback *cb) +DLLEXPORT +vhpiHandleT vhpi_register_cb(vhpiCbDataT *cb_data_p, int32_t flags) { - switch (cb->Reason) { + vhpi_clear_error(); + + VHPI_TRACE("cb_datap_p=%s flags=%x", cb_data_pp(cb_data_p), flags); + + rt_model_t *m = vhpi_context()->model; + + switch (cb_data_p->reason) { case vhpiCbRepEndOfProcesses: case vhpiCbRepLastKnownDeltaCycle: case vhpiCbRepNextTimeStep: @@ -1685,77 +1710,71 @@ static int enable_cb(c_callback *cb) case vhpiCbEndOfInitialization: case vhpiCbStartOfNextCycle: case vhpiCbRepStartOfNextCycle: - model_set_global_cb(vhpi_context()->model, vhpi_get_rt_event(cb->Reason), - vhpi_global_cb, handle_for(&(cb->object))); - return 0; + { + c_callback *cb = new_object(sizeof(c_callback), vhpiCallbackK); + cb->Reason = cb_data_p->reason; + cb->State = (flags & vhpiDisableCb) ? vhpiDisable : vhpiEnable; + cb->data = *cb_data_p; + cb->pending = handle_for(&(cb->object)); + + model_set_global_cb(m, vhpi_get_rt_event(cb->Reason), + vhpi_global_cb, cb->pending); + + return (flags & vhpiReturnCb) ? handle_for(&(cb->object)) : NULL; + } case vhpiCbAfterDelay: - model_set_timeout_cb(vhpi_context()->model, cb->when, - vhpi_global_cb, handle_for(&(cb->object))); - return 0; + { + if (cb_data_p->time == NULL) { + vhpi_error(vhpiError, NULL, "missing time for vhpiCbAfterDelay"); + return NULL; + } + + c_callback *cb = new_object(sizeof(c_callback), vhpiCallbackK); + cb->Reason = cb_data_p->reason; + cb->State = (flags & vhpiDisableCb) ? vhpiDisable : vhpiEnable; + cb->data = *cb_data_p; + cb->pending = handle_for(&(cb->object)); + + const uint64_t now = model_now(m, NULL); + cb->when = vhpi_time_to_native(cb_data_p->time) + now; + + model_set_timeout_cb(m, cb->when, vhpi_global_cb, cb->pending); + + return (flags & vhpiReturnCb) ? handle_for(&(cb->object)) : NULL; + } case vhpiCbValueChange: { - c_vhpiObject *obj = from_handle(cb->data.obj); + c_vhpiObject *obj = from_handle(cb_data_p->obj); if (obj == NULL) - return 1; + return NULL; c_objDecl *decl = cast_objDecl(obj); if (decl == NULL) - return 1; + return NULL; rt_signal_t *signal = vhpi_get_signal_objDecl(decl); if (signal == NULL) - return 1; - - vhpiHandleT handle = handle_for(&(cb->object)); - cb->watch = model_set_event_cb(vhpi_context()->model, signal, - vhpi_signal_event_cb, handle, false); - return 0; - } - - default: - vhpi_error(vhpiInternal, NULL, - "unsupported reason %d in vhpi_register_cb", cb->Reason); - return 1; - } -} - -DLLEXPORT -vhpiHandleT vhpi_register_cb(vhpiCbDataT *cb_data_p, int32_t flags) -{ - vhpi_clear_error(); + return NULL; - VHPI_TRACE("cb_datap_p=%s flags=%x", cb_data_pp(cb_data_p), flags); + c_callback *cb = new_object(sizeof(c_callback), vhpiCallbackK); + cb->Reason = cb_data_p->reason; + cb->State = (flags & vhpiDisableCb) ? vhpiDisable : vhpiEnable; + cb->data = *cb_data_p; + cb->pending = handle_for(&(cb->object)); - c_callback *cb = new_object(sizeof(c_callback), vhpiCallbackK); - cb->Reason = cb_data_p->reason; - cb->State = (flags & vhpiDisableCb) ? vhpiDisable : vhpiEnable; - cb->data = *cb_data_p; + cb->watch = model_set_event_cb(m, signal, vhpi_signal_event_cb, + cb->pending, false); - if (cb->Reason == vhpiCbAfterDelay) { - if (cb->data.time == NULL) { - vhpi_error(vhpiError, NULL, "missing time for vhpiCbAfterDelay"); - goto err; + return (flags & vhpiReturnCb) ? handle_for(&(cb->object)) : NULL; } - const uint64_t now = model_now(vhpi_context()->model, NULL); - cb->when = vhpi_time_to_native(cb->data.time) + now; - } - else if (cb->Reason == vhpiCbValueChange && cb->data.value) { - vhpi_error(vhpiInternal, NULL, - "values are not supported for Object callbacks"); - goto err; + default: + vhpi_error(vhpiInternal, NULL, "unsupported reason %s", + vhpi_cb_reason_str(cb_data_p->reason)); + return NULL; } - - if (!(flags & vhpiDisableCb) && enable_cb(cb)) - goto err; - - return (flags & vhpiReturnCb) ? handle_for(&(cb->object)) : NULL; - -err: - free(cb); - return NULL; } static bool disable_cb(c_callback *cb, vhpiHandleT handle) @@ -1805,10 +1824,11 @@ int vhpi_remove_cb(vhpiHandleT handle) return 1; if (cb->State == vhpiEnable) - disable_cb(cb, handle); + disable_cb(cb, cb->pending); cb->State = vhpiMature; + drop_handle(cb->pending); drop_handle(handle); return 0; } @@ -1834,7 +1854,6 @@ int vhpi_disable_cb(vhpiHandleT cb_obj) return 1; } - disable_cb(cb, cb_obj); cb->State = vhpiDisable; return 0; } @@ -1860,10 +1879,8 @@ int vhpi_enable_cb(vhpiHandleT cb_obj) return 1; } - int ret = enable_cb(cb); - if (!ret) - cb->State = vhpiEnable; - return ret; + cb->State = vhpiEnable; + return 0; } DLLEXPORT diff --git a/test/regress/gold/vhpi1.txt b/test/regress/gold/vhpi1.txt index 09fe74af..f1806482 100644 --- a/test/regress/gold/vhpi1.txt +++ b/test/regress/gold/vhpi1.txt @@ -57,8 +57,8 @@ VHPI printf start of sim callback! user data is 'some user data' VHPI printf after_5ns callback! 5ns+1: x=70 5000001fs+0: VHPI printf mutual callback! -5000001fs+0: VHPI printf deferred work callback! 5000001fs+0: VHPI printf mutual callback! +5000001fs+0: VHPI printf deferred work callback! 5000002fs+0: VHPI printf enabled callback! VHPI printf y value changed to 71 6ns+0: VHPI printf mutual callback! diff --git a/test/regress/testlist.txt b/test/regress/testlist.txt index 3cb13acb..22ffe113 100644 --- a/test/regress/testlist.txt +++ b/test/regress/testlist.txt @@ -958,3 +958,4 @@ driver22 fail,gold,2019 vhpi12 normal,vhpi issue862 normal,gold,2008,relaxed issue858 normal,2008 +vhpi13 normal,vhpi diff --git a/test/regress/vhpi13.vhd b/test/regress/vhpi13.vhd new file mode 100644 index 00000000..14d33a30 --- /dev/null +++ b/test/regress/vhpi13.vhd @@ -0,0 +1,10 @@ +entity vhpi13 is +end entity; + +architecture test of vhpi13 is + signal seq : natural; +begin + + seq <= 1 after 1 ns, 2 after 2 ns, 3 after 3 ns; + +end architecture; diff --git a/test/vhpi/Makemodule.am b/test/vhpi/Makemodule.am index 217da4ba..bceee7f3 100644 --- a/test/vhpi/Makemodule.am +++ b/test/vhpi/Makemodule.am @@ -17,7 +17,8 @@ lib_vhpi_test_so_SOURCES = \ test/vhpi/vhpi10.c \ test/vhpi/vhpi11.c \ test/vhpi/issue762.c \ - test/vhpi/vhpi12.c + test/vhpi/vhpi12.c \ + test/vhpi/vhpi13.c lib_vhpi_test_so_CFLAGS = $(PIC_FLAG) -I$(top_srcdir)/src/vhpi $(AM_CFLAGS) lib_vhpi_test_so_LDFLAGS = -shared $(VHPI_LDFLAGS) $(AM_LDFLAGS) diff --git a/test/vhpi/vhpi13.c b/test/vhpi/vhpi13.c new file mode 100644 index 00000000..fe5094ee --- /dev/null +++ b/test/vhpi/vhpi13.c @@ -0,0 +1,108 @@ +// +// Copyright (C) 2024 Nick Gasson +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// + +#include "vhpi_test.h" + +#include +#include + +static bool end_of_sim_called = false; +static int num_next_time_step = 0; +static vhpiHandleT never_call_handle_1 = NULL; +static vhpiHandleT never_call_handle_2 = NULL; + +static void rep_next_time_step(const vhpiCbDataT *cb_data) +{ + vhpi_printf("rep_next_time_step"); + num_next_time_step++; + + if (num_next_time_step == 2) { + vhpi_enable_cb(never_call_handle_1); + vhpi_release_handle(never_call_handle_1); + } +} + +static void start_of_sim(const vhpiCbDataT *cb_data) +{ + vhpi_printf("start of sim"); + + vhpi_disable_cb(never_call_handle_1); + vhpi_disable_cb(never_call_handle_2); +} + +static void end_of_sim(const vhpiCbDataT *cb_data) +{ + vhpi_printf("end of sim"); + end_of_sim_called = true; + + vhpi_printf("num_next_time_step = %d", num_next_time_step); + + fail_unless(num_next_time_step == 4); + + vhpi_remove_cb(never_call_handle_2); +} + +static void never_call(const vhpiCbDataT *cb_data) +{ + vhpi_printf("callback should never be called!"); + fail_if(1); +} + +static void check_end_of_sim_called(void) +{ + fail_unless(end_of_sim_called); +} + +void vhpi13_startup(void) +{ + vhpiCbDataT cb_data1 = { + .reason = vhpiCbStartOfSimulation, + .cb_rtn = start_of_sim, + }; + vhpi_register_cb(&cb_data1, 0); + check_error(); + + vhpiCbDataT cb_data2 = { + .reason = vhpiCbEndOfSimulation, + .cb_rtn = end_of_sim, + }; + vhpi_register_cb(&cb_data2, 0); + check_error(); + + vhpiCbDataT cb_data3 = { + .reason = vhpiCbRepNextTimeStep, + .cb_rtn = rep_next_time_step, + }; + vhpi_register_cb(&cb_data3, 0); + check_error(); + + vhpiCbDataT cb_data4 = { + .reason = vhpiCbNextTimeStep, + .cb_rtn = never_call, + }; + never_call_handle_1 = vhpi_register_cb(&cb_data4, vhpiReturnCb); + check_error(); + + vhpiCbDataT cb_data5 = { + .reason = vhpiCbRepNextTimeStep, + .cb_rtn = never_call, + }; + never_call_handle_2 = vhpi_register_cb(&cb_data5, vhpiReturnCb); + check_error(); + + atexit(check_end_of_sim_called); +} diff --git a/test/vhpi/vhpi_test.c b/test/vhpi/vhpi_test.c index 76aa6114..861a48ed 100644 --- a/test/vhpi/vhpi_test.c +++ b/test/vhpi/vhpi_test.c @@ -41,6 +41,7 @@ static const vhpi_test_t tests[] = { { "vhpi11", vhpi11_startup }, { "issue762", issue762_startup }, { "vhpi12", vhpi12_startup }, + { "vhpi13", vhpi13_startup }, { NULL, NULL }, }; diff --git a/test/vhpi/vhpi_test.h b/test/vhpi/vhpi_test.h index e16e37dd..65cd821e 100644 --- a/test/vhpi/vhpi_test.h +++ b/test/vhpi/vhpi_test.h @@ -40,6 +40,7 @@ void vhpi9_startup(void); void vhpi10_startup(void); void vhpi11_startup(void); void vhpi12_startup(void); +void vhpi13_startup(void); void issue744_startup(void); void issue762_startup(void); -- 2.39.2