From 36d0c153e8d32df680753d34deabd8e19256c1be Mon Sep 17 00:00:00 2001 From: Marco Brambilla Date: Sat, 7 Nov 2026 15:46:22 -0800 Subject: [PATCH] Keep the time steps of a flushed block without value changes Problem: when fstWriterFlushContext() is called and no value changed since the last flush, the value changes written after it are read back at wrong times. The flush itself returns at once, since the block has no value changes, but fstWriterEmitTimeChange() then appends the current time to the time table as if a new table had been started. The reader takes that absolute time as one more time step, and every later change of the section is read back at a later time than it was written: a 1-bit variable changed at time 230 is read back at time 500. A simulator that flushes the file at regular intervals, as for $dumpflush, meets this as soon as one interval has no change. Solution: fstWriterEmitTimeChange() does not flush a block without value changes. The flush request is dropped and the time table goes on. tests/flush_without_changes.c asks for a flush every 1, 2 and 3 time steps of 1, 2 and 4 variables, so that some blocks have no change, and checks that every change reads back at its time and that the end time is the last time step. Signed-off-by: Marco Brambilla Co-Authored-By: Claude Opus 5.5 --- src/fstapi.c | 10 ++- tests/flush_without_changes.c | 114 ++++++++++++++++++++++++++++++++++ tests/meson.build | 1 + 3 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 tests/flush_without_changes.c diff --git a/src/fstapi.c b/src/fstapi.c index 3ee23e2..1bc9ac9 100644 --- a/src/fstapi.c +++ b/src/fstapi.c @@ -3228,9 +3228,13 @@ void fstWriterEmitTimeChange(fstWriterContext *xc, uint64_t tim) } else { if (fstWriterGetFlushContextPendingInternal(xc)) { xc->flush_context_pending = 0; - fstWriterFlushContextPrivate(xc); - xc->tchn_cnt++; - fstWriterVarint(xc->tchn_handle, xc->curtime); + /* a block without value changes is not flushed, so its time + * steps stay in the time table, which then goes on */ + if (xc->vchg_siz > 1) { + fstWriterFlushContextPrivate(xc); + xc->tchn_cnt++; + fstWriterVarint(xc->tchn_handle, xc->curtime); + } } } diff --git a/tests/flush_without_changes.c b/tests/flush_without_changes.c new file mode 100644 index 0000000..e6dc033 --- /dev/null +++ b/tests/flush_without_changes.c @@ -0,0 +1,114 @@ +#include +#include +#include +#include +#include +#include + +// A flush asked for (fstWriterFlushContext()) when nothing changed since the +// last flush: the time steps of that block must still be read back at their +// times. + +static uint64_t rnd_state = 88172645463325252ULL; +static uint64_t rnd(void) +{ + rnd_state ^= rnd_state << 13; + rnd_state ^= rnd_state >> 7; + rnd_state ^= rnd_state << 17; + return rnd_state; +} + +// A value change as one string, "time handle value", whose order is the order +// of time, then handle. +static char *change_text(uint64_t time, fstHandle handle, char value) +{ + return g_strdup_printf("%020" PRIu64 " %06u %c", time, handle, value); +} + +static int compare_text(gconstpointer a, gconstpointer b) +{ + return strcmp(*(char *const *)a, *(char *const *)b); +} + +static void read_change(void *user_data, + uint64_t time, + fstHandle handle, + const unsigned char *value) +{ + g_ptr_array_add((GPtrArray *)user_data, change_text(time, handle, (char)value[0])); +} + +// Writes `steps` time steps of `vars` 1-bit variables, each changing in about +// one step in four, with a flush asked for every `flush_every` steps, and +// checks that every change reads back at its time. +static void check(int vars, int flush_every, int steps) +{ + const char *name = "flush_without_changes.fst"; + GPtrArray *written = g_ptr_array_new_with_free_func(g_free); + GPtrArray *read = g_ptr_array_new_with_free_func(g_free); + fstHandle handles[4]; + char values[4]; + + g_assert_cmpint(vars, <=, 4); + fstWriterContext *writer = fstWriterCreate(name, 1); + g_assert_true(writer != NULL); + fstWriterSetScope(writer, FST_ST_VCD_MODULE, "top", NULL); + for (int i = 0; i < vars; i++) { + char var_name[8]; + snprintf(var_name, sizeof(var_name), "v%d", i); + handles[i] = fstWriterCreateVar(writer, FST_VT_VCD_REG, FST_VD_IMPLICIT, 1, var_name, 0); + } + fstWriterSetUpscope(writer); + + for (int step = 0; step < steps; step++) { + uint64_t time = (uint64_t)step * 10; + fstWriterEmitTimeChange(writer, time); + for (int i = 0; i < vars; i++) { + char value = (rnd() & 1) ? '1' : '0'; + if (step > 0 && (rnd() % 4 != 0 || value == values[i])) { + continue; + } + values[i] = value; + fstWriterEmitValueChange(writer, handles[i], &values[i]); + g_ptr_array_add(written, change_text(time, handles[i], value)); + } + if (step % flush_every == flush_every - 1) { + fstWriterFlushContext(writer); + } + } + fstWriterEmitTimeChange(writer, (uint64_t)steps * 10); + g_assert_cmpint(fstWriterClose2(writer), ==, 0); + + fstReaderContext *reader = fstReaderOpen(name); + g_assert_true(reader != NULL); + g_assert_cmpuint(fstReaderGetEndTime(reader), ==, (uint64_t)steps * 10); + fstReaderSetFacProcessMaskAll(reader); + g_assert_true(fstReaderIterBlocks(reader, read_change, read, NULL)); + fstReaderClose(reader); + + g_ptr_array_sort(written, compare_text); + g_ptr_array_sort(read, compare_text); + g_assert_cmpuint(read->len, ==, written->len); + for (guint i = 0; i < written->len; i++) { + g_assert_cmpstr(g_ptr_array_index(read, i), ==, g_ptr_array_index(written, i)); + } + + g_ptr_array_unref(read); + g_ptr_array_unref(written); + g_remove(name); +} + +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + + // One variable, a flush every time step: most blocks have no change. + check(1, 1, 400); + // Two variables, a flush every second time step. + check(2, 2, 400); + // Four variables, a flush every third time step: few blocks have no change. + check(4, 3, 400); + + return 0; +} diff --git a/tests/meson.build b/tests/meson.build index 7ef1df9..020441b 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -2,6 +2,7 @@ libfst_tests = [ 'attr_enum_value_type', 'chain_table_across_sections', 'empty_file', + 'flush_without_changes', 'mask_no_variables', 'packed_extent_wraps', 'rvat_section_boundary',