Merge pull request #1511 from YosysHQ/dave/always

sv: Error checking for always_comb, always_latch and always_ff

6 files changed, 126 insertions, 9 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1fc139d49..a49c27b05 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -51,6 +51,8 @@ Yosys 0.9 .. Yosys 0.9-dev
     - "synth_ice40 -dsp" to infer DSP blocks
     - Added latch support to synth_xilinx
     - Added "check -mapped"
+    - Added checking of SystemVerilog always block types (always_comb,
+      always_latch and always_ff)
 
 Yosys 0.8 .. Yosys 0.9
 ----------------------
diff --git a/README.md b/README.md
index db7810cb4..e46971526 100644
--- a/README.md
+++ b/README.md
@@ -371,6 +371,11 @@ Verilog Attributes and non-standard features
   for example, to specify the clk-to-Q delay of a flip-flop for consideration
   during techmapping.
 
+- The frontend sets attributes ``always_comb``, ``always_latch`` and
+  ``always_ff`` on processes derived from SystemVerilog style always blocks
+  according to the type of the always. These are checked for correctness in
+  ``proc_dlatch``.
+
 - In addition to the ``(* ... *)`` attribute syntax, Yosys supports
   the non-standard ``{* ... *}`` attribute syntax to set default attributes
   for everything that comes after the ``{* ... *}`` statement. (Reset
diff --git a/frontends/verilog/verilog_lexer.l b/frontends/verilog/verilog_lexer.l
index 4acfb414d..c8984c2c4 100644
--- a/frontends/verilog/verilog_lexer.l
+++ b/frontends/verilog/verilog_lexer.l
@@ -188,9 +188,9 @@ YOSYS_NAMESPACE_END
 "unique0"      { SV_KEYWORD(TOK_UNIQUE); }
 "priority"     { SV_KEYWORD(TOK_PRIORITY); }
 
-"always_comb"  { SV_KEYWORD(TOK_ALWAYS); }
-"always_ff"    { SV_KEYWORD(TOK_ALWAYS); }
-"always_latch" { SV_KEYWORD(TOK_ALWAYS); }
+"always_comb"  { SV_KEYWORD(TOK_ALWAYS_COMB); }
+"always_ff"    { SV_KEYWORD(TOK_ALWAYS_FF); }
+"always_latch" { SV_KEYWORD(TOK_ALWAYS_LATCH); }
 
  /* use special token for labels on assert, assume, cover, and restrict because it's insanley complex
     to fix parsing of cells otherwise. (the current cell parser forces a reduce very early to update some
diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y
index 77f6d2051..daea3b43a 100644
--- a/frontends/verilog/verilog_parser.y
+++ b/frontends/verilog/verilog_parser.y
@@ -141,6 +141,7 @@ struct specify_rise_fall {
 %token TOK_INTERFACE TOK_ENDINTERFACE TOK_MODPORT TOK_VAR
 %token TOK_INPUT TOK_OUTPUT TOK_INOUT TOK_WIRE TOK_WAND TOK_WOR TOK_REG TOK_LOGIC
 %token TOK_INTEGER TOK_SIGNED TOK_ASSIGN TOK_ALWAYS TOK_INITIAL
+%token TOK_ALWAYS_FF TOK_ALWAYS_COMB TOK_ALWAYS_LATCH
 %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_CASE TOK_CASEX TOK_CASEZ TOK_ENDCASE TOK_DEFAULT
@@ -156,7 +157,7 @@ struct specify_rise_fall {
 %type <ast> range range_or_multirange  non_opt_range non_opt_multirange range_or_signed_int
 %type <ast> wire_type expr basic_expr concat_list rvalue lvalue lvalue_concat_list
 %type <string> opt_label opt_sva_label tok_prim_wrapper hierarchical_id hierarchical_type_id
-%type <boolean> opt_signed opt_property unique_case_attr
+%type <boolean> opt_signed opt_property unique_case_attr always_comb_or_latch always_or_always_ff
 %type <al> attr case_attr
 
 %type <specify_target_ptr> specify_target
@@ -1581,10 +1582,28 @@ cell_port:
 		free_attr($1);
 	};
 
+always_comb_or_latch:
+	TOK_ALWAYS_COMB {
+		$$ = false;
+	} |
+	TOK_ALWAYS_LATCH {
+		$$ = true;
+	};
+
+always_or_always_ff:
+	TOK_ALWAYS {
+		$$ = false;
+	} |
+	TOK_ALWAYS_FF {
+		$$ = true;
+	};
+
 always_stmt:
-	attr TOK_ALWAYS {
+	attr always_or_always_ff {
 		AstNode *node = new AstNode(AST_ALWAYS);
 		append_attr(node, $1);
+		if ($2)
+			node->attributes[ID(always_ff)] = AstNode::mkconst_int(1, false);
 		ast_stack.back()->children.push_back(node);
 		ast_stack.push_back(node);
 	} always_cond {
@@ -1595,6 +1614,22 @@ always_stmt:
 		ast_stack.pop_back();
 		ast_stack.pop_back();
 	} |
+	attr always_comb_or_latch {
+		AstNode *node = new AstNode(AST_ALWAYS);
+		append_attr(node, $1);
+		if ($2)
+			node->attributes[ID(always_latch)] = AstNode::mkconst_int(1, false);
+		else
+			node->attributes[ID(always_comb)] = AstNode::mkconst_int(1, false);
+		ast_stack.back()->children.push_back(node);
+		ast_stack.push_back(node);
+		AstNode *block = new AstNode(AST_BLOCK);
+		ast_stack.back()->children.push_back(block);
+		ast_stack.push_back(block);
+	} behavioral_stmt {
+		ast_stack.pop_back();
+		ast_stack.pop_back();
+	} |
 	attr TOK_INITIAL {
 		AstNode *node = new AstNode(AST_INITIAL);
 		append_attr(node, $1);
diff --git a/passes/proc/proc_dlatch.cc b/passes/proc/proc_dlatch.cc
index d9d5dfbed..a0c8351b6 100644
--- a/passes/proc/proc_dlatch.cc
+++ b/passes/proc/proc_dlatch.cc
@@ -349,6 +349,10 @@ void proc_dlatch(proc_dlatch_db_t &db, RTLIL::Process *proc)
 			continue;
 		}
 
+		if (proc->get_bool_attribute(ID(always_ff)))
+			log_error("Found non edge/level sensitive event in always_ff process `%s.%s'.\n",
+					db.module->name.c_str(), proc->name.c_str());
+
 		for (auto ss : sr->actions)
 		{
 			db.sigmap.apply(ss.first);
@@ -383,8 +387,12 @@ void proc_dlatch(proc_dlatch_db_t &db, RTLIL::Process *proc)
 	int offset = 0;
 	for (auto chunk : nolatches_bits.first.chunks()) {
 		SigSpec lhs = chunk, rhs = nolatches_bits.second.extract(offset, chunk.width);
-		log("No latch inferred for signal `%s.%s' from process `%s.%s'.\n",
-				db.module->name.c_str(), log_signal(lhs), db.module->name.c_str(), proc->name.c_str());
+		if (proc->get_bool_attribute(ID(always_latch)))
+			log_error("No latch inferred for signal `%s.%s' from always_latch process `%s.%s'.\n",
+					db.module->name.c_str(), log_signal(lhs), db.module->name.c_str(), proc->name.c_str());
+		else
+			log("No latch inferred for signal `%s.%s' from process `%s.%s'.\n",
+					db.module->name.c_str(), log_signal(lhs), db.module->name.c_str(), proc->name.c_str());
 		db.module->connect(lhs, rhs);
 		offset += chunk.width;
 	}
@@ -410,8 +418,12 @@ void proc_dlatch(proc_dlatch_db_t &db, RTLIL::Process *proc)
 			cell->set_src_attribute(src);
 			db.generated_dlatches.insert(cell);
 
-			log("Latch inferred for signal `%s.%s' from process `%s.%s': %s\n",
-					db.module->name.c_str(), log_signal(lhs), db.module->name.c_str(), proc->name.c_str(), log_id(cell));
+			if (proc->get_bool_attribute(ID(always_comb)))
+				log_error("Latch inferred for signal `%s.%s' from always_comb process `%s.%s'.\n",
+						db.module->name.c_str(), log_signal(lhs), db.module->name.c_str(), proc->name.c_str());
+			else
+				log("Latch inferred for signal `%s.%s' from process `%s.%s': %s\n",
+						db.module->name.c_str(), log_signal(lhs), db.module->name.c_str(), proc->name.c_str(), log_id(cell));
 		}
 
 		offset += width;
diff --git a/tests/various/svalways.sh b/tests/various/svalways.sh
new file mode 100755
index 000000000..2cc09f801
--- /dev/null
+++ b/tests/various/svalways.sh
@@ -0,0 +1,63 @@
+#!/bin/bash
+
+trap 'echo "ERROR in svalways.sh" >&2; exit 1' ERR
+
+# Good case
+../../yosys -f "verilog -sv" -qp proc - <<EOT
+module top(input clk, en, d, output reg p, q, r);
+
+always_ff @(posedge clk)
+	p <= d;
+
+always_comb
+	q = ~d;
+
+always_latch
+	if (en) r = d;
+
+endmodule
+EOT
+
+# Incorrect always_comb syntax
+((../../yosys -f "verilog -sv" -qp proc -|| true) <<EOT
+module top(input d, output reg q);
+
+always_comb @(d)
+	q = ~d;
+
+endmodule
+EOT
+) 2>&1 | grep -F "<stdin>:3: ERROR: syntax error, unexpected '@'" > /dev/null
+
+# Incorrect use of always_comb
+((../../yosys -f "verilog -sv" -qp proc -|| true) <<EOT
+module top(input en, d, output reg q);
+
+always_comb
+	if (en) q = d;
+
+endmodule
+EOT
+) 2>&1 | grep -F "ERROR: Latch inferred for signal \`\\top.\\q' from always_comb process" > /dev/null
+
+# Incorrect use of always_latch
+((../../yosys -f "verilog -sv" -qp proc -|| true) <<EOT
+module top(input en, d, output reg q);
+
+always_latch
+	q = !d;
+
+endmodule
+EOT
+) 2>&1 | grep -F "ERROR: No latch inferred for signal \`\\top.\\q' from always_latch process" > /dev/null
+
+# Incorrect use of always_ff
+((../../yosys -f "verilog -sv" -qp proc -|| true) <<EOT
+module top(input en, d, output reg q);
+
+always_ff @(*)
+	q = !d;
+
+endmodule
+EOT
+) 2>&1 | grep -F "ERROR: Found non edge/level sensitive event in always_ff process" > /dev/null