From c3e95eb1abb35fff98111b3a98e2fa7766754552 Mon Sep 17 00:00:00 2001
From: Zachary Snow <zach@zachjs.com>
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<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 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<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 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