From ad437c178da6c1842983209a15259b5c6542d90e Mon Sep 17 00:00:00 2001 From: Dag Lem Date: Thu, 2 Mar 2023 19:02:30 +0100 Subject: [PATCH 1/2] Handling of attributes for struct / union variables (* nowrshmsk *) on a struct / union variable now affects dynamic bit slice assignments to members of the struct / union. (* nowrshmsk *) can in some cases yield significant resource savings; the combination of pipeline shifting and indexed writes is an example of this. Constructs similar to the one below can benefit from (* nowrshmsk *), and in addition it is no longer necessary to split out the shift assignments on separate lines in order to avoid the error message "ERROR: incompatible mix of lookahead and non-lookahead IDs in LHS expression." always_ff @(posedge clk) begin if (rotate) begin { v5, v4, v3, v2, v1, v0 } <= { v4, v3, v2, v1, v0, v5 }; if (res) begin v0.bytes <= '0; end else if (w) begin v0.bytes[addr] <= data; end end end --- frontends/ast/simplify.cc | 25 ++++++++++++++++++++----- frontends/verilog/verilog_parser.y | 7 ++++++- tests/svtypes/.gitignore | 1 + tests/svtypes/struct_dynamic_range.sv | 4 ++-- tests/svtypes/struct_dynamic_range.ys | 4 ++++ 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index dfb1b890c..1efd1b24d 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -557,7 +557,7 @@ static int get_max_offset(AstNode *node) return node->range_left; } -static AstNode *make_packed_struct(AstNode *template_node, std::string &name) +static AstNode *make_packed_struct(AstNode *template_node, std::string &name, decltype(AstNode::attributes) &attributes) { // create a wire for the packed struct auto wnode = new AstNode(AST_WIRE); @@ -565,6 +565,9 @@ static AstNode *make_packed_struct(AstNode *template_node, std::string &name) wnode->is_logic = true; wnode->range_valid = true; wnode->is_signed = template_node->is_signed; + for (auto &pair : attributes) { + wnode->attributes[pair.first] = pair.second->clone(); + } int offset = get_max_offset(template_node); auto range = make_range(offset, 0); wnode->children.push_back(range); @@ -1368,7 +1371,7 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, // instance rather than just a type in a typedef or outer struct? if (!str.empty() && str[0] == '\\') { // instance so add a wire for the packed structure - auto wnode = make_packed_struct(this, str); + auto wnode = make_packed_struct(this, str, attributes); log_assert(current_ast_mod); current_ast_mod->children.push_back(wnode); } @@ -1792,7 +1795,7 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, if (template_node->type == AST_STRUCT || template_node->type == AST_UNION) { // replace with wire representing the packed structure - newNode = make_packed_struct(template_node, str); + newNode = make_packed_struct(template_node, str, attributes); newNode->attributes[ID::wiretype] = mkconst_str(resolved_type_node->str); // add original input/output attribute to resolved wire newNode->is_input = this->is_input; @@ -1857,7 +1860,7 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, if (template_node->type == AST_STRUCT || template_node->type == AST_UNION) { // replace with wire representing the packed structure - newNode = make_packed_struct(template_node, str); + newNode = make_packed_struct(template_node, str, attributes); newNode->attributes[ID::wiretype] = mkconst_str(resolved_type_node->str); newNode->type = type; current_scope[str] = this; @@ -2709,11 +2712,23 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, int source_width = children[0]->id2ast->range_left - children[0]->id2ast->range_right + 1; int source_offset = children[0]->id2ast->range_right; int result_width = 1; + int stride = 1; AST::AstNode *member_node = get_struct_member(children[0]); if (member_node) { // Clamp chunk to range of member within struct/union. log_assert(!source_offset && !children[0]->id2ast->range_swapped); source_width = member_node->range_left - member_node->range_right + 1; + + // When the (* nowrshmsk *) attribute is set, a CASE block is generated below + // to select the indexed bit slice. When a multirange array is indexed, the + // start of each possible slice is separated by the bit stride of the last + // index dimension, and we can optimize the CASE block accordingly. + // The dimension of the original array expression is saved in the 'integer' field. + int dims = children[0]->integer; + stride = source_width; + for (int dim = 0; dim < dims; dim++) { + stride /= get_struct_range_width(member_node, dim); + } } AstNode *shift_expr = NULL; @@ -2754,7 +2769,7 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, did_something = true; newNode = new AstNode(AST_CASE, shift_expr); - for (int i = 0; i < source_width; i++) { + for (int i = 0; i < source_width; i += stride) { int start_bit = source_offset + i; int end_bit = std::min(start_bit+result_width,source_width) - 1; AstNode *cond = new AstNode(AST_COND, mkconst_int(start_bit, true)); diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 96413afb0..4c5bd0b10 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -1809,7 +1809,12 @@ enum_decl: enum_type enum_var_list ';' { delete $1; } // struct or union ////////////////// -struct_decl: struct_type struct_var_list ';' { delete astbuf2; } +struct_decl: + attr struct_type { + append_attr($2, $1); + } struct_var_list ';' { + delete astbuf2; + } ; struct_type: struct_union { astbuf2 = $1; } struct_body { $$ = astbuf2; } diff --git a/tests/svtypes/.gitignore b/tests/svtypes/.gitignore index b48f808a1..75154e735 100644 --- a/tests/svtypes/.gitignore +++ b/tests/svtypes/.gitignore @@ -1,3 +1,4 @@ /*.log /*.out /run-test.mk +/temp diff --git a/tests/svtypes/struct_dynamic_range.sv b/tests/svtypes/struct_dynamic_range.sv index ce1f14670..12c493d1f 100644 --- a/tests/svtypes/struct_dynamic_range.sv +++ b/tests/svtypes/struct_dynamic_range.sv @@ -4,7 +4,7 @@ module range_shift_mask( input logic [2:0] addr_o, output logic [7:0] data_o ); - // (* nowrshmsk = 0 *) + (* nowrshmsk = 0 *) struct packed { logic [7:0] msb; logic [0:3][7:0] data; @@ -24,7 +24,7 @@ module range_case( input logic [2:0] addr_o, output logic [7:0] data_o ); - // (* nowrshmsk = 1 *) + (* nowrshmsk = 1 *) struct packed { logic [7:0] msb; logic [0:3][7:0] data; diff --git a/tests/svtypes/struct_dynamic_range.ys b/tests/svtypes/struct_dynamic_range.ys index d09e1924d..943a8c634 100644 --- a/tests/svtypes/struct_dynamic_range.ys +++ b/tests/svtypes/struct_dynamic_range.ys @@ -1,4 +1,8 @@ +! mkdir -p temp read_verilog -sv struct_dynamic_range.sv +write_rtlil temp/struct_dynamic_range.il +! grep -F -q ' cell $shift ' temp/struct_dynamic_range.il +! grep -F -q ' switch $mul' temp/struct_dynamic_range.il prep -top top flatten sat -enable_undef -verify -prove-asserts From fb7f3bb290c81b4cf2b4faa2ad9e10e0da41fd16 Mon Sep 17 00:00:00 2001 From: Dag Lem Date: Thu, 4 May 2023 13:36:57 +0200 Subject: [PATCH 2/2] Cleaner tests for RTLIL cells in struct_dynamic_range.sv --- tests/svtypes/.gitignore | 1 - tests/svtypes/struct_dynamic_range.ys | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/svtypes/.gitignore b/tests/svtypes/.gitignore index 75154e735..b48f808a1 100644 --- a/tests/svtypes/.gitignore +++ b/tests/svtypes/.gitignore @@ -1,4 +1,3 @@ /*.log /*.out /run-test.mk -/temp diff --git a/tests/svtypes/struct_dynamic_range.ys b/tests/svtypes/struct_dynamic_range.ys index 943a8c634..9606e7384 100644 --- a/tests/svtypes/struct_dynamic_range.ys +++ b/tests/svtypes/struct_dynamic_range.ys @@ -1,8 +1,7 @@ -! mkdir -p temp read_verilog -sv struct_dynamic_range.sv -write_rtlil temp/struct_dynamic_range.il -! grep -F -q ' cell $shift ' temp/struct_dynamic_range.il -! grep -F -q ' switch $mul' temp/struct_dynamic_range.il +select -assert-count 4 t:$mul +select -assert-count 2 t:$shift +select -assert-count 2 t:$shiftx prep -top top flatten sat -enable_undef -verify -prove-asserts