Skip to content

Commit

Permalink
Incorrect behaviour of resolution functions with nested records. Issue
Browse files Browse the repository at this point in the history
  • Loading branch information
nickg committed Aug 20, 2022
1 parent ccde636 commit a66d1fb
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 7 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
- Added support for `else` and `elsif` in generate statements (#510).
- Xilinx Vivado vendor libraries can now be compiled with `nvc --install
vivado`.
- Resolution functions with nested record types now behave correctly
(#516).

## Version 1.7.0 - 2022-08-07
- *Breaking change:* In-tree builds are no longer supported: use a
Expand Down
34 changes: 27 additions & 7 deletions src/rt/rtkern.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,18 @@ typedef enum {
SCOPE_SIGNAL,
} rt_scope_kind_t;

typedef enum {
SCOPE_F_RESOLVED = (1 << 0)
} rt_scope_flags_t;

typedef struct rt_scope_s {
rt_signal_t *signals;
rt_proc_t *procs;
rt_alias_t *aliases;
rt_scope_kind_t kind;
rt_scope_flags_t flags;
unsigned size; // For signal scopes
unsigned offset;
ident_t name;
tree_t where;
mptr_t privdata;
Expand Down Expand Up @@ -920,7 +926,7 @@ static void rt_copy_sub_signals(rt_scope_t *scope, void *buf, value_fn_t fn)
rt_copy_sub_signals(s, buf, fn);
}

static void rt_copy_sub_signal_sources(rt_scope_t *scope, void *buf)
static void rt_copy_sub_signal_sources(rt_scope_t *scope, void *buf, int stride)
{
assert(scope->kind == SCOPE_SIGNAL);

Expand All @@ -933,14 +939,14 @@ static void rt_copy_sub_signal_sources(rt_scope_t *scope, void *buf)
if (data == NULL)
continue;

memcpy(buf + s->shared.offset + (o++ * scope->size),
memcpy(buf + s->shared.offset + (o++ * stride),
data, n->size * n->width);
}
}
}

for (rt_scope_t *s = scope->child; s != NULL; s = s->chain)
rt_copy_sub_signal_sources(s, buf);
rt_copy_sub_signal_sources(s, buf, stride);
}

static void *rt_composite_signal(rt_signal_t *signal, size_t *psz, value_fn_t fn)
Expand Down Expand Up @@ -1240,6 +1246,10 @@ void __nvc_push_scope(DEBUG_LOCUS(locus), int32_t size)
s->size = size;
s->privdata = mptr_new(mspace, "push scope privdata");

type_t type = tree_type(where);
if (type_kind(type) == T_SUBTYPE && type_has_resolution(type))
s->flags |= SCOPE_F_RESOLVED;

active_scope->child = s;
active_scope = s;

Expand All @@ -1254,6 +1264,13 @@ void __nvc_pop_scope(void)
if (unlikely(active_scope->kind != SCOPE_SIGNAL))
fatal_trace("cannot pop non-signal scope");

int offset = INT_MAX;
for (rt_scope_t *s = active_scope->child; s; s = s->chain)
offset = MIN(offset, s->offset);
for (rt_signal_t *s = active_scope->signals; s; s = s->chain)
offset = MIN(offset, s->shared.offset);
active_scope->offset = offset;

active_scope = active_scope->parent;

