From 61948022d4796a200670a9d6608ca49feda5eea3 Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Tue, 5 Mar 2024 21:09:21 +0000 Subject: [PATCH] Rewrite external names earlier during elaboration. Fixes #855 --- NEWS.md | 2 +- src/driver.c | 52 ++++++++-------- src/dump.c | 7 ++- src/elab.c | 121 ++++++++++++++++++++++++++++++++++---- src/inst.c | 2 + src/lower.c | 49 +++------------ src/parse.c | 2 + src/tree.h | 1 + test/elab/ename1.vhd | 2 - test/elab/ename3.vhd | 34 +++++++++++ test/elab/issue855.vhd | 42 +++++++++++++ test/regress/ename8.vhd | 32 ++++++++++ test/regress/testlist.txt | 1 + test/test_elab.c | 44 ++++++++++++-- 14 files changed, 305 insertions(+), 86 deletions(-) create mode 100644 test/elab/ename3.vhd create mode 100644 test/elab/issue855.vhd create mode 100644 test/regress/ename8.vhd diff --git a/NEWS.md b/NEWS.md index 95b29d4b..95328c5d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,7 +18,7 @@ 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). -- Resolved several other minor issues (#654, #854). +- Resolved several other minor issues (#654, #854, #855). ## Version 1.11.3 - 2024-02-04 - Fixed incorrect effective value when a signal has multiple sources due diff --git a/src/driver.c b/src/driver.c index edda9c86..1685ceba 100644 --- a/src/driver.c +++ b/src/driver.c @@ -79,27 +79,27 @@ static void drives_signal(driver_set_t *ds, tree_t where, tree_t expr, tree_t prefix = longest_static_prefix(expr); - tree_t ref = name_to_ref(prefix); - if (ref == NULL) - return; - - tree_t decl = tree_ref(ref); - - if (tree_kind(decl) == T_PARAM_DECL) { - // Assignment to procedure parameter: this is handled at the call - // site instead - return; - } - - driver_info_t *chain = hash_get(ds->map, where), *last = NULL; - for (; chain; last = chain, chain = chain->chain_proc) { - if (chain->where != where || chain->decl != decl) - continue; - else if (tree_kind(chain->prefix) == T_REF) - return; // Already driving full signal - else if (same_tree(prefix, chain->prefix)) + driver_info_t *last = NULL; + tree_t ref = name_to_ref(prefix), decl = NULL; + if (ref != NULL) { + if (tree_kind((decl = tree_ref(ref))) == T_PARAM_DECL) { + // Assignment to procedure parameter: this is handled at the + // call site instead return; + } + + driver_info_t *chain = hash_get(ds->map, where); + for (; chain; last = chain, chain = chain->chain_proc) { + if (chain->where != where || chain->decl != decl) + continue; + else if (tree_kind(chain->prefix) == T_REF) + return; // Already driving full signal + else if (same_tree(prefix, chain->prefix)) + return; + } } + else if (tree_kind(prefix) != T_EXTERNAL_NAME) + return; driver_info_t *di = alloc_driver_info(ds); di->chain_decl = NULL; @@ -115,12 +115,14 @@ static void drives_signal(driver_set_t *ds, tree_t where, tree_t expr, else last->chain_proc = di; - driver_info_t *decl_head = hash_get(ds->map, decl); - if (decl_head == NULL) - hash_put(ds->map, decl, di); - else { - for (; decl_head->chain_decl; decl_head = decl_head->chain_decl); - decl_head->chain_decl = di; + if (decl != NULL) { + driver_info_t *decl_head = hash_get(ds->map, decl); + if (decl_head == NULL) + hash_put(ds->map, decl, di); + else { + for (; decl_head->chain_decl; decl_head = decl_head->chain_decl); + decl_head->chain_decl = di; + } } } diff --git a/src/dump.c b/src/dump.c index 74fc6a48..8b39c4d3 100644 --- a/src/dump.c +++ b/src/dump.c @@ -1,5 +1,5 @@ // -// Copyright (C) 2011-2023 Nick Gasson +// Copyright (C) 2011-2024 Nick Gasson // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -177,6 +177,11 @@ static void dump_external_name(tree_t t) case PE_CARET: print_syntax("^."); break; + case PE_GENERATE: + print_syntax("%s(", istr(tree_ident(part))); + dump_expr(tree_value(part)); + print_syntax(")."); + break; } } print_syntax(" : "); diff --git a/src/elab.c b/src/elab.c index a9dc432b..bd3d8b9d 100644 --- a/src/elab.c +++ b/src/elab.c @@ -229,6 +229,104 @@ static void elab_find_config_roots(tree_t t, tree_list_t *roots) } } +static void elab_rewrite_external_pre(tree_t t, void *context) +{ + tree_list_t *stack = context; + + const tree_kind_t kind = tree_kind(t); + if (kind == T_BLOCK || kind == T_IF_GENERATE || kind == T_FOR_GENERATE) { + tree_t pe = tree_new(T_PATH_ELT); + tree_set_ident(pe, tree_ident(t)); + tree_set_loc(pe, tree_loc(t)); + + if (kind == T_FOR_GENERATE) { + tree_set_subkind(pe, PE_GENERATE); + tree_set_value(pe, make_ref(tree_decl(t, 0))); + } + else + tree_set_subkind(pe, PE_SIMPLE); + + APUSH(*stack, pe); + } +} + +static tree_t elab_rewrite_external_post(tree_t t, void *context) +{ + tree_list_t *stack = context; + assert(stack->count > 0); + + switch (tree_kind(t)) { + case T_BLOCK: + case T_IF_GENERATE: + case T_FOR_GENERATE: + APOP(*stack); + break; + case T_EXTERNAL_NAME: + if (tree_subkind(tree_part(t, 0)) == PE_RELATIVE) { + tree_t new = tree_new(T_EXTERNAL_NAME); + tree_set_loc(new, tree_loc(t)); + tree_set_class(new, tree_class(t)); + tree_set_type(new, tree_type(t)); + + const int nparts = tree_parts(t); + int carets = 0; + for (int i = 1; i < nparts; i++, carets++) { + if (tree_subkind(tree_part(t, i)) != PE_CARET) + break; + } + + if (carets >= stack->count - 1) + error_at(tree_loc(tree_part(t, 2 + carets - stack->count)), + "relative pathname has no containing " + "declarative region"); + else { + for (int i = 0; i < stack->count - carets; i++) + tree_add_part(new, stack->items[i]); + } + + for (int i = 1 + carets; i < nparts; i++) + tree_add_part(new, tree_part(t, i)); + + return new; + } + break; + default: + break; + } + + return t; +} + +static void elab_external_name_stack(tree_list_t *stack, const elab_ctx_t *ctx) +{ + if (ctx->out == ctx->root) { + tree_t root = tree_new(T_PATH_ELT); + tree_set_subkind(root, PE_ABSOLUTE); + tree_set_loc(root, tree_loc(ctx->out)); + + APUSH(*stack, root); + } + else { + elab_external_name_stack(stack, ctx->parent); + + if (tree_decls(ctx->out) > 0) { + tree_t hier = tree_decl(ctx->out, 0); + assert(tree_kind(hier) == T_HIER); + + // Skip over implicit block for component declaration + if (tree_subkind(hier) == T_COMPONENT) + return; + } + + tree_t pe = tree_new(T_PATH_ELT); + tree_set_subkind(pe, PE_SIMPLE); + tree_set_ident(pe, tree_ident(ctx->out)); + tree_set_loc(pe, tree_loc(ctx->out)); + + APUSH(*stack, pe); + } +} + static tree_t elab_copy(tree_t t, const elab_ctx_t *ctx) { tree_list_t roots = AINIT; @@ -254,6 +352,16 @@ static tree_t elab_copy(tree_t t, const elab_ctx_t *ctx) tree_t copy = roots.items[roots.count - 1]; ACLEAR(roots); + if (gflags & TREE_GF_EXTERNAL_NAME) { + tree_list_t stack = AINIT; + elab_external_name_stack(&stack, ctx); + tree_rewrite(copy, elab_rewrite_external_pre, + elab_rewrite_external_post, NULL, &stack); + ACLEAR(stack); + + gflags &= ~TREE_GF_EXTERNAL_NAME; + } + tree_set_global_flags(copy, gflags); return copy; } @@ -2183,8 +2291,9 @@ tree_t elab_external_name(tree_t name, tree_t root, ident_t *path) for (int i = 0; i < nparts; i++, where = next, next = NULL) { tree_t pe = tree_part(name, i); - ident_t id = NULL; + assert(tree_kind(pe) == T_PATH_ELT); + ident_t id = NULL; switch (tree_subkind(pe)) { case PE_ABSOLUTE: { @@ -2212,16 +2321,6 @@ tree_t elab_external_name(tree_t name, tree_t root, ident_t *path) id = ident_new(tb_get(tb)); } break; - case PE_RELATIVE: - next = where; - continue; - case PE_CARET: - if ((next = where) == NULL) { - error_at(tree_loc(pe), "relative pathname has no containing " - "declarative region"); - return NULL; - } - continue; default: error_at(tree_loc(pe), "sorry, this form of external name is not " "yet supported"); diff --git a/src/inst.c b/src/inst.c index 354ac6ec..9f1871da 100644 --- a/src/inst.c +++ b/src/inst.c @@ -171,6 +171,8 @@ static bool instantiate_should_copy_tree(tree_t t, void *__ctx) return decls != NULL && hset_contains(decls, t); case T_REF: return tree_kind(tree_ref(t)) == T_GENERIC_DECL; + case T_EXTERNAL_NAME: + return true; // Relative paths get converted to absolute default: return false; } diff --git a/src/lower.c b/src/lower.c index 8ecf10ec..c635b2ad 100644 --- a/src/lower.c +++ b/src/lower.c @@ -3008,49 +3008,18 @@ static vcode_reg_t lower_ref(lower_unit_t *lu, tree_t ref, expr_ctx_t ctx) static vcode_reg_t lower_external_name(lower_unit_t *lu, tree_t ref) { - tree_t root = NULL; - ident_t path = NULL; + assert(tree_subkind(tree_part(ref, 0)) == PE_ABSOLUTE); - switch (tree_subkind(tree_part(ref, 0))) { - case PE_ABSOLUTE: - { - tree_t p1 = tree_part(ref, 1); - path = ident_prefix(lib_name(lib_work()), tree_ident(p1), '.'); + tree_t p1 = tree_part(ref, 1), root = NULL; + ident_t path = ident_prefix(lib_name(lib_work()), tree_ident(p1), '.'); - vcode_unit_t vu = unit_registry_get(lu->registry, path); - if (vu != NULL && vcode_unit_kind(vu) == VCODE_UNIT_INSTANCE) { - ident_t module; - ptrdiff_t offset; - vcode_unit_object(vu, &module, &offset); + vcode_unit_t vu = unit_registry_get(lu->registry, path); + if (vu != NULL && vcode_unit_kind(vu) == VCODE_UNIT_INSTANCE) { + ident_t module; + ptrdiff_t offset; + vcode_unit_object(vu, &module, &offset); - root = tree_from_locus(module, offset, NULL); - } - } - break; - - case PE_RELATIVE: - { - int caret = 0; - const int nparts = tree_parts(ref); - for (int i = 1; i < nparts; i++) { - if (tree_subkind(tree_part(ref, i)) == PE_CARET) - caret++; - } - - for (lower_unit_t *it = lu; it; it = it->parent) { - if (is_concurrent_block(it->container)) { - tree_t hier = tree_decl(it->container, 0); - if (tree_subkind(hier) == T_COMPONENT) - continue; // Skip over implicit block for component - else if (caret-- == 0) { - root = it->container; - path = it->name; - break; - } - } - } - } - break; + root = tree_from_locus(module, offset, NULL); } tree_t decl = elab_external_name(ref, root, &path); diff --git a/src/parse.c b/src/parse.c index b3f14e07..f4c0f210 100644 --- a/src/parse.c +++ b/src/parse.c @@ -3666,6 +3666,8 @@ static tree_t p_external_name(void) consume(tGTGT); + tree_set_global_flags(t, TREE_GF_EXTERNAL_NAME); + tree_set_loc(t, CURRENT_LOC); return t; } diff --git a/src/tree.h b/src/tree.h index 723037c9..9c19e432 100644 --- a/src/tree.h +++ b/src/tree.h @@ -418,6 +418,7 @@ typedef enum { TREE_GF_INSTANCE_NAME = (1 << 0), TREE_GF_PATH_NAME = (1 << 1), TREE_GF_DEFERRED_INST = (1 << 2), + TREE_GF_EXTERNAL_NAME = (1 << 3), } tree_global_flags_t; tree_t tree_new(tree_kind_t kind); diff --git a/test/elab/ename1.vhd b/test/elab/ename1.vhd index 13cbd10b..7aad30e6 100644 --- a/test/elab/ename1.vhd +++ b/test/elab/ename1.vhd @@ -33,9 +33,7 @@ begin assert <> = 0; -- Error assert <> = 0; -- OK assert <> = '0'; -- Error - assert <> = '0'; -- Error assert <> = 0; -- Error - assert <> = '0'; -- Error assert <> = 0; -- Error assert <> = '0'; -- Error assert <> = 0; -- Error diff --git a/test/elab/ename3.vhd b/test/elab/ename3.vhd new file mode 100644 index 00000000..978bc3bd --- /dev/null +++ b/test/elab/ename3.vhd @@ -0,0 +1,34 @@ +entity bot is +end entity; + +architecture test of bot is + signal x, y : natural; +begin + + p1: process is + begin + wait for 5 ns; + x <= 5; + wait; + end process; + +end architecture; + +------------------------------------------------------------------------------- + +entity ename3 is +end entity; + +architecture test of ename3 is +begin + + uut: entity work.bot; + + p2: process is + begin + assert <> = '0'; -- Error + assert <> = '0'; -- Error + wait; + end process; + +end architecture; diff --git a/test/elab/issue855.vhd b/test/elab/issue855.vhd new file mode 100644 index 00000000..60fcecbe --- /dev/null +++ b/test/elab/issue855.vhd @@ -0,0 +1,42 @@ +entity B is + port ( + clk : in bit; + f : in bit + ); +end B; + +architecture rtl of B is +begin +end architecture; + +entity C is +port ( + clk : in bit; + e : in bit + ); +end C; + +architecture rtl of C is +signal f : bit; +begin +end architecture; + +entity issue855 is +end issue855; + +architecture sim of issue855 is + signal clk : bit; + signal e : bit; +begin + G : entity work.C(rtl) + port map ( + clk => clk, + e => e + ); + + H : entity work.B(rtl) + port map ( + clk => clk, + f => << signal G.f : bit >> + ); +end architecture; diff --git a/test/regress/ename8.vhd b/test/regress/ename8.vhd new file mode 100644 index 00000000..edd05ca5 --- /dev/null +++ b/test/regress/ename8.vhd @@ -0,0 +1,32 @@ +entity ename8 is +end entity; + +architecture test of ename8 is +begin + + g: for i in 1 to 1 generate + signal y : integer := 0; + begin + b: block is + signal x : integer := 0; + begin + process is + begin + << signal ^.^.g(1).y : integer >> <= 55; + wait for 1 ns; + assert x = 42; + wait; + end process; + end block; + + process is + begin + << signal b.x : integer >> <= 42; + wait for 1 ns; + assert y = 55; + wait; + end process; + + end generate; + +end architecture; diff --git a/test/regress/testlist.txt b/test/regress/testlist.txt index b2261d87..323d4fbd 100644 --- a/test/regress/testlist.txt +++ b/test/regress/testlist.txt @@ -952,3 +952,4 @@ wave11 wave,2008,dump-arrays vlog9 verilog issue654 normal,2008 issue854 normal,2019 +ename8 normal,2008 diff --git a/test/test_elab.c b/test/test_elab.c index 49cb1683..e0a87780 100644 --- a/test/test_elab.c +++ b/test/test_elab.c @@ -1088,12 +1088,10 @@ START_TEST(test_ename1) { 32, "class of object X is not variable" }, { 33, "external name X not found" }, { 35, "type of signal X is not BIT" }, - { 36, "relative pathname has no containing declarative region" }, - { 37, "external name X not found" }, - { 38, "relative pathname has no containing declarative region" }, - { 39, "FOO is not the name of the root of the design hierarchy" }, - { 40, "class of object UUT is not signal" }, - { 41, "X is not a concurrent region" }, + { 36, "external name X not found" }, + { 37, "FOO is not the name of the root of the design hierarchy" }, + { 38, "class of object UUT is not signal" }, + { 39, "X is not a concurrent region" }, { -1, NULL } }; expect_errors(expect); @@ -1828,6 +1826,38 @@ START_TEST(test_vlog1) } END_TEST +START_TEST(test_ename3) +{ + set_standard(STD_08); + input_from_file(TESTDIR "/elab/ename3.vhd"); + + const error_t expect[] = { + { 29, "relative pathname has no containing declarative region" }, + { 30, "relative pathname has no containing declarative region" }, + { -1, NULL } + }; + expect_errors(expect); + + tree_t e = run_elab(); + fail_unless(e == NULL); + + check_expected_errors(); +} +END_TEST + +START_TEST(test_issue855) +{ + set_standard(STD_08); + + input_from_file(TESTDIR "/elab/issue855.vhd"); + + tree_t e = run_elab(); + fail_if(e == NULL); + + fail_if_errors(); +} +END_TEST + Suite *get_elab_tests(void) { Suite *s = suite_create("elab"); @@ -1925,6 +1955,8 @@ Suite *get_elab_tests(void) tcase_add_test(tc, test_bounds11); tcase_add_test(tc, test_jcore2); tcase_add_test(tc, test_vlog1); + tcase_add_test(tc, test_ename3); + tcase_add_test(tc, test_issue855); suite_add_tcase(s, tc); return s; -- 2.39.2