This PR should be the base for discussion, do not merge it yet!

It correctly detects reg/wire mix and incorrect use on blocking,nonblocking assignments within blocks and assign statements.

What it DOES'T do:
Detect registers connected to output ports of instances.

Where it FAILS:
memorty nonblocking assignments causes spurious (I assume??) errors on yosys-generated "_ADDR", "_DATA", "EN" signals.

You can test it with tests/simple/reg_wire_error.v (look inside for the comments to enable/disable specific lines)
This commit is contained in:
Udi Finkelstein 2018-03-09 10:35:33 +02:00
parent efaef82f75
commit 2b9c75f8e3
6 changed files with 63 additions and 4 deletions

View File

@ -191,6 +191,7 @@ AstNode::AstNode(AstNodeType type, AstNode *child1, AstNode *child2, AstNode *ch
is_input = false; is_input = false;
is_output = false; is_output = false;
is_reg = false; is_reg = false;
is_logic = false;
is_signed = false; is_signed = false;
is_string = false; is_string = false;
range_valid = false; range_valid = false;
@ -285,7 +286,9 @@ void AstNode::dumpAst(FILE *f, std::string indent) const
fprintf(f, " input"); fprintf(f, " input");
if (is_output) if (is_output)
fprintf(f, " output"); fprintf(f, " output");
if (is_reg) if (is_logic)
fprintf(f, " logic");
if (is_reg) // this is an AST dump, not Verilog - if we see "logic reg" that's fine.
fprintf(f, " reg"); fprintf(f, " reg");
if (is_signed) if (is_signed)
fprintf(f, " signed"); fprintf(f, " signed");
@ -652,6 +655,8 @@ bool AstNode::operator==(const AstNode &other) const
return false; return false;
if (is_output != other.is_output) if (is_output != other.is_output)
return false; return false;
if (is_logic != other.is_logic)
return false;
if (is_reg != other.is_reg) if (is_reg != other.is_reg)
return false; return false;
if (is_signed != other.is_signed) if (is_signed != other.is_signed)

View File

@ -168,7 +168,7 @@ namespace AST
// node content - most of it is unused in most node types // node content - most of it is unused in most node types
std::string str; std::string str;
std::vector<RTLIL::State> bits; std::vector<RTLIL::State> bits;
bool is_input, is_output, is_reg, is_signed, is_string, range_valid, range_swapped; bool is_input, is_output, is_reg, is_logic, is_signed, is_string, range_valid, range_swapped;
int port_id, range_left, range_right; int port_id, range_left, range_right;
uint32_t integer; uint32_t integer;
double realvalue; double realvalue;

View File

@ -327,6 +327,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
if (node->type == AST_WIRE) { if (node->type == AST_WIRE) {
if (this_wire_scope.count(node->str) > 0) { if (this_wire_scope.count(node->str) > 0) {
AstNode *first_node = this_wire_scope[node->str]; AstNode *first_node = this_wire_scope[node->str];
if (first_node->is_input && node->is_reg)
goto wires_are_incompatible;
if (!node->is_input && !node->is_output && node->is_reg && node->children.size() == 0) if (!node->is_input && !node->is_output && node->is_reg && node->children.size() == 0)
goto wires_are_compatible; goto wires_are_compatible;
if (first_node->children.size() == 0 && node->children.size() == 1 && node->children[0]->type == AST_RANGE) { if (first_node->children.size() == 0 && node->children.size() == 1 && node->children[0]->type == AST_RANGE) {
@ -361,6 +363,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
first_node->is_output = true; first_node->is_output = true;
if (node->is_reg) if (node->is_reg)
first_node->is_reg = true; first_node->is_reg = true;
if (node->is_logic)
first_node->is_logic = true;
if (node->is_signed) if (node->is_signed)
first_node->is_signed = true; first_node->is_signed = true;
for (auto &it : node->attributes) { for (auto &it : node->attributes) {
@ -440,6 +444,12 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
children[1]->detectSignWidth(width_hint, sign_hint); children[1]->detectSignWidth(width_hint, sign_hint);
width_hint = max(width_hint, backup_width_hint); width_hint = max(width_hint, backup_width_hint);
child_0_is_self_determined = true; child_0_is_self_determined = true;
if ((type == AST_ASSIGN_LE || type == AST_ASSIGN_EQ) && children[0]->id2ast->is_logic)
children[0]->id2ast->is_reg = true; // if logic type is used in a block asignment
if ((type == AST_ASSIGN_LE || type == AST_ASSIGN_EQ) && !children[0]->id2ast->is_reg)
log_warning("wire '%s' is assigned in a block at %s:%d.\n", children[0]->str.c_str(), filename.c_str(), linenum);
if (type == AST_ASSIGN && children[0]->id2ast->is_reg)
log_error("reg '%s' is assigned in a continuous assignment at %s:%d.\n", children[0]->str.c_str(), filename.c_str(), linenum);
break; break;
case AST_PARAMETER: case AST_PARAMETER:

View File

@ -189,7 +189,7 @@ YOSYS_NAMESPACE_END
"const" { if (formal_mode) return TOK_CONST; SV_KEYWORD(TOK_CONST); } "const" { if (formal_mode) return TOK_CONST; SV_KEYWORD(TOK_CONST); }
"checker" { if (formal_mode) return TOK_CHECKER; SV_KEYWORD(TOK_CHECKER); } "checker" { if (formal_mode) return TOK_CHECKER; SV_KEYWORD(TOK_CHECKER); }
"endchecker" { if (formal_mode) return TOK_ENDCHECKER; SV_KEYWORD(TOK_ENDCHECKER); } "endchecker" { if (formal_mode) return TOK_ENDCHECKER; SV_KEYWORD(TOK_ENDCHECKER); }
"logic" { SV_KEYWORD(TOK_REG); } "logic" { SV_KEYWORD(TOK_LOGIC); }
"bit" { SV_KEYWORD(TOK_REG); } "bit" { SV_KEYWORD(TOK_REG); }
"eventually" { if (formal_mode) return TOK_EVENTUALLY; SV_KEYWORD(TOK_EVENTUALLY); } "eventually" { if (formal_mode) return TOK_EVENTUALLY; SV_KEYWORD(TOK_EVENTUALLY); }

View File

@ -105,7 +105,7 @@ static void free_attr(std::map<std::string, AstNode*> *al)
%token ATTR_BEGIN ATTR_END DEFATTR_BEGIN DEFATTR_END %token ATTR_BEGIN ATTR_END DEFATTR_BEGIN DEFATTR_END
%token TOK_MODULE TOK_ENDMODULE TOK_PARAMETER TOK_LOCALPARAM TOK_DEFPARAM %token TOK_MODULE TOK_ENDMODULE TOK_PARAMETER TOK_LOCALPARAM TOK_DEFPARAM
%token TOK_PACKAGE TOK_ENDPACKAGE TOK_PACKAGESEP %token TOK_PACKAGE TOK_ENDPACKAGE TOK_PACKAGESEP
%token TOK_INPUT TOK_OUTPUT TOK_INOUT TOK_WIRE TOK_REG %token TOK_INPUT TOK_OUTPUT TOK_INOUT TOK_WIRE TOK_REG TOK_LOGIC
%token TOK_INTEGER TOK_SIGNED TOK_ASSIGN TOK_ALWAYS TOK_INITIAL %token TOK_INTEGER TOK_SIGNED TOK_ASSIGN TOK_ALWAYS TOK_INITIAL
%token TOK_BEGIN TOK_END TOK_IF TOK_ELSE TOK_FOR TOK_WHILE TOK_REPEAT %token TOK_BEGIN TOK_END TOK_IF TOK_ELSE TOK_FOR TOK_WHILE TOK_REPEAT
%token TOK_DPI_FUNCTION TOK_POSEDGE TOK_NEGEDGE TOK_OR TOK_AUTOMATIC %token TOK_DPI_FUNCTION TOK_POSEDGE TOK_NEGEDGE TOK_OR TOK_AUTOMATIC
@ -394,6 +394,9 @@ wire_type_token:
TOK_REG { TOK_REG {
astbuf3->is_reg = true; astbuf3->is_reg = true;
} | } |
TOK_LOGIC {
astbuf3->is_logic = true;
} |
TOK_INTEGER { TOK_INTEGER {
astbuf3->is_reg = true; astbuf3->is_reg = true;
astbuf3->range_left = 31; astbuf3->range_left = 31;
@ -827,6 +830,7 @@ wire_name:
node->port_id = current_function_or_task_port_id++; node->port_id = current_function_or_task_port_id++;
} }
ast_stack.back()->children.push_back(node); ast_stack.back()->children.push_back(node);
delete $1; delete $1;
}; };

View File

@ -0,0 +1,40 @@
module sub_mod(input i_in, output o_out);
assign o_out = i_in;
endmodule
module test(i_clk, i_reg, o_reg, o_wire);
input i_clk;
input i_reg;
output o_reg;
output o_wire;
// Enable this to see how it doesn't fail on yosys although it should
//reg o_wire;
// Enable this instead of the above to see how logic can be mapped to a wire
logic o_wire;
// Enable this to see how it doesn't fail on yosys although it should
//reg i_reg;
// Disable this to see how it doesn't fail on yosys although it should
reg o_reg;
logic l_reg;
// Enable this to tst if logic-turne-reg will catch assignments even if done before it turned into a reg
//assign l_reg = !o_reg;
initial o_reg = 1'b0;
always @(posedge i_clk)
begin
o_reg <= !o_reg;
l_reg <= !o_reg;
end
assign o_wire = !o_reg;
// Uncomment this to see how a logic already turned intoa reg can be freely assigned on yosys
//assign l_reg = !o_reg;
sub_mod sm_inst (
.i_in(1'b1),
.o_out(o_reg)
);
endmodule