From c3e95eb1abb35fff98111b3a98e2fa7766754552 Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Fri, 31 Jul 2020 20:13:05 -0600 Subject: [PATCH] Fix generate scoping issues - expand_genblock defers prefixing of items within named sub-blocks - Allow partially-qualified references to local scopes - Handle shadowing within generate blocks - Resolve generate scope references within tasks and functions - Apply generate scoping to genvars - Resolves #2214, resolves #1456 --- frontends/ast/ast.h | 2 +- frontends/ast/simplify.cc | 123 +++++++++++++++++++++++++------------- tests/simple/generate.v | 85 ++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 42 deletions(-) diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index 9a5aa15f9..203b50021 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -250,7 +250,7 @@ namespace AST // it also sets the id2ast pointers so that identifier lookups are fast in genRTLIL() bool simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, int width_hint, bool sign_hint, bool in_param); AstNode *readmem(bool is_readmemh, std::string mem_filename, AstNode *memory, int start_addr, int finish_addr, bool unconditional_init); - void expand_genblock(std::string index_var, std::string prefix, std::map &name_map); + void expand_genblock(std::string index_var, std::string prefix, std::map &name_map, bool original_scope = true); void replace_ids(const std::string &prefix, const std::map &rules); void mem2reg_as_needed_pass1(dict> &mem2reg_places, dict &mem2reg_flags, dict &proc_flags, uint32_t &status_flags); diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 66f22e113..b0f936c15 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -3711,8 +3711,11 @@ AstNode *AstNode::readmem(bool is_readmemh, std::string mem_filename, AstNode *m } // annotate the names of all wires and other named objects in a generate block -void AstNode::expand_genblock(std::string index_var, std::string prefix, std::map &name_map) +void AstNode::expand_genblock(std::string index_var, std::string prefix, std::map &name_map, bool original_scope) { + // `original_scope` defaults to false, and is used to prevent the premature + // prefixing of items in named sub-blocks + if (!index_var.empty() && type == AST_IDENTIFIER && str == index_var) { if (children.empty()) { current_scope[index_var]->children[0]->cloneInto(this); @@ -3725,53 +3728,85 @@ void AstNode::expand_genblock(std::string index_var, std::string prefix, std::ma } } - if ((type == AST_IDENTIFIER || type == AST_FCALL || type == AST_TCALL || type == AST_WIRETYPE) && name_map.count(str) > 0) - str = name_map[str]; + if (type == AST_IDENTIFIER || type == AST_FCALL || type == AST_TCALL || type == AST_WIRETYPE) { + if (name_map.count(str) > 0) { + str = name_map[str]; + } else { + // remap the prefix of this ident if it is a local generate scope + size_t pos = str.rfind('.'); + if (pos != std::string::npos) { + std::string existing_prefix = str.substr(0, pos); + if (name_map.count(existing_prefix) > 0) { + str = name_map[existing_prefix] + str.substr(pos); + } + } + } + } std::map backup_name_map; + auto prefix_node = [&](AstNode* child) { + if (backup_name_map.size() == 0) + backup_name_map = name_map; + + // if within a nested scope + if (!original_scope) { + // this declaration shadows anything in the parent scope(s) + name_map[child->str] = child->str; + return; + } + + std::string new_name = prefix[0] == '\\' ? prefix.substr(1) : prefix; + size_t pos = child->str.rfind('.'); + if (pos == std::string::npos) + pos = child->str[0] == '\\' && prefix[0] == '\\' ? 1 : 0; + else + pos = pos + 1; + new_name = child->str.substr(0, pos) + new_name + child->str.substr(pos); + if (new_name[0] != '$' && new_name[0] != '\\') + new_name = prefix[0] + new_name; + + name_map[child->str] = new_name; + if (child->type == AST_FUNCTION) + replace_result_wire_name_in_function(child, child->str, new_name); + else + child->str = new_name; + current_scope[new_name] = child; + }; + for (size_t i = 0; i < children.size(); i++) { AstNode *child = children[i]; - if (child->type == AST_WIRE || child->type == AST_MEMORY || child->type == AST_PARAMETER || child->type == AST_LOCALPARAM || - child->type == AST_FUNCTION || child->type == AST_TASK || child->type == AST_CELL || child->type == AST_TYPEDEF || child->type == AST_ENUM_ITEM) { - if (backup_name_map.size() == 0) - backup_name_map = name_map; - std::string new_name = prefix[0] == '\\' ? prefix.substr(1) : prefix; - size_t pos = child->str.rfind('.'); - if (pos == std::string::npos) - pos = child->str[0] == '\\' && prefix[0] == '\\' ? 1 : 0; - else - pos = pos + 1; - new_name = child->str.substr(0, pos) + new_name + child->str.substr(pos); - if (new_name[0] != '$' && new_name[0] != '\\') - new_name = prefix[0] + new_name; - name_map[child->str] = new_name; - if (child->type == AST_FUNCTION) - replace_result_wire_name_in_function(child, child->str, new_name); - else - child->str = new_name; - current_scope[new_name] = child; - } - if (child->type == AST_ENUM){ + + switch (child->type) { + case AST_WIRE: + case AST_MEMORY: + case AST_PARAMETER: + case AST_LOCALPARAM: + case AST_FUNCTION: + case AST_TASK: + case AST_CELL: + case AST_TYPEDEF: + case AST_ENUM_ITEM: + case AST_GENVAR: + prefix_node(child); + break; + + case AST_BLOCK: + case AST_GENBLOCK: + if (!child->str.empty()) + prefix_node(child); + break; + + case AST_ENUM: current_scope[child->str] = child; for (auto enode : child->children){ log_assert(enode->type == AST_ENUM_ITEM); - if (backup_name_map.size() == 0) - backup_name_map = name_map; - std::string new_name = prefix[0] == '\\' ? prefix.substr(1) : prefix; - size_t pos = enode->str.rfind('.'); - if (pos == std::string::npos) - pos = enode->str[0] == '\\' && prefix[0] == '\\' ? 1 : 0; - else - pos = pos + 1; - new_name = enode->str.substr(0, pos) + new_name + enode->str.substr(pos); - if (new_name[0] != '$' && new_name[0] != '\\') - new_name = prefix[0] + new_name; - name_map[enode->str] = new_name; - - enode->str = new_name; - current_scope[new_name] = enode; + prefix_node(enode); } + break; + + default: + break; } } @@ -3781,8 +3816,14 @@ void AstNode::expand_genblock(std::string index_var, std::string prefix, std::ma // still needs to recursed-into if (type == AST_PREFIX && i == 1 && child->type == AST_IDENTIFIER) continue; - if (child->type != AST_FUNCTION && child->type != AST_TASK) - child->expand_genblock(index_var, prefix, name_map); + // functions/tasks may reference wires, constants, etc. in this scope + if (child->type == AST_FUNCTION || child->type == AST_TASK) + child->expand_genblock(index_var, prefix, name_map, false); + // continue prefixing if this child block is anonymous + else if (child->type == AST_GENBLOCK || child->type == AST_BLOCK) + child->expand_genblock(index_var, prefix, name_map, original_scope && child->str.empty()); + else + child->expand_genblock(index_var, prefix, name_map, original_scope); } diff --git a/tests/simple/generate.v b/tests/simple/generate.v index 0e353ad9b..dcd450e47 100644 --- a/tests/simple/generate.v +++ b/tests/simple/generate.v @@ -159,3 +159,88 @@ generate end endgenerate endmodule + +// ------------------------------------------ + +module gen_test7; + reg [2:0] out1; + reg [2:0] out2; + wire [2:0] out3; + generate + begin : cond + reg [2:0] sub_out1; + reg [2:0] sub_out2; + wire [2:0] sub_out3; + initial begin : init + reg signed [31:0] x; + x = 2 ** 2; + out1 = x; + sub_out1 = x; + end + always @* begin : proc + reg signed [31:0] x; + x = 2 ** 1; + out2 = x; + sub_out2 = x; + end + genvar x; + for (x = 0; x < 3; x = x + 1) begin + assign out3[x] = 1; + assign sub_out3[x] = 1; + end + end + endgenerate + +// `define VERIFY +`ifdef VERIFY + assert property (out1 == 4); + assert property (out2 == 2); + assert property (out3 == 7); + assert property (cond.sub_out1 == 4); + assert property (cond.sub_out2 == 2); + assert property (cond.sub_out3 == 7); +`endif +endmodule + +// ------------------------------------------ + +module gen_test8; + +// `define VERIFY +`ifdef VERIFY + `define ASSERT(expr) assert property (expr); +`else + `define ASSERT(expr) +`endif + + wire [1:0] x = 2'b11; + generate + begin : A + wire [1:0] x; + begin : B + wire [1:0] x = 2'b00; + `ASSERT(x == 0) + `ASSERT(A.x == 2) + `ASSERT(A.C.x == 1) + `ASSERT(A.B.x == 0) + end + begin : C + wire [1:0] x = 2'b01; + `ASSERT(x == 1) + `ASSERT(A.x == 2) + `ASSERT(A.C.x == 1) + `ASSERT(A.B.x == 0) + end + assign x = B.x ^ 2'b11 ^ C.x; + `ASSERT(x == 2) + `ASSERT(A.x == 2) + `ASSERT(A.C.x == 1) + `ASSERT(A.B.x == 0) + end + endgenerate + + `ASSERT(x == 3) + `ASSERT(A.x == 2) + `ASSERT(A.C.x == 1) + `ASSERT(A.B.x == 0) +endmodule -- 2.30.2