From 33ba293b302a651ec0c42fee1a7345907f40b0c5 Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Sun, 28 Jan 2024 11:33:16 +0000 Subject: [PATCH] Avoid multiple evaluation of range bounds --- src/lower.c | 66 +++++++++++++++++++---------------------- src/simp.c | 23 +++++++------- test/lower/issue837.vhd | 30 +++++++++++++++++++ test/test_lower.c | 44 +++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 49 deletions(-) create mode 100644 test/lower/issue837.vhd diff --git a/src/lower.c b/src/lower.c index c217012e..5f0d99da 100644 --- a/src/lower.c +++ b/src/lower.c @@ -154,6 +154,7 @@ static void lower_check_indexes(lower_unit_t *lu, type_t from, type_t to, static vcode_reg_t lower_conversion(lower_unit_t *lu, vcode_reg_t value_reg, tree_t where, type_t from, type_t to); static vcode_reg_t lower_get_type_bounds(lower_unit_t *lu, type_t type); +static vcode_reg_t lower_attr_prefix(lower_unit_t *lu, tree_t prefix); typedef vcode_reg_t (*lower_signal_flag_fn_t)(vcode_reg_t, vcode_reg_t); typedef vcode_reg_t (*arith_fn_t)(vcode_reg_t, vcode_reg_t); @@ -296,36 +297,29 @@ static bool lower_can_be_signal(type_t type) } } -static bool lower_is_reverse_range(tree_t r, int *dim) +static vcode_reg_t lower_range_expr(lower_unit_t *lu, tree_t r, int *dim) { + assert(tree_subkind(r) == RANGE_EXPR); + tree_t value = tree_value(r); assert(tree_kind(value) == T_ATTR_REF); if (tree_params(value) > 0) - *dim = assume_int(tree_value(tree_param(value, 0))) - 1; - else - *dim = 0; + *dim = assume_int(tree_value(tree_param(value, 0))); - return tree_subkind(value) == ATTR_REVERSE_RANGE; -} + if (tree_subkind(value) == ATTR_REVERSE_RANGE) + *dim *= -1; -static vcode_reg_t lower_range_expr(lower_unit_t *lu, tree_t r) -{ - assert(tree_subkind(r) == RANGE_EXPR); + tree_t prefix = tree_name(value); + type_t type = tree_type(prefix); + assert(!type_const_bounds(type)); - tree_t array = tree_name(tree_value(r)); - assert(!type_const_bounds(tree_type(array))); + vcode_reg_t prefix_reg = lower_attr_prefix(lu, prefix); + if (prefix_reg == VCODE_INVALID_REG) + return lower_get_type_bounds(lu, type); - if (lower_is_signal_ref(array)) { - // Must use lower_lvalue here to avoid getting the resolved value - vcode_reg_t array_reg = lower_lvalue(lu, array); - if (have_uarray_ptr(array_reg)) - return emit_load_indirect(array_reg); - else - return array_reg; - } - else - return lower_rvalue(lu, array); + assert(vcode_reg_kind(prefix_reg) == VCODE_TYPE_UARRAY); + return prefix_reg; } static vcode_reg_t lower_range_left(lower_unit_t *lu, tree_t r) @@ -333,13 +327,13 @@ static vcode_reg_t lower_range_left(lower_unit_t *lu, tree_t r) assert(tree_kind(r) == T_RANGE); if (tree_subkind(r) == RANGE_EXPR) { - vcode_reg_t array_reg = lower_range_expr(lu, r), left_reg; + int dim = 1; + vcode_reg_t array_reg = lower_range_expr(lu, r, &dim), left_reg; - int dim; - if (lower_is_reverse_range(r, &dim)) - left_reg = emit_uarray_right(array_reg, dim); + if (dim < 0) + left_reg = emit_uarray_right(array_reg, -dim - 1); else - left_reg = emit_uarray_left(array_reg, dim); + left_reg = emit_uarray_left(array_reg, dim - 1); type_t type = tree_type(r); vcode_type_t vtype = lower_type(type); @@ -355,13 +349,13 @@ static vcode_reg_t lower_range_right(lower_unit_t *lu, tree_t r) assert(tree_kind(r) == T_RANGE); if (tree_subkind(r) == RANGE_EXPR) { - vcode_reg_t array_reg = lower_range_expr(lu, r), right_reg; + int dim = 1; + vcode_reg_t array_reg = lower_range_expr(lu, r, &dim), right_reg; - int dim; - if (lower_is_reverse_range(r, &dim)) - right_reg = emit_uarray_left(array_reg, dim); + if (dim < 0) + right_reg = emit_uarray_left(array_reg, -dim - 1); else - right_reg = emit_uarray_right(array_reg, dim); + right_reg = emit_uarray_right(array_reg, dim - 1); type_t type = tree_type(r); vcode_type_t vtype = lower_type(type); @@ -383,13 +377,13 @@ static vcode_reg_t lower_range_dir(lower_unit_t *lu, tree_t r) case RANGE_EXPR: { - vcode_reg_t array_reg = lower_range_expr(lu, r); + int dim = 1; + vcode_reg_t array_reg = lower_range_expr(lu, r, &dim); - int dim; - if (lower_is_reverse_range(r, &dim)) - return emit_not(emit_uarray_dir(array_reg, dim)); + if (dim < 0) + return emit_not(emit_uarray_dir(array_reg, -dim - 1)); else - return emit_uarray_dir(array_reg, dim); + return emit_uarray_dir(array_reg, dim - 1); } case RANGE_ERROR: diff --git a/src/simp.c b/src/simp.c index 26ef3237..29170aa1 100644 --- a/src/simp.c +++ b/src/simp.c @@ -1350,7 +1350,7 @@ static tree_t simp_range(tree_t t) tree_t name = tree_name(value); type_t type = tree_type(name); - if (type_is_unconstrained(type)) + if (!type_const_bounds(type)) return t; int dim = 0; @@ -1364,18 +1364,15 @@ static tree_t simp_range(tree_t t) if (attr == ATTR_REVERSE_RANGE) { tree_t base_r = range_of(type, dim); const range_kind_t base_kind = tree_subkind(base_r); - if (base_kind == RANGE_EXPR) - return t; - else { - tree_t rev = tree_new(T_RANGE); - tree_set_subkind(rev, base_kind ^ 1); - tree_set_loc(rev, tree_loc(t)); - tree_set_type(rev, tree_type(t)); - tree_set_left(rev, tree_right(base_r)); - tree_set_right(rev, tree_left(base_r)); - - return rev; - } + + tree_t rev = tree_new(T_RANGE); + tree_set_subkind(rev, base_kind ^ 1); + tree_set_loc(rev, tree_loc(t)); + tree_set_type(rev, tree_type(t)); + tree_set_left(rev, tree_right(base_r)); + tree_set_right(rev, tree_left(base_r)); + + return rev; } else return range_of(type, dim); diff --git a/test/lower/issue837.vhd b/test/lower/issue837.vhd new file mode 100644 index 00000000..d091f496 --- /dev/null +++ b/test/lower/issue837.vhd @@ -0,0 +1,30 @@ +package pack is + function expensive (x : string) return integer; +end package; + +package body pack is + function expensive (x : string) return integer is + begin + return integer'value(x); + end function; +end package body; + +------------------------------------------------------------------------------- + +use work.pack.all; + +entity issue837 is +end entity; + +architecture test of issue837 is + signal x : bit_vector(1 to 12); + constant str : string := "12"; +begin + b: block is + port ( p : in bit_vector(1 to expensive(str)) ); + port map ( p => x ); + signal s : p'subtype; -- Should not call EXPENSIVE + begin + end block; + +end architecture; diff --git a/test/test_lower.c b/test/test_lower.c index 133d4d42..87349aed 100644 --- a/test/test_lower.c +++ b/test/test_lower.c @@ -5868,6 +5868,49 @@ START_TEST(test_directmap5) } END_TEST +START_TEST(test_issue837) +{ + set_standard(STD_08); + + input_from_file(TESTDIR "/lower/issue837.vhd"); + + run_elab(); + + vcode_unit_t vu = find_unit("WORK.ISSUE837.B"); + vcode_select_unit(vu); + + EXPECT_BB(0) = { + { VCODE_OP_PACKAGE_INIT, .name = "STD.STANDARD" }, + { VCODE_OP_PACKAGE_INIT, .name = "WORK.PACK" }, + { VCODE_OP_VAR_UPREF, .hops = 1, .name = "X" }, + { VCODE_OP_LOAD_INDIRECT }, + { VCODE_OP_CONST, .value = 1 }, + { VCODE_OP_VAR_UPREF, .hops = 1, .name = "STR" }, + { VCODE_OP_CONST, .value = 2 }, + { VCODE_OP_CONST, .value = 0 }, + { VCODE_OP_WRAP }, + { VCODE_OP_FCALL, .func = "WORK.PACK.EXPENSIVE(S)I" }, + { VCODE_OP_DEBUG_LOCUS }, + { VCODE_OP_RANGE_LENGTH }, + { VCODE_OP_CONST, .value = 12 }, + { VCODE_OP_LENGTH_CHECK }, + { VCODE_OP_DEBUG_LOCUS }, + { VCODE_OP_ALIAS_SIGNAL }, + { VCODE_OP_WRAP }, + { VCODE_OP_STORE, .name = "P" }, + { VCODE_OP_CONST, .value = 1 }, + { VCODE_OP_DEBUG_LOCUS }, + { VCODE_OP_CONST, .value = 0 }, + { VCODE_OP_INIT_SIGNAL }, + { VCODE_OP_WRAP }, + { VCODE_OP_STORE, .name = "S" }, + { VCODE_OP_RETURN }, + }; + + CHECK_BB(0); +} +END_TEST + Suite *get_lower_tests(void) { Suite *s = suite_create("lower"); @@ -6009,6 +6052,7 @@ Suite *get_lower_tests(void) tcase_add_test(tc, test_directmap4); tcase_add_test(tc, test_subtype2); tcase_add_test(tc, test_directmap5); + tcase_add_test(tc, test_issue837); suite_add_tcase(s, tc); return s; -- 2.39.2