Skip to content

Fix and refactor conversion from double to uint64_t (fixes test br1029b on x86-64-v4) #1264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Fix and refactor conversion from double to uint64_t (fixes test br1029b on x86-64-v4) #1264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a912c5
Fix casting from double to uint64_t to avoid UB
weiren2 Jul 27, 2025
aa6ae73
Use rounding functions from <cmath> and put `vlg_round_to_u64` into a
weiren2 Jul 29, 2025
b35b74a
Refactor the logic for binary string value conversion.
weiren2 Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 23 additions & 41 deletions vvp/vpi_callback.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

# include "vpi_user.h"
# include "vpi_priv.h"
# include "vpi_utils.h"
# include "vvp_net.h"
# include "schedule.h"
# include "event.h"
Expand Down Expand Up @@ -949,23 +950,6 @@ void vvp_signal_value::get_signal_value(struct t_vpi_value*vp)
}
}

static double vlg_round(double rval)
{
if (rval >= 0.0) {
return floor(rval + 0.5);
} else {
return ceil(rval - 0.5);
}
}

static uint64_t vlg_round_to_u64(double rval)
{
// Directly casting a negative double to an unsigned integer types is
// undefined behavior and behaves differently on different architectures.
// Cast to signed integer first to get the behavior we want.
return static_cast<uint64_t>(static_cast<int64_t>(vlg_round(rval)));
}

static void real_signal_value(struct t_vpi_value*vp, double rval)
{
static const size_t RBUF_SIZE = 64 + 1;
Expand All @@ -984,7 +968,7 @@ static void real_signal_value(struct t_vpi_value*vp, double rval)
if (rval != rval || (rval && (rval == 0.5*rval))) {
rval = 0.0;
} else {
rval = vlg_round(rval);
rval = std::round(rval);
}
vp->value.integer = (PLI_INT32)rval;
break;
Expand All @@ -993,7 +977,7 @@ static void real_signal_value(struct t_vpi_value*vp, double rval)
if (std::isnan(rval))
snprintf(rbuf, RBUF_SIZE, "%s", "nan");
else
snprintf(rbuf, RBUF_SIZE, "%0.0f", vlg_round(rval));
snprintf(rbuf, RBUF_SIZE, "%0.0f", std::round(rval));
vp->value.str = rbuf;
break;

Expand All @@ -1003,28 +987,26 @@ static void real_signal_value(struct t_vpi_value*vp, double rval)
break;

case vpiBinStrVal: {
uint64_t val = vlg_round_to_u64(rval);
unsigned len = 0;

while (val > 0) {
len += 1;
val /= 2;
}

val = vlg_round_to_u64(rval);
for (unsigned idx = 0 ; idx < len ; idx += 1) {
rbuf[len-idx-1] = (val & 1)? '1' : '0';
val /= 2;
}

rbuf[len] = 0;
if (len == 0) {
rbuf[0] = '0';
rbuf[1] = 0;
}
vp->value.str = rbuf;
break;
}
const uint64_t val = vlg_round_to_u64(rval);
unsigned len = 0;

// Compute bit‑width; For val==0, this yields len==1
uint64_t tmp = val;
do {
len += 1;
tmp /= 2;
} while (tmp > 0);

tmp = val;
for (unsigned idx = 0; idx < len; idx += 1) {
rbuf[len - idx - 1] = (tmp & 1) ? '1' : '0';
tmp /= 2;
}

rbuf[len] = '\0';
vp->value.str = rbuf;
break;
}

case vpiSuppressVal:
break;
Expand Down
15 changes: 15 additions & 0 deletions vvp/vpi_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef IVL_vpi_utils_H
#define IVL_vpi_utils_H

#include <cmath>
#include <cstdint>

static inline uint64_t vlg_round_to_u64(double rval)
{
// Directly casting a negative double to an unsigned integer types is
// undefined behavior and behaves differently on different architectures.
// Cast to signed integer first to get the behavior we want.
return static_cast<uint64_t>(static_cast<int64_t>(std::llround(rval)));
}

