From c258c7cf7eae765b3a889629d06721ab2fbf4eda Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Wed, 28 Feb 2024 13:40:19 +0000 Subject: [PATCH] Incorrect record field length in FST dump. Fixes #852 --- NEWS.md | 2 + src/rt/wave.c | 95 +++++++++++++++++++-------------- test/regress/gold/issue586.dump | 6 +-- test/regress/gold/issue852.dump | 13 +++++ test/regress/gold/wave11.dump | 9 ++++ test/regress/issue536.sh | 10 ---- test/regress/issue852.vhd | 31 +++++++++++ test/regress/testlist.txt | 6 ++- test/regress/wave11.vhd | 34 ++++++++++++ test/regress/wave4.sh | 10 ---- test/run_regr.c | 6 +++ 11 files changed, 156 insertions(+), 66 deletions(-) create mode 100644 test/regress/gold/issue852.dump create mode 100644 test/regress/gold/wave11.dump delete mode 100644 test/regress/issue536.sh create mode 100644 test/regress/issue852.vhd create mode 100644 test/regress/wave11.vhd delete mode 100644 test/regress/wave4.sh diff --git a/NEWS.md b/NEWS.md index 655569d8..c3ed0a76 100644 --- a/NEWS.md +++ b/NEWS.md @@ -16,6 +16,8 @@ - Updated to OSVVM 2023.09a and UVVM 2023.09.16 for `nvc --install`. - The `--exit-severity=` option now also controls which severity level results in a non-zero exit code (#850). +- Improvements to waveform dumping for signals with record types or + types with non-locally-static bounds (#851, #852). ## Version 1.11.3 - 2024-02-04 - Fixed incorrect effective value when a signal has multiple sources due diff --git a/src/rt/wave.c b/src/rt/wave.c index 55680637..6483db77 100644 --- a/src/rt/wave.c +++ b/src/rt/wave.c @@ -21,6 +21,7 @@ #include "diag.h" #include "fstapi.h" #include "hash.h" +#include "jit/jit-layout.h" #include "option.h" #include "rt/model.h" #include "rt/rt.h" @@ -418,6 +419,28 @@ static fst_type_t *fst_type_for(type_t type, const loc_t *loc) return NULL; } +static void *fst_get_ptr(wave_dumper_t *wd, rt_scope_t *scope, tree_t where) +{ + if (tree_kind(where) == T_FIELD_DECL) { + assert(scope->kind == SCOPE_SIGNAL); + + type_t rtype = tree_type(scope->where); + assert(type_is_record(rtype)); + assert(type_field(rtype, tree_pos(where)) == where); + + const jit_layout_t *l = signal_layout_of(rtype); + assert(l->nparts == type_fields(rtype)); + + const ptrdiff_t offset = l->parts[tree_pos(where)].offset; + return fst_get_ptr(wd, scope->parent, scope->where) + offset; + } + else { + assert(scope->kind == SCOPE_INSTANCE); + jit_handle_t handle = jit_lazy_compile(wd->jit, scope->name); + return jit_get_frame_var(wd->jit, handle, tree_ident(where)); + } +} + static void fst_get_array_range(wave_dumper_t *wd, type_t type, rt_signal_t *s, int64_t *msb, int64_t *lsb, int64_t *length) { @@ -430,21 +453,11 @@ static void fst_get_array_range(wave_dumper_t *wd, type_t type, rt_signal_t *s, } } - if (s->parent->kind == SCOPE_INSTANCE) { - jit_handle_t handle = jit_lazy_compile(wd->jit, s->parent->name); - void *base = jit_get_frame_var(wd->jit, handle, tree_ident(s->where)); - ffi_dim_t *dims = base + 2*sizeof(int64_t); + ffi_dim_t *dims = fst_get_ptr(wd, s->parent, s->where) + 2*sizeof(int64_t); - *lsb = ffi_array_right(dims[0].left, dims[0].length); - *msb = dims[0].left; - *length = ffi_array_length(dims[0].length); - } - else { - const int signal_w = s->shared.size / s->nexus.size; - *msb = signal_w - 1; - *lsb = 0; - *length = signal_w; - } + *lsb = ffi_array_right(dims[0].left, dims[0].length); + *msb = dims[0].left; + *length = ffi_array_length(dims[0].length); } static void fst_create_array_var(wave_dumper_t *wd, tree_t d, rt_signal_t *s, @@ -471,11 +484,35 @@ static void fst_create_array_var(wave_dumper_t *wd, tree_t d, rt_signal_t *s, fst_data_t *data; type_t elem = type_elem(type); - if (!type_is_enum(elem)) { - // Dumping memories and nested arrays can be slow - if (!opt_get_int(OPT_DUMP_ARRAYS)) + if (type_is_enum(elem)) { + fst_type_t *ft = fst_type_for(type, tree_loc(d)); + if (ft == NULL) return; + data = xcalloc(sizeof(fst_data_t) + sizeof(fstHandle)); + data->type = ft; + data->size = length * ft->size; + data->count = 1; + + data->handle[0] = fstWriterCreateVar2( + wd->fst_ctx, + ft->vartype, + dir, + data->size, + tb_get(tb), + 0, + type_pp(type), + FST_SVT_VHDL_SIGNAL, + ft->sdt); + + if (wd->gtkw != NULL) + fprintf(wd->gtkw->file, "%s.%s\n", tb_get(wd->gtkw->hier), tb_get(tb)); + } + else if (!opt_get_int(OPT_DUMP_ARRAYS)) + return; // Dumping memories and nested arrays can be slow + else if (type_is_record(elem)) + return; // Not yet supported + else { fst_type_t *ft = fst_type_for(elem, tree_loc(d)); if (ft == NULL) return; @@ -519,30 +556,6 @@ static void fst_create_array_var(wave_dumper_t *wd, tree_t d, rt_signal_t *s, fstWriterSetAttrEnd(wd->fst_ctx); } - else { - fst_type_t *ft = fst_type_for(type, tree_loc(d)); - if (ft == NULL) - return; - - data = xcalloc(sizeof(fst_data_t) + sizeof(fstHandle)); - data->type = ft; - data->size = length * ft->size; - data->count = 1; - - data->handle[0] = fstWriterCreateVar2( - wd->fst_ctx, - ft->vartype, - dir, - data->size, - tb_get(tb), - 0, - type_pp(type), - FST_SVT_VHDL_SIGNAL, - ft->sdt); - - if (wd->gtkw != NULL) - fprintf(wd->gtkw->file, "%s.%s\n", tb_get(wd->gtkw->hier), tb_get(tb)); - } assert(find_watch(&(s->nexus), fst_event_cb) == NULL); diff --git a/test/regress/gold/issue586.dump b/test/regress/gold/issue586.dump index c31c2e4f..009e584b 100644 --- a/test/regress/gold/issue586.dump +++ b/test/regress/gold/issue586.dump @@ -1,6 +1,6 @@ -#0 issue586.u2.r.x[4:0] 00000 -#0 issue586.u1.r.x[2:0] 000 +#0 issue586.u2.r.x[1:5] 00000 +#0 issue586.u1.r.x[1:3] 000 #0 issue586.r.r.x[1:5] 00000 #0 issue586.s[1:3] 000 #1000000 issue586.r.r.x[1:5] 00100 -#1000000 issue586.u2.r.x[4:0] 00100 +#1000000 issue586.u2.r.x[1:5] 00100 diff --git a/test/regress/gold/issue852.dump b/test/regress/gold/issue852.dump new file mode 100644 index 00000000..3344d006 --- /dev/null +++ b/test/regress/gold/issue852.dump @@ -0,0 +1,13 @@ +#0 issue852.mc.write_count 0000000000000000000000000000000000000000000000000000000000000000 +#0 issue852.mc.read_count 0000000000000000000000000000000000000000000000000000000000000000 +#0 issue852.mc.memory[7][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[6][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[5][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[4][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[3][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[2][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[1][31:0] 00000000000000000000000000000000 +#0 issue852.mc.memory[0][31:0] 00000000000000000000000000000000 +#0 issue852.mc.prefix[1:21] apb: mock completer: +#2000000 issue852.mc.memory[4][31:0] 11011110101011011011111011101111 +#5000000 issue852.mc.memory[7][31:0] 11001010111111101011101010111110 diff --git a/test/regress/gold/wave11.dump b/test/regress/gold/wave11.dump new file mode 100644 index 00000000..04aef258 --- /dev/null +++ b/test/regress/gold/wave11.dump @@ -0,0 +1,9 @@ +#0 wave11.b.r.f3.f2[1:3] 000 +#0 wave11.b.r.f3.f1 false +#0 wave11.b.r.f2[1:3] 000 +#0 wave11.b.r.f1 10000000000000000000000000000000 +#0 wave11.b.p1[1:3] 000 +#0 wave11.s[1:3] 000 +#1000000 wave11.b.r.f2[1:3] 100 +#2000000 wave11.b.r.f2[1:3] 000 +#3000000 wave11.b.r.f3.f2[1:3] 010 diff --git a/test/regress/issue536.sh b/test/regress/issue536.sh deleted file mode 100644 index 8dc4843b..00000000 --- a/test/regress/issue536.sh +++ /dev/null @@ -1,10 +0,0 @@ -set -xe - -pwd -which nvc -which fstdump - -nvc -a $TESTDIR/regress/issue536.vhd -e issue536 -r -w --dump-arrays - -fstdump issue536.fst > issue536.dump -diff -u $TESTDIR/regress/gold/issue536.dump issue536.dump diff --git a/test/regress/issue852.vhd b/test/regress/issue852.vhd new file mode 100644 index 00000000..b49f49b6 --- /dev/null +++ b/test/regress/issue852.vhd @@ -0,0 +1,31 @@ +entity issue852 is +end entity; + +architecture test of issue852 is + + type data_array_t is array (natural range <>) of bit_vector(31 downto 0); + + type mock_completer_t is record + -- Configuration elements + prefix : string; -- Optional prefix used in report messages. + -- Internal elements + memory : data_array_t; + -- Statistics elements + read_count : natural; -- Number of read transfers. + write_count : natural; -- Number of write transfers. + end record; + + function init (memory_size: natural; prefix: string := "apb: mock completer: ") return mock_completer_t is + variable mc : mock_completer_t(prefix(1 to prefix'length), memory(0 to memory_size - 1)); + begin + mc.prefix := prefix; + return mc; + end function; + + signal mc : mock_completer_t := init(memory_size => 8); +begin + + mc.memory(4) <= X"deadbeef" after 2 ns; + mc.memory(7) <= X"cafebabe" after 5 ns; + +end architecture; diff --git a/test/regress/testlist.txt b/test/regress/testlist.txt index df307800..8168f53b 100644 --- a/test/regress/testlist.txt +++ b/test/regress/testlist.txt @@ -478,7 +478,7 @@ ieee7 normal,2008 wave1 wave wave2 shell wave3 wave,gtkw,stop-time=1us -wave4 shell +wave4 wave,dump-arrays wave5 wave,gtkw record20 normal attr14 normal @@ -663,7 +663,7 @@ vests48 normal signal27 normal issue519 normal,2008 delay3 fail,gold -issue536 shell +issue536 wave,dump-arrays wait25 normal issue539 normal,2008 issue542 normal @@ -947,3 +947,5 @@ wait27 normal issue808 verilog,gold issue851 wave,2008 vlog8 verilog +issue852 wave,2019,dump-arrays +wave11 wave,2008,dump-arrays diff --git a/test/regress/wave11.vhd b/test/regress/wave11.vhd new file mode 100644 index 00000000..57dfc9b2 --- /dev/null +++ b/test/regress/wave11.vhd @@ -0,0 +1,34 @@ +entity wave11 is +end entity; + +architecture test of wave11 is + signal s : bit_vector(1 to 3); +begin + + b: block is + port ( p1 : bit_vector ); + port map ( s ); + + type t_rec1 is record + f1 : boolean; + f2 : p1'subtype; + end record; + + type t_rec1_array is array (natural range <>) of t_rec1; + + type t_rec2 is record + f1 : integer; + f2 : p1'subtype; + f3 : t_rec1; + f4 : t_rec1_array(0 to 2); -- Not dumped currently + end record; + + signal r : t_rec2; + begin + + r.f2(1) <= '1' after 1 ns, '0' after 2 ns; + r.f3.f2(2) <= '1' after 3 ns; + + end block; + +end architecture; diff --git a/test/regress/wave4.sh b/test/regress/wave4.sh deleted file mode 100644 index e3b94b36..00000000 --- a/test/regress/wave4.sh +++ /dev/null @@ -1,10 +0,0 @@ -set -xe - -pwd -which nvc -which fstdump - -nvc -a $TESTDIR/regress/wave4.vhd -e wave4 -r -w --dump-arrays - -fstdump wave4.fst > wave4.dump -diff -u $TESTDIR/regress/gold/wave4.dump wave4.dump diff --git a/test/run_regr.c b/test/run_regr.c index dd26af6a..f6a93353 100644 --- a/test/run_regr.c +++ b/test/run_regr.c @@ -96,6 +96,7 @@ #define F_EXPORT (1 << 23) #define F_SHUFFLE (1 << 24) #define F_NOTBSD (1 << 25) +#define F_ARRAYS (1 << 26) typedef struct test test_t; typedef struct param param_t; @@ -411,6 +412,8 @@ static bool parse_test_list(int argc, char **argv) test->flags |= F_SHUFFLE; else if (strcmp(opt, "no-collapse") == 0) test->flags |= F_NOCOLL; + else if (strcmp(opt, "dump-arrays") == 0) + test->flags |= F_ARRAYS; else if (strncmp(opt, "O", 1) == 0) { if (sscanf(opt + 1, "%u", &(test->olevel)) != 1) { fprintf(stderr, "Error on testlist line %d: invalid " @@ -926,6 +929,9 @@ static bool run_test(test_t *test) if (test->flags & F_WAVE) push_arg(&args, "-w"); + if (test->flags & F_ARRAYS) + push_arg(&args, "--dump-arrays"); + if (test->flags & F_GTKW) push_arg(&args, "-g"); -- 2.39.2