for (signals_tail = &(active_scope->signals);
Expand Down Expand Up @@ -2504,22 +2521,25 @@ static void *rt_call_resolution(rt_nexus_t *nexus, res_memo_t *r, int nonnull)
else if (r->flags & R_COMPOSITE) {
// Call resolution function of composite type

rt_scope_t *scope = nexus->signal->parent;
while (scope->parent->kind == SCOPE_SIGNAL)
rt_scope_t *scope = nexus->signal->parent, *rscope = scope;
while (scope->parent->kind == SCOPE_SIGNAL) {
scope = scope->parent;
if (scope->flags & SCOPE_F_RESOLVED)
rscope = scope;
}

TRACE("resolved composite signal needs %d bytes", scope->size);

uint8_t *inputs = rt_tlab_alloc(nonnull * scope->size);
rt_copy_sub_signal_sources(scope, inputs);
rt_copy_sub_signal_sources(scope, inputs, scope->size);

void *resolved;
ffi_uarray_t u = { inputs, { { r->ileft, nonnull } } };
ffi_call(&(r->closure), &u, sizeof(u), &resolved, sizeof(resolved));

const ptrdiff_t noff =
nexus->resolved - (void *)nexus->signal->shared.data;
return resolved + nexus->signal->shared.offset + noff;
return resolved + nexus->signal->shared.offset + noff - rscope->offset;
}
else {
void *resolved = rt_tlab_alloc(nexus->width * nexus->size);
Expand Down
111 changes: 111 additions & 0 deletions test/regress/issue516.vhd
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity issue516 is
end entity;
architecture beh of issue516 is
type t_event_ctrl_unresolved is record
val : std_logic;
ack : std_logic;
end record;
type t_event_ctrl_drivers is array (natural range <> ) of t_event_ctrl_unresolved;
function resolved_event_ctrl(input_vector : t_event_ctrl_drivers) return t_event_ctrl_unresolved;
subtype t_event_ctrl is resolved_event_ctrl t_event_ctrl_unresolved;

type t_events_unresolved is record
flag1 : t_event_ctrl;
flag2 : t_event_ctrl;
end record;
type t_events_drivers is array (natural range <> ) of t_events_unresolved;
function resolved_events(input_vector : t_events_drivers) return t_events_unresolved;
subtype t_events is resolved_events t_events_unresolved;
function resolved_event_ctrl(
input_vector : t_event_ctrl_drivers
) return t_event_ctrl_unresolved
is
variable ret : t_event_ctrl_unresolved := (others => '0');
begin
if input_vector'length = 0 THEN
return ret;
else
for i in input_vector'range loop
if input_vector(i).val = '1' then
ret.val := '1';
end if;
if input_vector(i).ack = '1' then
ret.ack := '1';
end if;
end loop;
return ret;
end if;
end function resolved_event_ctrl;

function resolved_events(
input_vector : t_events_drivers
) return t_events_unresolved
is
variable ret : t_events_unresolved := (others => (others => '0'));
begin
if input_vector'length = 0 THEN
return ret;
else
for i in input_vector'range loop
ret.flag1 := resolved_event_ctrl(t_event_ctrl_drivers'(ret.flag1, input_vector(i).flag2));
ret.flag2 := resolved_event_ctrl(t_event_ctrl_drivers'(ret.flag2, input_vector(i).flag2));
end loop;
return ret;
end if;
end function resolved_events;
signal events : t_events;
signal passed : boolean;
begin
p_events : process
begin
events <= (others => (others => '0'));
loop
wait on events.flag2.val;
report "Got val" severity note;
if rising_edge(events.flag2.val) then
report "Sending ack" severity note;
events.flag2.ack <= '1' after 1 us, '0' after 1.1 us;
end if;
if falling_edge(events.flag2.val) then
report "Resetting ack" severity note;
events.flag2.ack <= '1', '0' after 1 ns;
end if;
end loop;
end process p_events;

process
procedure com2(signal flag : inout t_event_ctrl) is
begin
events.flag2.val <= '1';
report "Send val 1" severity note;
wait until events.flag2.ack = '1';
report "Got ack" severity note;
report "Send val 0" severity note;
events.flag2.val <= '1';
wait until events.flag2.ack = '0';
report "Got ack 0" severity note;
end procedure;
procedure com(signal flag_event : inout t_events) is
begin
com2(flag_event.flag2);
end procedure;
begin
wait for 1 us;
report "Running" severity note;
com(events);
passed <= true;
wait;
end process;

check: process is
begin
wait for 5 us;
assert passed;
wait;
end process;

end architecture beh;
1 change: 1 addition & 0 deletions test/regress/testlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -653,3 +653,4 @@ issue521 fail,gold
issue552 normal,2008
stdenv2 normal,2019
driver14 normal
issue516 normal

0 comments on commit a66d1fb

Please sign in to comment.