#endif
59 changes: 24 additions & 35 deletions vvp/vpi_vthr_vector.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/

# include "vpi_priv.h"
# include "vpi_utils.h"
# include "vthread.h"
# include "config.h"
#ifdef CHECK_WITH_VALGRIND
Expand Down Expand Up @@ -87,15 +88,6 @@ static int vthr_word_get(int code, vpiHandle ref)
}
}

static double vlg_round(double rval)
{
if (rval >= 0.0) {
return floor(rval + 0.5);
} else {
return ceil(rval - 0.5);
}
}

static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp)
{
__vpiVThrWord*obj = dynamic_cast<__vpiVThrWord*>(ref);
Expand Down Expand Up @@ -127,7 +119,7 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp)
if (val != val || (val && (val == 0.5*val))) {
val = 0.0;
} else {
val = vlg_round(val);
val = std::round(val);
}
vp->value.integer = (PLI_INT32)val;
break;
Expand All @@ -136,43 +128,40 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp)
if (std::isnan(val))
snprintf(rbuf, RBUF_USE_SIZE, "%s", "nan");
else
snprintf(rbuf, RBUF_USE_SIZE, "%0.0f", vlg_round(val));
snprintf(rbuf, RBUF_USE_SIZE, "%0.0f", std::round(val));
vp->value.str = rbuf;
break;

case vpiOctStrVal:
snprintf(rbuf, RBUF_USE_SIZE, "%" PRIo64, (uint64_t)vlg_round(val));
snprintf(rbuf, RBUF_USE_SIZE, "%" PRIo64, vlg_round_to_u64(val));
vp->value.str = rbuf;
break;

case vpiHexStrVal:
snprintf(rbuf, RBUF_USE_SIZE, "%" PRIx64, (uint64_t)vlg_round(val));
snprintf(rbuf, RBUF_USE_SIZE, "%" PRIx64, vlg_round_to_u64(val));
vp->value.str = rbuf;
break;

case vpiBinStrVal: {
uint64_t vali = (uint64_t)vlg_round(val);
unsigned len = 0;

while (vali > 0) {
len += 1;
vali /= 2;
}

vali = (uint64_t)vlg_round(val);
for (unsigned idx = 0 ; idx < len ; idx += 1) {
rbuf[len-idx-1] = (vali & 1)? '1' : '0';
vali /= 2;
}

rbuf[len] = 0;
if (len == 0) {
rbuf[0] = '0';
rbuf[1] = 0;
}
vp->value.str = rbuf;
break;
}
const uint64_t vali = vlg_round_to_u64(val);
unsigned len = 0;

uint64_t tmp = vali;
do {
len += 1;
tmp /= 2;
} while (tmp > 0);

tmp = vali;
for (unsigned idx = 0; idx < len; idx += 1) {
rbuf[len - idx - 1] = (tmp & 1) ? '1' : '0';
tmp /= 2;
}

rbuf[len] = '\0';
vp->value.str = rbuf;
break;
}

default:
fprintf(stderr, "vvp error: get %d not supported "
Expand Down
7 changes: 2 additions & 5 deletions vvp/vthread.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
# include "ufunc.h"
# include "event.h"
# include "vpi_priv.h"
# include "vpi_utils.h"
# include "vvp_net_sig.h"
# include "vvp_cobject.h"
# include "vvp_darray.h"
Expand Down Expand Up @@ -2380,11 +2381,7 @@ bool of_CVT_SR(vthread_t thr, vvp_code_t cp)
bool of_CVT_UR(vthread_t thr, vvp_code_t cp)
{
double r = thr->pop_real();
if (r >= 0.0)
thr->words[cp->bit_idx[0]].w_uint = (uint64_t)floor(r+0.5);
else
thr->words[cp->bit_idx[0]].w_uint = (uint64_t)ceil(r-0.5);

thr->words[cp->bit_idx[0]].w_uint = vlg_round_to_u64(r);
return true;
}

Expand Down