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

diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h
index 9a5aa15f941844a7168c2b15f77ee5081ad4e135..203b500214cf2cd5144fd69543db0914aae87242 100644 (file)
--- 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<std::string, std::string> &name_map);
+               void expand_genblock(std::string index_var, std::string prefix, std::map<std::string, std::string> &name_map, bool original_scope = true);
                void replace_ids(const std::string &prefix, const std::map<std::string, std::string> &rules);
                void mem2reg_as_needed_pass1(dict<AstNode*, pool<std::string>> &mem2reg_places,
                                dict<AstNode*, uint32_t> &mem2reg_flags, dict<AstNode*, uint32_t> &proc_flags, uint32_t &status_flags);

diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc
index 66f22e1135ebc4469f43389371b03dbf6c295061..b0f936c15e8909702422ed185b69979601ea2bdf 100644 (file)
--- 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<std::string, std::string> &name_map)
+void AstNode::expand_genblock(std::string index_var, std::string prefix, std::map<std::string, std::string> &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<std::string, std::string> 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 0e353ad9b7b5422f6d82bcb2884873bfcb685378..dcd450e472ad42004dc4fb45909ced5e9eb11134 100644 (file)
--- 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