From e07206a0faf693c41b6c2d682b912e1945f8fb0f Mon Sep 17 00:00:00 2001 From: Wojciech Sipak Date: Wed, 7 Jun 2023 15:14:42 +0200 Subject: [PATCH 1/4] simplify range before duplicating it --- systemverilog-plugin/UhdmAst.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index 0be44d71..3ed2ba72 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -319,10 +319,11 @@ static size_t add_multirange_attribute(AST::AstNode *wire_node, const std::vecto size_t size = 1; for (size_t i = 0; i < ranges.size(); i++) { log_assert(AST_INTERNAL::current_ast_mod); + simplify_sv(ranges[i], wire_node); + // If it's a range of [A], make it [A:A]. if (ranges[i]->children.size() == 1) { ranges[i]->children.push_back(ranges[i]->children[0]->clone()); } - simplify_sv(ranges[i], wire_node); while (simplify(ranges[i], true, false, false, 1, -1, false, false)) { } // this workaround case, where yosys doesn't follow id2ast and simplifies it to resolve constant From 81ce03b9fd05eeb8d31b4cdae1d0afad4082bbe6 Mon Sep 17 00:00:00 2001 From: Wojciech Sipak Date: Wed, 7 Jun 2023 16:46:23 +0200 Subject: [PATCH 2/4] refactor adding multirange attributes --- systemverilog-plugin/UhdmAst.cc | 79 ++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index 3ed2ba72..68fe1748 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -309,52 +309,58 @@ static void add_multirange_wire(AST::AstNode *node, std::vector } } -static size_t add_multirange_attribute(AST::AstNode *wire_node, const std::vector ranges) +// Sets the `wire_node->multirange_dimensions` attribute and returns the total sizes of packed and unpacked ranges. +static std::pair set_multirange_dimensions(AST::AstNode *wire_node, const std::vector packed_ranges, + const std::vector unpacked_ranges) { // node->multirange_dimensions stores dimensions' offsets and widths. // It shall have even number of elements. // For a range of [A:B] it should be appended with {min(A,B)} and {max(A,B)-min(A,B)+1} // For a range of [A] it should be appended with {0} and {A} - size_t size = 1; - for (size_t i = 0; i < ranges.size(); i++) { - log_assert(AST_INTERNAL::current_ast_mod); - simplify_sv(ranges[i], wire_node); - // If it's a range of [A], make it [A:A]. - if (ranges[i]->children.size() == 1) { - ranges[i]->children.push_back(ranges[i]->children[0]->clone()); - } - while (simplify(ranges[i], true, false, false, 1, -1, false, false)) { - } - // this workaround case, where yosys doesn't follow id2ast and simplifies it to resolve constant - if (ranges[i]->children[0]->id2ast) { - simplify_sv(ranges[i]->children[0]->id2ast, ranges[i]->children[0]); - while (simplify(ranges[i]->children[0]->id2ast, true, false, false, 1, -1, false, false)) { + auto calc_range_size = [wire_node](const std::vector &ranges) -> size_t { + size_t size = 1; + for (size_t i = 0; i < ranges.size(); i++) { + log_assert(AST_INTERNAL::current_ast_mod); + simplify_sv(ranges[i], wire_node); + // If it's a range of [A], make it [A:A]. + if (ranges[i]->children.size() == 1) { + ranges[i]->children.push_back(ranges[i]->children[0]->clone()); } - } - if (ranges[i]->children[1]->id2ast) { - simplify_sv(ranges[i]->children[1]->id2ast, ranges[i]->children[1]); - while (simplify(ranges[i]->children[1]->id2ast, true, false, false, 1, -1, false, false)) { + while (simplify(ranges[i], true, false, false, 1, -1, false, false)) { } - } - simplify_sv(ranges[i], wire_node); - while (simplify(ranges[i], true, false, false, 1, -1, false, false)) { - } - log_assert(ranges[i]->children[0]->type == AST::AST_CONSTANT); - log_assert(ranges[i]->children[1]->type == AST::AST_CONSTANT); + // this workaround case, where yosys doesn't follow id2ast and simplifies it to resolve constant + if (ranges[i]->children[0]->id2ast) { + simplify_sv(ranges[i]->children[0]->id2ast, ranges[i]->children[0]); + while (simplify(ranges[i]->children[0]->id2ast, true, false, false, 1, -1, false, false)) { + } + } + if (ranges[i]->children[1]->id2ast) { + simplify_sv(ranges[i]->children[1]->id2ast, ranges[i]->children[1]); + while (simplify(ranges[i]->children[1]->id2ast, true, false, false, 1, -1, false, false)) { + } + } + simplify_sv(ranges[i], wire_node); + while (simplify(ranges[i], true, false, false, 1, -1, false, false)) { + } + log_assert(ranges[i]->children[0]->type == AST::AST_CONSTANT); + log_assert(ranges[i]->children[1]->type == AST::AST_CONSTANT); - const auto low = min(ranges[i]->children[0]->integer, ranges[i]->children[1]->integer); - const auto high = max(ranges[i]->children[0]->integer, ranges[i]->children[1]->integer); - const auto elem_size = high - low + 1; + const auto low = min(ranges[i]->children[0]->integer, ranges[i]->children[1]->integer); + const auto high = max(ranges[i]->children[0]->integer, ranges[i]->children[1]->integer); + const auto elem_size = high - low + 1; - wire_node->multirange_dimensions.push_back(low); - wire_node->multirange_dimensions.push_back(elem_size); - wire_node->multirange_swapped.push_back(ranges[i]->range_swapped); - size *= elem_size; - } + wire_node->multirange_dimensions.push_back(low); + wire_node->multirange_dimensions.push_back(elem_size); + wire_node->multirange_swapped.push_back(ranges[i]->range_swapped); + size *= elem_size; + } + return size; + }; + size_t packed_size = calc_range_size(packed_ranges); + size_t unpacked_size = calc_range_size(unpacked_ranges); log_assert(wire_node->multirange_dimensions.size() % 2 == 0); - - return size; + return {packed_size, unpacked_size}; } static AST::AstNode *convert_range(AST::AstNode *id, int packed_ranges_size, int unpacked_ranges_size, int i) @@ -670,8 +676,7 @@ static void convert_packed_unpacked_range(AST::AstNode *wire_node) if (convert_node) { // if not already converted if (wire_node->multirange_dimensions.empty()) { - const size_t packed_size = add_multirange_attribute(wire_node, packed_ranges); - const size_t unpacked_size = add_multirange_attribute(wire_node, unpacked_ranges); + const auto [packed_size, unpacked_size] = set_multirange_dimensions(wire_node, packed_ranges, unpacked_ranges); if (packed_ranges.size() == 1 && unpacked_ranges.empty()) { ranges.push_back(packed_ranges[0]->clone()); } else if (unpacked_ranges.size() == 1 && packed_ranges.empty()) { From 195cd50d2be4cf02558079a39a9a1e6dc37ce06e Mon Sep 17 00:00:00 2001 From: Wojciech Sipak Date: Thu, 15 Jun 2023 14:33:48 +0200 Subject: [PATCH 3/4] fix process_net function --- systemverilog-plugin/UhdmAst.cc | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index 68fe1748..18cd2e83 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -4768,8 +4768,6 @@ void UhdmAst::process_port() void UhdmAst::process_net() { current_node = make_ast_node(AST::AST_WIRE); - std::vector packed_ranges; // comes before wire name - std::vector unpacked_ranges; // comes after wire name auto net_type = vpi_get(vpiNetType, obj_h); current_node->is_reg = net_type == vpiReg; current_node->is_output = net_type == vpiOutput; @@ -4782,14 +4780,13 @@ void UhdmAst::process_net() // wiretype needs to be 1st node current_node->children.insert(current_node->children.begin(), wiretype_node); current_node->is_custom_type = true; + } else { + // Ranges from the typespec are copied to the current node as attributes. + // So that multiranges can be replaced with a single range as a node later. + copy_packed_unpacked_attribute(node, current_node); } delete node; }); - if (vpiHandle typespec_h = vpi_handle(vpiTypespec, obj_h)) { - visit_one_to_many({vpiRange}, typespec_h, [&](AST::AstNode *node) { packed_ranges.push_back(node); }); - vpi_release_handle(typespec_h); - } - add_multirange_wire(current_node, packed_ranges, unpacked_ranges); } void UhdmAst::process_parameter() From ef522538afd504ddf1436039f426a2c1bbf9c282 Mon Sep 17 00:00:00 2001 From: Mariusz Glebocki Date: Fri, 18 Aug 2023 15:29:45 +0200 Subject: [PATCH 4/4] Fix null pointer dereference in `UhdmAst::process_net()`. --- systemverilog-plugin/UhdmAst.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index 18cd2e83..048b36b0 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -4774,7 +4774,9 @@ void UhdmAst::process_net() current_node->is_logic = !current_node->is_reg; current_node->is_signed = vpi_get(vpiSigned, obj_h); visit_one_to_one({vpiTypespec}, obj_h, [&](AST::AstNode *node) { - if (node && !node->str.empty()) { + if (!node) + return; + if (!node->str.empty()) { auto wiretype_node = new AST::AstNode(AST::AST_WIRETYPE); wiretype_node->str = node->str; // wiretype needs to be 1st node