From 4edb1a19210b9b3118d7b0072e13a4363f81fb94 Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Tue, 5 Sep 2023 22:19:28 -0400 Subject: [PATCH 1/3] sv: support assignments within expressions - Add support for assignments within expressions, e.g., `x[y++] = z;` or `x = (y *= 2) - 1;`. The logic is handled entirely within the parser by injecting statements into the current procedural block. - Add support for pre-increment/decrement statements, which are behaviorally equivalent to post-increment/decrement statements. - Fix non-standard attribute position used for post-increment/decrement statements. --- CHANGELOG | 4 + README.md | 2 + frontends/verilog/verilog_parser.y | 113 ++++++++++++++++++++------ tests/verilog/asgn_expr.sv | 60 ++++++++++++++ tests/verilog/asgn_expr.ys | 3 + tests/verilog/asgn_expr_not_proc_1.ys | 7 ++ tests/verilog/asgn_expr_not_proc_2.ys | 7 ++ tests/verilog/asgn_expr_not_proc_3.ys | 7 ++ tests/verilog/asgn_expr_not_proc_4.ys | 7 ++ tests/verilog/asgn_expr_not_proc_5.ys | 7 ++ tests/verilog/asgn_expr_not_sv_1.ys | 7 ++ tests/verilog/asgn_expr_not_sv_2.ys | 7 ++ tests/verilog/asgn_expr_not_sv_3.ys | 7 ++ tests/verilog/asgn_expr_not_sv_4.ys | 15 ++++ tests/verilog/task_attr.ys | 2 +- 15 files changed, 231 insertions(+), 24 deletions(-) create mode 100644 tests/verilog/asgn_expr.sv create mode 100644 tests/verilog/asgn_expr.ys create mode 100644 tests/verilog/asgn_expr_not_proc_1.ys create mode 100644 tests/verilog/asgn_expr_not_proc_2.ys create mode 100644 tests/verilog/asgn_expr_not_proc_3.ys create mode 100644 tests/verilog/asgn_expr_not_proc_4.ys create mode 100644 tests/verilog/asgn_expr_not_proc_5.ys create mode 100644 tests/verilog/asgn_expr_not_sv_1.ys create mode 100644 tests/verilog/asgn_expr_not_sv_2.ys create mode 100644 tests/verilog/asgn_expr_not_sv_3.ys create mode 100644 tests/verilog/asgn_expr_not_sv_4.ys diff --git a/CHANGELOG b/CHANGELOG index 465918a36..a662ba4da 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,10 @@ List of major changes and improvements between releases Yosys 0.33 .. Yosys 0.34-dev -------------------------- + * SystemVerilog + - Added support for assignments within expressions, e.g., `x[y++] = z;` or + `x = (y *= 2) - 1;`. + Yosys 0.32 .. Yosys 0.33 -------------------------- * Various diff --git a/README.md b/README.md index 5e5a8ec3e..69a227b7f 100644 --- a/README.md +++ b/README.md @@ -592,6 +592,8 @@ from SystemVerilog: - SystemVerilog interfaces (SVIs) are supported. Modports for specifying whether ports are inputs or outputs are supported. +- Assignments within expressions are supported. + Building the documentation ========================== diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 1e82940bb..d901b3b55 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -292,6 +292,61 @@ static void rewriteGenForDeclInit(AstNode *loop) substitute(incr); } +static void ensureAsgnExprAllowed() +{ + if (!sv_mode) + frontend_verilog_yyerror("Assignments within expressions are only supported in SystemVerilog mode."); + if (ast_stack.back()->type != AST_BLOCK) + frontend_verilog_yyerror("Assignments within expressions are only permitted within procedures."); +} + +// add a pre/post-increment/decrement statement +static const AstNode *addIncOrDecStmt(AstNode *lhs, dict *attr, AST::AstNodeType op, YYLTYPE begin, YYLTYPE end) +{ + AstNode *one = AstNode::mkconst_int(1, true); + AstNode *rhs = new AstNode(op, lhs->clone(), one); + AstNode *stmt = new AstNode(AST_ASSIGN_EQ, lhs, rhs); + SET_AST_NODE_LOC(stmt, begin, end); + if (attr != nullptr) + append_attr(stmt, attr); + ast_stack.back()->children.push_back(stmt); + return stmt; +} + +// create a pre/post-increment/decrement expression, and add the corresponding statement +static AstNode *addIncOrDecExpr(AstNode *lhs, dict *attr, AST::AstNodeType op, YYLTYPE begin, YYLTYPE end, bool undo) +{ + ensureAsgnExprAllowed(); + const AstNode *stmt = addIncOrDecStmt(lhs, attr, op, begin, end); + log_assert(stmt->type == AST_ASSIGN_EQ); + AstNode *expr = stmt->children[0]->clone(); + if (undo) { + AstNode *minus_one = AstNode::mkconst_int(-1, true); + expr = new AstNode(op, expr, minus_one); + } + SET_AST_NODE_LOC(expr, begin, end); + return expr; +} + +// add a binary operator assignment statement, e.g., a += b +static const AstNode *addAsgnBinopStmt(dict *attr, AstNode *lhs, AST::AstNodeType op, AstNode *rhs, YYLTYPE begin, YYLTYPE end) +{ + SET_AST_NODE_LOC(rhs, end, end); + if (op == AST_SHIFT_LEFT || op == AST_SHIFT_RIGHT || + op == AST_SHIFT_SLEFT || op == AST_SHIFT_SRIGHT) { + rhs = new AstNode(AST_TO_UNSIGNED, rhs); + SET_AST_NODE_LOC(rhs, end, end); + } + rhs = new AstNode(op, lhs->clone(), rhs); + AstNode *stmt = new AstNode(AST_ASSIGN_EQ, lhs, rhs); + SET_AST_NODE_LOC(rhs, begin, end); + SET_AST_NODE_LOC(stmt, begin, end); + ast_stack.back()->children.push_back(stmt); + if (attr != nullptr) + append_attr(stmt, attr); + return lhs; +} + %} %define api.prefix {frontend_verilog_yy} @@ -358,7 +413,7 @@ static void rewriteGenForDeclInit(AstNode *loop) %type integer_atom_type integer_vector_type %type attr case_attr %type struct_union -%type asgn_binop +%type asgn_binop inc_or_dec_op %type genvar_identifier %type specify_target @@ -2610,17 +2665,15 @@ simple_behavioral_stmt: SET_AST_NODE_LOC(node, @2, @5); append_attr(node, $1); } | - attr lvalue TOK_INCREMENT { - AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, new AstNode(AST_ADD, $2->clone(), AstNode::mkconst_int(1, true))); - ast_stack.back()->children.push_back(node); - SET_AST_NODE_LOC(node, @2, @3); - append_attr(node, $1); + attr lvalue attr inc_or_dec_op { + // The position 1 attr to avoid shift/reduce conflicts with the + // other productions. We reject attributes in that position. + if (!$1->empty()) + frontend_verilog_yyerror("Attributes are not allowed on this size of the lvalue in an inc_or_dec_expression!"); + addIncOrDecStmt($2, $3, $4, @1, @4); } | - attr lvalue TOK_DECREMENT { - AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, new AstNode(AST_SUB, $2->clone(), AstNode::mkconst_int(1, true))); - ast_stack.back()->children.push_back(node); - SET_AST_NODE_LOC(node, @2, @3); - append_attr(node, $1); + inc_or_dec_op attr lvalue { + addIncOrDecStmt($3, $2, $1, @1, @3); } | attr lvalue OP_LE delay expr { AstNode *node = new AstNode(AST_ASSIGN_LE, $2, $5); @@ -2629,18 +2682,7 @@ simple_behavioral_stmt: append_attr(node, $1); } | attr lvalue asgn_binop delay expr { - AstNode *expr_node = $5; - if ($3 == AST_SHIFT_LEFT || $3 == AST_SHIFT_RIGHT || - $3 == AST_SHIFT_SLEFT || $3 == AST_SHIFT_SRIGHT) { - expr_node = new AstNode(AST_TO_UNSIGNED, expr_node); - SET_AST_NODE_LOC(expr_node, @5, @5); - } - AstNode *op_node = new AstNode($3, $2->clone(), expr_node); - AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, op_node); - SET_AST_NODE_LOC(op_node, @2, @5); - SET_AST_NODE_LOC(node, @2, @5); - ast_stack.back()->children.push_back(node); - append_attr(node, $1); + addAsgnBinopStmt($1, $2, $3, $5, @2, @5); }; asgn_binop: @@ -2657,6 +2699,12 @@ asgn_binop: TOK_SSHL_ASSIGN { $$ = AST_SHIFT_SLEFT; } | TOK_SSHR_ASSIGN { $$ = AST_SHIFT_SRIGHT; } ; +inc_or_dec_op: + // NOTE: These should only be permitted in SV mode, but Yosys has + // allowed them in all modes since support for them was added in 2017. + TOK_INCREMENT { $$ = AST_ADD; } | + TOK_DECREMENT { $$ = AST_SUB; } ; + for_initialization: TOK_ID '=' expr { AstNode *ident = new AstNode(AST_IDENTIFIER); @@ -3149,6 +3197,14 @@ expr: $$->children.push_back($6); SET_AST_NODE_LOC($$, @1, @$); append_attr($$, $3); + } | + inc_or_dec_op attr rvalue { + $$ = addIncOrDecExpr($3, $2, $1, @1, @3, false); + } | + // TODO: Attributes are allowed in the middle here, but they create some + // non-trivial conflicts that don't seem worth solving for now. + rvalue inc_or_dec_op { + $$ = addIncOrDecExpr($1, nullptr, $2, @1, @2, true); }; basic_expr: @@ -3436,6 +3492,17 @@ basic_expr: frontend_verilog_yyerror("Static cast is only supported in SystemVerilog mode."); $$ = new AstNode(AST_CAST_SIZE, $1, $4); SET_AST_NODE_LOC($$, @1, @4); + } | + '(' expr '=' expr ')' { + ensureAsgnExprAllowed(); + AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, $4); + ast_stack.back()->children.push_back(node); + SET_AST_NODE_LOC(node, @2, @4); + $$ = $2->clone(); + } | + '(' expr asgn_binop expr ')' { + ensureAsgnExprAllowed(); + $$ = addAsgnBinopStmt(nullptr, $2, $3, $4, @2, @4)-> clone(); }; concat_list: diff --git a/tests/verilog/asgn_expr.sv b/tests/verilog/asgn_expr.sv new file mode 100644 index 000000000..567034d10 --- /dev/null +++ b/tests/verilog/asgn_expr.sv @@ -0,0 +1,60 @@ +module top; + integer x, y, z; + task check; + input integer a, b, c; + assert (x == a); + assert (y == b); + assert (z == c); + endtask + always_comb begin + x = 0; y = 0; z = 0; + check(0, 0, 0); + + // post-increment/decrement statements + x++; + check(1, 0, 0); + y (* foo *) ++; + check(1, 1, 0); + z--; + check(1, 1, -1); + z (* foo *) --; + check(1, 1, -2); + + // pre-increment/decrement statements are equivalent + ++z; + check(1, 1, -1); + ++ (* foo *) z; + check(1, 1, 0); + --x; + check(0, 1, 0); + -- (* foo *) y; + check(0, 0, 0); + + // procedural pre-increment/decrement expressions + z = ++x; + check(1, 0, 1); + z = ++ (* foo *) x; + check(2, 0, 2); + y = --x; + check(1, 1, 2); + y = -- (* foo *) x; + + // procedural post-increment/decrement expressions + // TODO: support attributes on post-increment/decrement + check(0, 0, 2); + y = x++; + check(1, 0, 2); + y = x--; + check(0, 1, 2); + + // procedural assignment expressions + x = (y = (z = 99) + 1) + 1; + check(101, 100, 99); + x = (y *= 2); + check(200, 200, 99); + x = (z >>= 2) * 4; + check(96, 200, 24); + y = (z >>= 1'sb1) * 2; // shift is implicitly cast to unsigned + check(96, 24, 12); + end +endmodule diff --git a/tests/verilog/asgn_expr.ys b/tests/verilog/asgn_expr.ys new file mode 100644 index 000000000..18180c785 --- /dev/null +++ b/tests/verilog/asgn_expr.ys @@ -0,0 +1,3 @@ +read_verilog -sv asgn_expr.sv +proc +sat -verify -prove-asserts -show-all diff --git a/tests/verilog/asgn_expr_not_proc_1.ys b/tests/verilog/asgn_expr_not_proc_1.ys new file mode 100644 index 000000000..72ba0bd89 --- /dev/null +++ b/tests/verilog/asgn_expr_not_proc_1.ys @@ -0,0 +1,7 @@ +logger -expect error "Assignments within expressions are only permitted within procedures." 1 +read_verilog -sv < Date: Mon, 18 Sep 2023 23:26:35 -0400 Subject: [PATCH 2/3] allow attributes in front of ++/-- statements --- frontends/verilog/verilog_parser.y | 22 +++++++++++----------- tests/verilog/asgn_expr.sv | 8 ++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index d901b3b55..04bf2c87e 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -301,14 +301,18 @@ static void ensureAsgnExprAllowed() } // add a pre/post-increment/decrement statement -static const AstNode *addIncOrDecStmt(AstNode *lhs, dict *attr, AST::AstNodeType op, YYLTYPE begin, YYLTYPE end) +static const AstNode *addIncOrDecStmt(dict *stmt_attr, AstNode *lhs, + dict *op_attr, AST::AstNodeType op, + YYLTYPE begin, YYLTYPE end) { AstNode *one = AstNode::mkconst_int(1, true); AstNode *rhs = new AstNode(op, lhs->clone(), one); + if (op_attr != nullptr) + append_attr(rhs, op_attr); AstNode *stmt = new AstNode(AST_ASSIGN_EQ, lhs, rhs); SET_AST_NODE_LOC(stmt, begin, end); - if (attr != nullptr) - append_attr(stmt, attr); + if (stmt_attr != nullptr) + append_attr(stmt, stmt_attr); ast_stack.back()->children.push_back(stmt); return stmt; } @@ -317,7 +321,7 @@ static const AstNode *addIncOrDecStmt(AstNode *lhs, dict *at static AstNode *addIncOrDecExpr(AstNode *lhs, dict *attr, AST::AstNodeType op, YYLTYPE begin, YYLTYPE end, bool undo) { ensureAsgnExprAllowed(); - const AstNode *stmt = addIncOrDecStmt(lhs, attr, op, begin, end); + const AstNode *stmt = addIncOrDecStmt(nullptr, lhs, attr, op, begin, end); log_assert(stmt->type == AST_ASSIGN_EQ); AstNode *expr = stmt->children[0]->clone(); if (undo) { @@ -2666,14 +2670,10 @@ simple_behavioral_stmt: append_attr(node, $1); } | attr lvalue attr inc_or_dec_op { - // The position 1 attr to avoid shift/reduce conflicts with the - // other productions. We reject attributes in that position. - if (!$1->empty()) - frontend_verilog_yyerror("Attributes are not allowed on this size of the lvalue in an inc_or_dec_expression!"); - addIncOrDecStmt($2, $3, $4, @1, @4); + addIncOrDecStmt($1, $2, $3, $4, @1, @4); } | - inc_or_dec_op attr lvalue { - addIncOrDecStmt($3, $2, $1, @1, @3); + attr inc_or_dec_op attr lvalue { + addIncOrDecStmt($1, $4, $3, $2, @1, @4); } | attr lvalue OP_LE delay expr { AstNode *node = new AstNode(AST_ASSIGN_LE, $2, $5); diff --git a/tests/verilog/asgn_expr.sv b/tests/verilog/asgn_expr.sv index 567034d10..9b874ede3 100644 --- a/tests/verilog/asgn_expr.sv +++ b/tests/verilog/asgn_expr.sv @@ -13,21 +13,21 @@ module top; // post-increment/decrement statements x++; check(1, 0, 0); - y (* foo *) ++; + (* bar *) y (* foo *) ++; check(1, 1, 0); z--; check(1, 1, -1); - z (* foo *) --; + (* bar *) z (* foo *) --; check(1, 1, -2); // pre-increment/decrement statements are equivalent ++z; check(1, 1, -1); - ++ (* foo *) z; + (* bar *) ++ (* foo *) z; check(1, 1, 0); --x; check(0, 1, 0); - -- (* foo *) y; + (* bar *) -- (* foo *) y; check(0, 0, 0); // procedural pre-increment/decrement expressions From 28e99f2b8c50557e0376fbded1c77236a987ecec Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Mon, 18 Sep 2023 23:35:18 -0400 Subject: [PATCH 3/3] fix width of post-increment/decrement expressions --- frontends/verilog/verilog_parser.y | 2 +- tests/verilog/asgn_expr.sv | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 04bf2c87e..cb8c453c0 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -325,7 +325,7 @@ static AstNode *addIncOrDecExpr(AstNode *lhs, dict *attr, AS log_assert(stmt->type == AST_ASSIGN_EQ); AstNode *expr = stmt->children[0]->clone(); if (undo) { - AstNode *minus_one = AstNode::mkconst_int(-1, true); + AstNode *minus_one = AstNode::mkconst_int(-1, true, 1); expr = new AstNode(op, expr, minus_one); } SET_AST_NODE_LOC(expr, begin, end); diff --git a/tests/verilog/asgn_expr.sv b/tests/verilog/asgn_expr.sv index 9b874ede3..25f9caa33 100644 --- a/tests/verilog/asgn_expr.sv +++ b/tests/verilog/asgn_expr.sv @@ -56,5 +56,18 @@ module top; check(96, 200, 24); y = (z >>= 1'sb1) * 2; // shift is implicitly cast to unsigned check(96, 24, 12); + + // check width of post-increment expressions + z = (y = 0); + begin + byte w; + w = 0; + x = {1'b1, ++w}; + check(257, 0, 0); + assert (w == 1); + x = {2'b10, w++}; + check(513, 0, 0); + assert (w == 2); + end end endmodule