Fix VPI cbValueChange for 1-bit select (#8063)

This commit is contained in:
Bartłomiej Chmiel 2026-08-10 13:52:18 +02:00 committed by GitHub
parent 5d888d2ac0
commit 121fc62aff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 364 additions and 28 deletions

View File

@ -539,7 +539,7 @@ public:
for (auto idx : index()) m_fullname += "[" + std::to_string(idx) + "]";
return m_fullname.c_str();
}
void* prevDatap() const { return m_prevDatap; }
uint8_t* prevDatap() const { return m_prevDatap; }
void* varDatap() const override { return m_varDatap; }
void createPrevDatap() {
if (VL_UNLIKELY(!m_prevDatap)) {
@ -1027,6 +1027,26 @@ struct VerilatedVpiTimedCbsCmp final {
class VerilatedVpiError;
void vl_vpi_put_word(const VerilatedVpioVar* vop, QData word, size_t bitCount, size_t addOffset);
// Information about how to access packed array data.
// If underlying type is multi-word (VLVT_WDATA), the packed element might straddle word
// boundaries, in which case m_maskHi != 0.
template <typename T>
struct VarAccessInfo final {
T* m_datap; // Typed pointer to packed array base address
size_t m_bitOffset; // Data start location (bit offset)
size_t m_wordOffset; // Data start location (word offset, VLVT_WDATA only)
T m_maskLo; // Access mask for m_datap[m_wordOffset]
T m_maskHi; // Access mask for m_datap[m_wordOffset + 1] (VLVT_WDATA only)
};
template <typename T>
VarAccessInfo<T> vl_vpi_var_access_info(const VerilatedVpioVarBase* vop, size_t bitCount,
size_t addOffset);
template <typename T>
T vl_vpi_get_word_gen(VarAccessInfo<T> info);
template <typename T>
void vl_vpi_put_word_gen(VarAccessInfo<T> info, T word);
class VerilatedVpiImp final {
enum { CB_ENUM_MAX_VALUE = cbAtEndOfSimTime + 1 }; // Maximum callback reason
using VpioCbList = std::list<VerilatedVpiCbHolder>;
@ -1169,6 +1189,72 @@ public:
s().m_cbCallList.clear();
return called;
}
template <typename T>
static bool valueDiffersFromPrev(VerilatedVpioVar* varop) {
VL_DEBUG_IF_PLI(VL_DBG_MSGF("- vpi: value_test %s v[0]=%d/%d %p %p size=%d\n",
varop->fullname(), *(static_cast<CData*>(varop->varDatap())),
*(varop->prevDatap()), varop->varDatap(), varop->prevDatap(),
varop->entSize()););
if (varop->bitSize() == 1) {
T* const prevDatap = reinterpret_cast<T*>(
varop->prevDatap()); // Was malloced when we added the callback
const VarAccessInfo<T> currInfo
= vl_vpi_var_access_info<T>(varop, varop->bitSize(), 0);
VarAccessInfo<T> prevInfo = currInfo;
prevInfo.m_datap = prevDatap;
return vl_vpi_get_word_gen(currInfo) != vl_vpi_get_word_gen(prevInfo);
}
return std::memcmp(varop->prevDatap(), varop->varDatap(), varop->entSize()) != 0;
}
static bool valueDiffersFromPrev(VerilatedVpioVar* varop) {
switch (varop->varp()->vltype()) {
case VLVT_UINT8: return valueDiffersFromPrev<CData>(varop);
case VLVT_UINT16: return valueDiffersFromPrev<SData>(varop);
case VLVT_UINT32: return valueDiffersFromPrev<IData>(varop);
case VLVT_UINT64: return valueDiffersFromPrev<QData>(varop);
case VLVT_WDATA:
return valueDiffersFromPrev<EData>(varop);
// LCOV_EXCL_START - Would require earlier type check to not catch that
default:
const std::string msg
= "Unsupported type (" + std::to_string(varop->varp()->vltype()) + ")";
VL_FATAL_MT(__FILE__, __LINE__, "", msg.c_str());
return true;
// LCOV_EXCL_STOP
}
}
template <typename T>
static void updatePrev(const VerilatedVpioVar* const varop) {
if (varop->bitSize() == 1) {
const VarAccessInfo<T> currInfo
= vl_vpi_var_access_info<T>(varop, varop->bitSize(), 0);
VarAccessInfo<T> prevInfo = currInfo;
T* const prevDatap = reinterpret_cast<T*>(varop->prevDatap());
prevInfo.m_datap = prevDatap;
const T currWord = vl_vpi_get_word_gen(currInfo);
vl_vpi_put_word_gen(prevInfo, currWord);
assert(std::memcmp(varop->prevDatap(), varop->varDatap(), varop->entSize()) == 0);
} else {
std::memcpy(varop->prevDatap(), varop->varDatap(), varop->entSize());
}
}
static void updatePrev(const VerilatedVpioVar* const varop) {
switch (varop->varp()->vltype()) {
case VLVT_UINT8: updatePrev<CData>(varop); break;
case VLVT_UINT16: updatePrev<SData>(varop); break;
case VLVT_UINT32: updatePrev<IData>(varop); break;
case VLVT_UINT64: updatePrev<QData>(varop); break;
case VLVT_WDATA:
updatePrev<EData>(varop);
break;
// LCOV_EXCL_START - Would require earlier type check to not catch that
default:
const std::string msg
= "Unsupported type (" + std::to_string(varop->varp()->vltype()) + ")";
VL_FATAL_MT(__FILE__, __LINE__, "", msg.c_str());
// LCOV_EXCL_STOP
}
}
static bool callValueCbs() VL_MT_UNSAFE_ONE {
assertOneCheck();
VpioCbList& cbObjList = s().m_cbCurrentLists[cbValueChange];
@ -1187,15 +1273,10 @@ public:
VerilatedVpiCbHolder& ho = *it++;
VerilatedVpioVar* const varop
= reinterpret_cast<VerilatedVpioVar*>(ho.cb_datap()->obj);
void* const newDatap = varop->varDatap();
void* const prevDatap = varop->prevDatap(); // Was malloced when we added the callback
VL_DEBUG_IF_PLI(VL_DBG_MSGF("- vpi: value_test %s v[0]=%d/%d %p %p\n",
varop->fullname(), *(static_cast<CData*>(newDatap)),
*(static_cast<CData*>(prevDatap)), newDatap, prevDatap););
if (std::memcmp(prevDatap, newDatap, varop->entSize()) != 0) {
if (valueDiffersFromPrev(varop)) {
VL_DEBUG_IF_PLI(VL_DBG_MSGF("- vpi: value_callback %" PRId64 " %s v[0]=%d\n",
ho.id(), varop->fullname(),
*(static_cast<CData*>(newDatap))););
*(static_cast<CData*>(varop->varDatap()))););
update.insert(varop);
vpi_get_value(ho.cb_datap()->obj, ho.cb_datap()->value);
(ho.cb_rtnp())(ho.cb_datap());
@ -1203,9 +1284,7 @@ public:
}
if (was_last) break;
}
for (const VerilatedVpioVar* const ip : update) {
std::memcpy(ip->prevDatap(), ip->varDatap(), ip->entSize());
}
for (const VerilatedVpioVar* const varop : update) updatePrev(varop);
return called;
}
static void dumpCbs() VL_MT_UNSAFE_ONE;
@ -3195,18 +3274,6 @@ static void vl_strprintf(std::string& buffer, char const* fmt, ...) {
va_end(args);
}
// Information about how to access packed array data.
// If underlying type is multi-word (VLVT_WDATA), the packed element might straddle word
// boundaries, in which case m_maskHi != 0.
template <typename T>
struct VarAccessInfo final {
T* m_datap; // Typed pointer to packed array base address
size_t m_bitOffset; // Data start location (bit offset)
size_t m_wordOffset; // Data start location (word offset, VLVT_WDATA only)
T m_maskLo; // Access mask for m_datap[m_wordOffset]
T m_maskHi; // Access mask for m_datap[m_wordOffset + 1] (VLVT_WDATA only)
};
template <typename T>
VarAccessInfo<T> vl_vpi_var_access_info(const VerilatedVpioVarBase* vop, size_t bitCount,
size_t addOffset) {
@ -3262,21 +3329,25 @@ VarAccessInfo<T> vl_vpi_var_access_info(const VerilatedVpioVarBase* vop, size_t
}
template <typename T>
T vl_vpi_get_word_gen(const VerilatedVpioVarBase* vop, size_t bitCount, size_t addOffset) {
T vl_vpi_get_word_gen(VarAccessInfo<T> info) {
const size_t wordBits = sizeof(T) * 8;
const VarAccessInfo<T> info = vl_vpi_var_access_info<T>(vop, bitCount, addOffset);
if (info.m_maskHi)
if (info.m_maskHi) {
return ((info.m_datap[info.m_wordOffset] & info.m_maskLo) >> info.m_bitOffset)
| ((info.m_datap[info.m_wordOffset + 1] & info.m_maskHi)
<< (wordBits - info.m_bitOffset));
}
return (info.m_datap[info.m_wordOffset] & info.m_maskLo) >> info.m_bitOffset;
}
template <typename T>
void vl_vpi_put_word_gen(const VerilatedVpioVar* vop, T word, size_t bitCount, size_t addOffset) {
const size_t wordBits = sizeof(T) * 8;
T vl_vpi_get_word_gen(const VerilatedVpioVarBase* vop, size_t bitCount, size_t addOffset) {
const VarAccessInfo<T> info = vl_vpi_var_access_info<T>(vop, bitCount, addOffset);
return vl_vpi_get_word_gen(info);
}
template <typename T>
void vl_vpi_put_word_gen(VarAccessInfo<T> info, T word) {
const size_t wordBits = sizeof(T) * 8;
if (info.m_maskHi) {
info.m_datap[info.m_wordOffset + 1]
= (info.m_datap[info.m_wordOffset + 1] & ~info.m_maskHi)
@ -3287,6 +3358,12 @@ void vl_vpi_put_word_gen(const VerilatedVpioVar* vop, T word, size_t bitCount, s
| ((word << info.m_bitOffset) & info.m_maskLo);
}
template <typename T>
void vl_vpi_put_word_gen(const VerilatedVpioVar* vop, T word, size_t bitCount, size_t addOffset) {
const VarAccessInfo<T> info = vl_vpi_var_access_info<T>(vop, bitCount, addOffset);
vl_vpi_put_word_gen(info, word);
}
// bitCount: maximum number of bits to read, will stop earlier if it reaches the var bounds
// addOffset: additional read bitoffset
QData vl_vpi_get_word(const VerilatedVpioVarBase* vop, size_t bitCount, size_t addOffset) {

View File

@ -66,6 +66,8 @@ unsigned int callback_count_half = 0;
unsigned int callback_count_quad = 0;
unsigned int callback_count_strs = 0;
unsigned int callback_count_strs_max = 500;
unsigned int callback_count_3d = 0;
unsigned int callback_count_endian = 0;
//======================================================================
@ -237,6 +239,242 @@ int _mon_check_value_callbacks() {
return 0;
}
int _value_callback_3d(p_cb_data cb_data) {
++callback_count_3d;
return 0;
}
int _value_callback_big_endian(p_cb_data cb_data) {
static bool called = false;
if (called) {
printf("%%Error: callback should be called only once\n");
exit(-1);
}
called = true;
++callback_count_endian;
return 0;
}
int _value_callback_little_endian(p_cb_data cb_data) {
static bool called = false;
if (called) {
printf("%%Error: callback should be called only once\n");
exit(-1);
}
called = true;
++callback_count_endian;
return 0;
}
int _mon_check_value_callbacks_array() {
t_cb_data cb_data{};
cb_data.reason = cbValueChange;
cb_data.time = nullptr;
{
TestVpiHandle vh = VPI_HANDLE("multi_packed_bit");
CHECK_RESULT_NZ(vh);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 0x123456;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(vh, &put_val, &time_s, vpiNoDelay);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_bit[1][2][0]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
// Put value between other values and check if value change applies only here
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_bit[1][2][1]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_3d;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 0;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(vh1, &put_val, &time_s, vpiNoDelay);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_bit[1][2][2]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_bit[1][2]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_bit[0][0][0]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_bit[0][0]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{ // Set a single bit and hit it with value change, big endian
TestVpiHandle vh = VPI_HANDLE("multi_packed_endian");
CHECK_RESULT_NZ(vh);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 0b000100;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(vh, &put_val, &time_s, vpiNoDelay);
for (int i = -1; i <= 1; ++i) {
TestVpiHandle i_handle = vpi_handle_by_index(vh, i);
CHECK_RESULT_NZ(i_handle);
for (int j = 1; j <= 2; ++j) {
TestVpiHandle i_j_handle = vpi_handle_by_index(i_handle, j);
CHECK_RESULT_NZ(i_j_handle);
cb_data.obj = i_j_handle;
if (i == 0 && j == 1) {
cb_data.cb_rtn = _value_callback_big_endian;
} else {
cb_data.cb_rtn = _value_callback_never;
}
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 0;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(i_j_handle, &put_val, &time_s, vpiNoDelay);
}
}
}
{ // Set a single bit and hit it with value change, little endian
TestVpiHandle vh = VPI_HANDLE("multi_packed_little_endian");
CHECK_RESULT_NZ(vh);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 0b001000;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(vh, &put_val, &time_s, vpiNoDelay);
for (int i = -1; i <= 1; ++i) {
TestVpiHandle i_handle = vpi_handle_by_index(vh, i);
CHECK_RESULT_NZ(i_handle);
for (int j = 1; j <= 2; ++j) {
TestVpiHandle i_j_handle = vpi_handle_by_index(i_handle, j);
CHECK_RESULT_NZ(i_j_handle);
cb_data.obj = i_j_handle;
if (i == 0 && j == 1) {
cb_data.cb_rtn = _value_callback_little_endian;
} else {
cb_data.cb_rtn = _value_callback_never;
}
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 0;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(i_j_handle, &put_val, &time_s, vpiNoDelay);
}
}
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_unpacked_bit[1][2][3]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_unpacked_bit[1][2]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_short[1][2]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_short[1][0]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_3d;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 1;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(vh1, &put_val, &time_s, vpiNoDelay);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_wide[1][2]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_never;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
}
{
TestVpiHandle vh1 = VPI_HANDLE("multi_packed_wide[1][0]");
CHECK_RESULT_NZ(vh1);
cb_data.obj = vh1;
cb_data.cb_rtn = _value_callback_3d;
TestVpiHandle callback_h = vpi_register_cb(&cb_data);
CHECK_RESULT_NZ(callback_h);
s_vpi_value put_val;
put_val.format = vpiIntVal;
put_val.value.integer = 1;
s_vpi_time time_s = {vpiSimTime, 0, 0, 0.0};
vpi_put_value(vh1, &put_val, &time_s, vpiNoDelay);
}
return 0;
}
int _mon_check_big() {
#ifdef VERILATOR
s_vpi_value v;
@ -2209,6 +2447,7 @@ extern "C" int mon_check() {
if (int status = _mon_check_mcd()) return status;
if (int status = _mon_check_callbacks()) return status;
if (int status = _mon_check_value_callbacks()) return status;
if (int status = _mon_check_value_callbacks_array()) return status;
if (int status = _mon_check_var()) return status;
if (int status = _mon_check_rev()) return status;
if (int status = _mon_check_varlist()) return status;
@ -2311,6 +2550,8 @@ int main(int argc, char** argv) {
CHECK_RESULT(callback_count_half, 250);
CHECK_RESULT(callback_count_quad, 2);
CHECK_RESULT(callback_count_strs, callback_count_strs_max);
CHECK_RESULT(callback_count_3d, 3);
CHECK_RESULT(callback_count_endian, 2);
VerilatedVpi::clearEvalNeeded();
if (VerilatedVpi::evalNeeded()) {
vl_fatal(FILENM, __LINE__, "main", "%Error: Unexpected VPI dirty state");

View File

@ -52,6 +52,12 @@ extern "C" int mon_check();
reg [0:95] mem_3d[0:1][1:0][0:1] /*verilator public_flat_rw */; // Mixed: asc, desc, asc
reg [0:15][0:3][7:0] multi_packed[2:0] /*verilator public_flat_rw */;
reg [0:1][0:3][0:2] multi_packed_bit /*verilator public_flat_rw */;
reg [1:-1][2:1] multi_packed_endian /*verilator public_flat_rw */;
reg [-1:1][1:2] multi_packed_little_endian /*verilator public_flat_rw */;
reg [0:1][0:7] multi_packed_short /*verilator public_flat_rw */;
reg [0:1][0:127] multi_packed_wide /*verilator public_flat_rw */;
reg multi_unpacked_bit[0:1][0:3][0:7] /*verilator public_flat_rw */;
reg [8:-7] [3:-4] negative_multi_packed[0:-2] /*verilator public_flat_rw */;
// verilator lint_on ASCRANGE
reg unpacked_only[7:0];

View File

@ -66,6 +66,12 @@ extern "C" int mon_check();
// Signal with multiple packed dimensions
reg [0:15][0:3][7:0] multi_packed[2:0];
reg [0:1][0:3][0:2] multi_packed_bit /*verilator public_flat_rw */;
reg [1:-1][2:1] multi_packed_endian /*verilator public_flat_rw */;
reg [-1:1][1:2] multi_packed_little_endian /*verilator public_flat_rw */;
reg [0:1][0:7] multi_packed_short /*verilator public_flat_rw */;
reg [0:1][0:127] multi_packed_wide /*verilator public_flat_rw */;
reg multi_unpacked_bit[0:1][0:3][0:7] /*verilator public_flat_rw */;
reg [8:-7] [3:-4] negative_multi_packed[0:-2];
// verilator lint_on ASCRANGE
reg unpacked_only[7:0];

View File

@ -51,6 +51,12 @@ extern "C" int mon_check();
// Signal with multiple packed dimensions
reg [0:15][0:3][7:0] multi_packed[2:0];
reg [0:1][0:3][0:2] multi_packed_bit /*verilator public_flat_rw */;
reg [1:-1][2:1] multi_packed_endian /*verilator public_flat_rw */;
reg [-1:1][1:2] multi_packed_little_endian /*verilator public_flat_rw */;
reg [0:1][0:7] multi_packed_short /*verilator public_flat_rw */;
reg [0:1][0:127] multi_packed_wide /*verilator public_flat_rw */;
reg multi_unpacked_bit[0:1][0:3][0:7] /*verilator public_flat_rw */;
reg [8:-7] [3:-4] negative_multi_packed[0:-2];
// verilator lint_on ASCRANGE
reg unpacked_only[7:0];