Merge 1c73870f4d into 5d0847f6fb

Catch partial support of SVI

Makefile

@@ -95,14 +95,14 @@ TARGETS = $(PROGRAM_PREFIX)yosys$(EXE) $(PROGRAM_PREFIX)yosys-config
 PRETTY = 1
 SMALL = 0
 
-# Unit test
-UNITESTPATH := tests/unit
-
 all: top-all
 
 YOSYS_SRC := $(dir $(firstword $(MAKEFILE_LIST)))
 VPATH := $(YOSYS_SRC)
 
+# Unit test
+UNITESTPATH := $(YOSYS_SRC)/tests/unit
+
 export CXXSTD ?= c++17
 CXXFLAGS := $(CXXFLAGS) -Wall -Wextra -ggdb -I. -I"$(YOSYS_SRC)" -MD -MP -D_YOSYS_ -fPIC -I$(PREFIX)/include
 LIBS := $(LIBS) -lstdc++ -lm
@@ -161,7 +161,7 @@ ifeq ($(OS), Haiku)
 CXXFLAGS += -D_DEFAULT_SOURCE
 endif
 
-YOSYS_VER := 0.58+138
+YOSYS_VER := 0.58+162
 YOSYS_MAJOR := $(shell echo $(YOSYS_VER) | cut -d'.' -f1)
 YOSYS_MINOR := $(shell echo $(YOSYS_VER) | cut -d'.' -f2 | cut -d'+' -f1)
 YOSYS_COMMIT := $(shell echo $(YOSYS_VER) | cut -d'+' -f2)
@@ -1135,7 +1135,7 @@ DOC_TARGET ?= html
 docs: docs/prep
 	$(Q) $(MAKE) -C docs $(DOC_TARGET)
 
-clean: clean-py
+clean: clean-py clean-unit-test
 	rm -rf share
 	rm -f $(OBJS) $(GENFILES) $(TARGETS) $(EXTRA_TARGETS) $(EXTRA_OBJS)
 	rm -f kernel/version_*.o kernel/version_*.cc
@@ -1150,7 +1150,7 @@ clean: clean-py
 	rm -f tests/svinterfaces/*.log_stdout tests/svinterfaces/*.log_stderr tests/svinterfaces/dut_result.txt tests/svinterfaces/reference_result.txt tests/svinterfaces/a.out tests/svinterfaces/*_syn.v tests/svinterfaces/*.diff
 	rm -f  tests/tools/cmp_tbdata
 	rm -f $(addsuffix /run-test.mk,$(MK_TEST_DIRS))
-	-$(MAKE) -C docs clean
+	-$(MAKE) -C $(YOSYS_SRC)/docs clean
 	rm -rf docs/util/__pycache__
 	rm -f libyosys.so
 
@@ -1162,7 +1162,7 @@ clean-py:
 	rm -rf kernel/*.pyh
 
 clean-abc:
-	$(MAKE) -C abc DEP= clean
+	$(MAKE) -C $(YOSYS_SRC)/abc DEP= clean
 	rm -f $(PROGRAM_PREFIX)yosys-abc$(EXE) $(PROGRAM_PREFIX)yosys-libabc.a abc/abc-[0-9a-f]* abc/libabc-[0-9a-f]*.a .git-abc-submodule-hash
 
 mrproper: clean

abc

@@ -1 +1 @@
-Subproject commit fa186342baefea06e7c2aa13fe51f338ffc84912
+Subproject commit 1c5ed1ce378cc04beac30bb31abc4c37c8467042

docs/source/using_yosys/verilog.rst

@@ -9,7 +9,7 @@ Yosys and there are currently no plans to add support
 for them:
 
 - Non-synthesizable language features as defined in
-	IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002
+  IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002
 
 - The ``tri``, ``triand`` and ``trior`` net types
 
@@ -356,21 +356,29 @@ from SystemVerilog:
   files being read into the same design afterwards.
 
 - typedefs are supported (including inside packages)
-	- type casts are currently not supported
+
+  - type casts are currently not supported
 
 - enums are supported (including inside packages)
-	- but are currently not strongly typed
+
+  - but are currently not strongly typed
 
 - packed structs and unions are supported
-	- arrays of packed structs/unions are currently not supported
-	- structure literals are currently not supported
+
+  - arrays of packed structs/unions are currently not supported
+  - structure literals are currently not supported
 
 - multidimensional arrays are supported
-	- array assignment of unpacked arrays is currently not supported
-	- array literals are currently not supported
 
-- SystemVerilog interfaces (SVIs) are supported. Modports for specifying whether
-  ports are inputs or outputs are supported.
+  - array assignment of unpacked arrays is currently not supported
+  - array literals are currently not supported
+
+- SystemVerilog interfaces (SVIs), including modports for specifying whether
+  ports are inputs or outputs, are partially supported.
+
+  - interfaces must be provided as *named* arguments, not positional arguments.
+    i.e. ``foo bar(.intf(intf0), .x(x));`` is supported but ``foo bar(intf0,
+    x);`` is not.
 
 - Assignments within expressions are supported.
 

passes/hierarchy/hierarchy.cc

@@ -156,6 +156,21 @@ std::string basic_cell_type(const std::string celltype, int pos[3] = nullptr) {
 	return basicType;
 }
 
+// Try to read an IdString as a numbered connection name ("$123" or similar),
+// writing the result to dst. If the string isn't of the right format, ignore
+// dst and return false.
+bool read_id_num(RTLIL::IdString str, int *dst)
+{
+	log_assert(dst);
+
+	const char *c_str = str.c_str();
+	if (c_str[0] != '$' || !('0' <= c_str[1] && c_str[1] <= '9'))
+		return false;
+
+	*dst = atoi(c_str + 1);
+	return true;
+}
+
 // A helper struct for expanding a module's interface connections in expand_module
 struct IFExpander
 {
@@ -283,15 +298,42 @@ struct IFExpander
 	                   RTLIL::IdString       conn_name,
 	                   const RTLIL::SigSpec &conn_signals)
 	{
-		// Check if the connection is present as an interface in the sub-module's port list
-		const RTLIL::Wire *wire = submodule.wire(conn_name);
-		if (!wire || !wire->get_bool_attribute(ID::is_interface))
+		// Does the connection look like an interface
+		if (
+			conn_signals.size() != 1 ||
+			conn_signals[0].wire == nullptr ||
+			conn_signals[0].wire->get_bool_attribute(ID::is_interface) == false ||
+			conn_signals[0].wire->name.str().find("$dummywireforinterface") != 0
+		)
 			return;
 
+		// Check if the connection is present as an interface in the sub-module's port list
+		int id;
+		if (read_id_num(conn_name, &id)) {
+			/* Interface expansion is incompatible with positional arguments
+			 * during expansion, the port list gets each interface signal
+			 * inserted after the interface itself which means that the argument
+			 * positions in the parent module no longer match.
+			 *
+			 * Supporting this would require expanding the interfaces in the
+			 * parent module, renumbering the arguments to match, and then
+			 * iterating over the ports list to find the matching interface
+			 * (refactoring on_interface to accept different conn_names on the
+			 * parent and child).
+			 */
+			log_error("Unable to connect `%s' to submodule `%s' with positional interface argument `%s'!\n",
+				module.name,
+				submodule.name,
+				conn_signals[0].wire->name.str().substr(23)
+			);
+		} else {
+			// Lookup connection by name
+			const RTLIL::Wire *wire = submodule.wire(conn_name);
+			if (!wire || !wire->get_bool_attribute(ID::is_interface))
+				return;
+			}
 		// If the connection looks like an interface, handle it.
-		const auto &bits = conn_signals;
-		if (bits.size() == 1 && bits[0].wire->get_bool_attribute(ID::is_interface))
-			on_interface(submodule, conn_name, conn_signals);
+		on_interface(submodule, conn_name, conn_signals);
 	}
 
 	// Iterate over the connections in a cell, tracking any interface
@@ -376,21 +418,6 @@ RTLIL::Module *get_module(RTLIL::Design                  &design,
 	return nullptr;
 }
 
-// Try to read an IdString as a numbered connection name ("$123" or similar),
-// writing the result to dst. If the string isn't of the right format, ignore
-// dst and return false.
-bool read_id_num(RTLIL::IdString str, int *dst)
-{
-	log_assert(dst);
-
-	const char *c_str = str.c_str();
-	if (c_str[0] != '$' || !('0' <= c_str[1] && c_str[1] <= '9'))
-		return false;
-
-	*dst = atoi(c_str + 1);
-	return true;
-}
-
 // Check that the connections on the cell match those that are defined
 // on the type: each named connection should match the name of a port
 // and each positional connection should have an index smaller than

passes/techmap/dfflibmap.cc

@@ -127,7 +127,7 @@ static bool parse_next_state(const LibertyAst *cell, const LibertyAst *attr, std
 		return false;
 	}
 
-	auto pin_names = pool<std::string>{};
+	auto pin_names = std::unordered_set<std::string>{};
 	tree.get_pin_names(pin_names);
 
 	// from the `ff` block, we know the flop output signal name for loopback.
@@ -156,7 +156,7 @@ static bool parse_next_state(const LibertyAst *cell, const LibertyAst *attr, std
 		auto pins = std::vector<std::string>(pin_names.begin(), pin_names.end());
 		int lut = 0;
 		for (int n = 0; n < 8; n++) {
-			auto values = dict<std::string, bool>{};
+			auto values = std::unordered_map<std::string, bool>{};
 			values.insert(std::make_pair(pins[0], (n & 1) == 1));
 			values.insert(std::make_pair(pins[1], (n & 2) == 2));
 			values.insert(std::make_pair(ff_output, (n & 4) == 4));

passes/techmap/libparse.cc

@@ -25,9 +25,22 @@
 #include <fstream>
 #include <iostream>
 #include <sstream>
+#include <algorithm>
 
-#ifndef FILTERLIB
+#ifdef FILTERLIB
+#undef log_assert
+void log_assert(bool cond) {
+	if (!cond)
+		fprintf(stderr, "Unspecified assertion failed\n");
+}
+void warn(std::string str) {
+	std::cerr << str;
+}
+#else
 #include "kernel/log.h"
+void warn(std::string str) {
+	Yosys::log_formatted_warning("", str);
+}
 #endif
 
 using namespace Yosys;
@@ -162,13 +175,15 @@ void LibertyAst::dump(FILE *f, sieve &blacklist, sieve &whitelist, std::string i
 		fprintf(f, " ;\n");
 }
 
-#ifndef FILTERLIB
-
 // binary operators excluding ' '
-bool LibertyExpression::is_nice_binop(char c) {
+bool LibertyExpression::char_is_nice_binop(char c) {
 	return c == '*' || c == '&' || c == '^' || c == '+' || c == '|';
 }
 
+bool LibertyExpression::is_binop() {
+	return kind == AND || kind == OR || kind == XOR;
+}
+
 // https://matklad.github.io/2020/04/13/simple-but-powerful-pratt-parsing.html
 LibertyExpression LibertyExpression::parse(Lexer &s, int min_prio) {
 	if (s.empty())
@@ -177,7 +192,7 @@ LibertyExpression LibertyExpression::parse(Lexer &s, int min_prio) {
 	char c = s.peek();
 	auto lhs = LibertyExpression{};
 
-	while (isspace(c)) {
+	while (isspace(c) || c == '"') {
 		if (s.empty())
 			return lhs;
 		s.next();
@@ -191,7 +206,9 @@ LibertyExpression LibertyExpression::parse(Lexer &s, int min_prio) {
 		s.next();
 		lhs = parse(s);
 		if (s.peek() != ')') {
-			log_warning("expected ')' instead of '%c' while parsing Liberty expression '%s'\n", s.peek(), s.full_expr());
+			std::stringstream ss;
+			ss << "expected ')' instead of " << s.peek() << " while parsing Liberty expression '" << s.full_expr() << "'\n";
+			warn(ss.str());
 			return lhs;
 		}
 		s.next();
@@ -200,10 +217,11 @@ LibertyExpression LibertyExpression::parse(Lexer &s, int min_prio) {
 		lhs.kind = Kind::NOT;
 		lhs.children.push_back(parse(s, 7));
 	} else {
-		log_warning("unrecognised character '%c' while parsing Liberty expression '%s'\n", c, s.full_expr());
+		std::stringstream ss;
+		ss << "unrecognised character " << c << " while parsing Liberty expression " << s.full_expr() << "\n";
+		warn(ss.str());
 		return lhs;
 	}
-
 	while (true) {
 		if (s.empty())
 			break;
@@ -246,9 +264,10 @@ LibertyExpression LibertyExpression::parse(Lexer &s, int min_prio) {
 					s.next();
 					c = s.peek();
 				}
-				if (is_nice_binop(c)) {
+				if (char_is_nice_binop(c) || c == ')' || c == '\'' || c == '\"') {
 					// We found a real binop, so this space wasn't an AND
 					// and we just discard it as meaningless whitespace
+					// Tail operators also imply this isn't an AND
 					continue;
 				}
 			} else {
@@ -286,7 +305,7 @@ LibertyExpression LibertyExpression::parse(Lexer &s, int min_prio) {
 	return lhs;
 }
 
-void LibertyExpression::get_pin_names(pool<std::string>& names) {
+void LibertyExpression::get_pin_names(std::unordered_set<std::string>& names) {
 	if (kind == Kind::PIN) {
 		names.insert(name);
 	} else {
@@ -295,7 +314,7 @@ void LibertyExpression::get_pin_names(pool<std::string>& names) {
 	}
 }
 
-bool LibertyExpression::eval(dict<std::string, bool>& values) {
+bool LibertyExpression::eval(std::unordered_map<std::string, bool>& values) {
 	bool result = false;
 	switch (kind) {
 	case Kind::AND:
@@ -324,7 +343,7 @@ bool LibertyExpression::eval(dict<std::string, bool>& values) {
 	return false;
 }
 
-std::string LibertyExpression::str(int indent)
+std::string LibertyExpression::sexpr_str(int indent)
 {
 	std::string prefix;
 	switch (kind) {
@@ -355,16 +374,55 @@ std::string LibertyExpression::str(int indent)
 		if (!first) {
 			prefix += "\n" + std::string(indent + add_indent, ' ');
 		}
-		prefix += child.str(indent + add_indent);
+		prefix += child.sexpr_str(indent + add_indent);
 		first = false;
 	}
 	prefix += ")";
 	return prefix;
 }
 
-#endif
+std::string LibertyExpression::vlog_str()
+{
+	std::string prefix;
+	if (kind != PIN)
+		prefix += "(";
+	if (is_binop()) {
+		log_assert(children.size() == 2);
+		prefix += children[0].vlog_str();
+		switch (kind) {
+			case AND:
+				prefix += "&";
+				break;
+			case OR:
+				prefix += "|";
+				break;
+			case XOR:
+				prefix += "^";
+				break;
+			default:
+				log_assert(false);
+		}
+		prefix += children[1].vlog_str();
+	} else {
+		switch (kind) {
+			case NOT:
+				log_assert(children.size() == 1);
+				prefix += "~";
+				prefix += children[0].vlog_str();
+				break;
+			case PIN:
+				prefix += name;
+				break;
+			default:
+				log_assert(false);
+			}
+	}
+	if (kind != PIN)
+		prefix += ")";
+	return prefix;
+}
 
-int LibertyParser::lexer(std::string &str)
+int LibertyParser::lexer_inner(std::string &str)
 {
 	int c;
 
@@ -390,11 +448,9 @@ int LibertyParser::lexer(std::string &str)
 
 		if (str == "+" || str == "-") {
 			/* Single operator is not an identifier */
-			// fprintf(stderr, "LEX: char >>%s<<\n", str.c_str());
 			return str[0];
 		}
 		else {
-			// fprintf(stderr, "LEX: identifier >>%s<<\n", str.c_str());
 			return 'v';
 		}
 	}
@@ -402,24 +458,25 @@ int LibertyParser::lexer(std::string &str)
 	// if it wasn't an identifer, number of array range,
 	// maybe it's a string?
 	if (c == '"') {
+		f.consume(1);
 		size_t i = 0;
 		while (true) {
 			c = f.peek(i);
 			line += (c == '\n');
-			if (c != '"')
+			if (c != '"' && c != EOF)
 				i += 1;
 			else
 				break;
 		}
 		str.clear();
-#ifdef FILTERLIB
 		f.unget();
-		str.append(f.buffered_data(), f.buffered_data() + i + 2);
-		f.consume(i + 2);
-#else
-		str.append(f.buffered_data(), f.buffered_data() + i);
-		f.consume(i + 1);
+		str.append(f.buffered_data(), f.buffered_data() + i + 1);
+		// Usage in filterlib is expected to retain quotes
+		// but yosys expects to get unquoted
+#ifdef FILTERLIB
+		str = "\"" + str + "\"";
 #endif
+		f.consume(i + 2);
 		return 'v';
 	}
 
@@ -442,13 +499,12 @@ int LibertyParser::lexer(std::string &str)
 			return lexer(str);
 		}
 		f.unget();
-		// fprintf(stderr, "LEX: char >>/<<\n");
 		return '/';             // a single '/' charater.
 	}
 
 	// check for a backslash
 	if (c == '\\') {
-		c = f.get();		
+		c = f.get();
 		if (c == '\r')
 			c = f.get();
 		if (c == '\n') {
@@ -467,14 +523,22 @@ int LibertyParser::lexer(std::string &str)
 
 	// anything else, such as ';' will get passed
 	// through as literal items.
-
-	// if (c >= 32 && c < 255)
-	// 	fprintf(stderr, "LEX: char >>%c<<\n", c);
-	// else
-	// 	fprintf(stderr, "LEX: char %d\n", c);
 	return c;
 }
 
+int LibertyParser::lexer(std::string &str)
+{
+	int ret = lexer_inner(str);
+	// if (ret >= 32 && ret < 255) {
+	// 	fprintf(stdout, "LEX: ret >>%c<<\n", ret);
+	// } else if (ret == 'v') {
+	// 	fprintf(stdout, "LEX: ret v str %s\n", str.c_str());
+	// } else {
+	// 	fprintf(stdout, "LEX: ret %d\n", ret);
+	// }
+	return ret;
+}
+
 void LibertyParser::report_unexpected_token(int tok)
 {
 	std::string eReport;
@@ -545,6 +609,25 @@ void LibertyParser::parse_vector_range(int tok)
 	}
 }
 
+// Consume into out_str any string-ish tokens, seperated with spaces
+// to cope with abuse of the underdefined spec by real world PDKs
+// enabled by proprietary implementations.
+// Sorry.
+int LibertyParser::consume_wrecked_str(int tok, std::string& out_str) {
+	std::string str = "";
+	while (tok != ';' && tok != EOF && tok != 'n') {
+		out_str += " ";
+		if (tok == 'v')
+			out_str += str;
+		else
+			out_str += tok;
+		tok = lexer(str);
+	}
+	if (tok == EOF)
+		error("wrecked string EOF");
+	return tok;
+}
+
 LibertyAst *LibertyParser::parse(bool top_level)
 {
 	std::string str;
@@ -591,7 +674,14 @@ LibertyAst *LibertyParser::parse(bool top_level)
 				if (tok == '[') {
 					parse_vector_range(tok);
 					tok = lexer(str);
+				} else {
+					// Hack for when an expression string is unquoted
+					tok = consume_wrecked_str(tok, ast->value);
 				}
+			} else if (tok == '(') {
+				// Hack for when an expression string is unquoted and starts with
+				// parentheses
+				tok = consume_wrecked_str(tok, ast->value);
 			}
 			while (tok == '+' || tok == '-' || tok == '*' || tok == '/' || tok == '!') {
 				ast->value += tok;
@@ -601,7 +691,7 @@ LibertyAst *LibertyParser::parse(bool top_level)
 				ast->value += str;
 				tok = lexer(str);
 			}
-			
+
 			// In a liberty file, all key : value pairs should end in ';'
 			// However, there are some liberty files in the wild that
 			// just have a newline. We'll be kind and accept a newline
@@ -621,11 +711,11 @@ LibertyAst *LibertyParser::parse(bool top_level)
 					continue;
 				if (tok == ')')
 					break;
-				
+
 				if (tok == '[')
 				{
 					parse_vector_range(tok);
-					continue;           
+					continue;
 				}
 				if (tok == 'n')
 					continue;
@@ -727,42 +817,13 @@ const LibertyAst *find_non_null(const LibertyAst *node, const char *name)
 
 std::string func2vl(std::string str)
 {
-	for (size_t pos = str.find_first_of("\" \t"); pos != std::string::npos; pos = str.find_first_of("\" \t")) {
-		char c_left = pos > 0 ? str[pos-1] : ' ';
-		char c_right = pos+1 < str.size() ? str[pos+1] : ' ';
-		if (std::string("\" \t*+").find(c_left) != std::string::npos)
-			str.erase(pos, 1);
-		else if (std::string("\" \t*+").find(c_right) != std::string::npos)
-			str.erase(pos, 1);
-		else
-			str[pos] = '*';
-	}
-
-	std::vector<size_t> group_start;
-	for (size_t pos = 0; pos < str.size(); pos++) {
-		if (str[pos] == '(')
-			group_start.push_back(pos);
-		if (str[pos] == ')' && group_start.size() > 0) {
-			if (pos+1 < str.size() && str[pos+1] == '\'') {
-				std::string group = str.substr(group_start.back(), pos-group_start.back()+1);
-				str[group_start.back()] = '~';
-				str.replace(group_start.back()+1, group.size(), group);
-				pos++;
-			}
-			group_start.pop_back();
-		}
-		if (str[pos] == '\'' && pos > 0) {
-			size_t start = str.find_last_of("()'*+^&| ", pos-1)+1;
-			std::string group = str.substr(start, pos-start);
-			str[start] = '~';
-			str.replace(start+1, group.size(), group);
-		}
-		if (str[pos] == '*')
-			str[pos] = '&';
-		if (str[pos] == '+')
-			str[pos] = '|';
-	}
+	auto helper = LibertyExpression::Lexer(str);
+	return LibertyExpression::parse(helper).vlog_str();
+}
 
+std::string vlog_identifier(std::string str)
+{
+	str.erase(std::remove(str.begin(), str.end(), '\"'), str.end());
 	return str;
 }
 
@@ -772,11 +833,13 @@ void event2vl(const LibertyAst *ast, std::string &edge, std::string &expr)
 	expr.clear();
 
 	if (ast != NULL) {
-		expr = func2vl(ast->value);
-		if (expr.size() > 0 && expr[0] == '~')
-			edge = "negedge " + expr.substr(1);
+		auto helper = LibertyExpression::Lexer(ast->value);
+		auto parsed = LibertyExpression::parse(helper);
+		expr = parsed.vlog_str();
+		if (parsed.kind == LibertyExpression::Kind::NOT)
+			edge = "negedge " + parsed.children[0].vlog_str();
 		else
-			edge = "posedge " + expr;
+			edge = "posedge " + parsed.vlog_str();
 	}
 }
 
@@ -806,13 +869,13 @@ void gen_verilogsim_cell(const LibertyAst *ast)
 		return;
 
 	CHECK_NV(ast->args.size(), == 1);
-	printf("module %s (", ast->args[0].c_str());
+	printf("module %s (", vlog_identifier(ast->args[0]).c_str());
 	bool first = true;
 	for (auto child : ast->children) {
 		if (child->id != "pin")
 			continue;
 		CHECK_NV(child->args.size(), == 1);
-		printf("%s%s", first ? "" : ", ", child->args[0].c_str());
+		printf("%s%s", first ? "" : ", ", vlog_identifier(child->args[0]).c_str());
 		first = false;
 	}
 	printf(");\n");
@@ -823,7 +886,7 @@ void gen_verilogsim_cell(const LibertyAst *ast)
 		printf("  reg ");
 		first = true;
 		for (auto arg : child->args) {
-			printf("%s%s", first ? "" : ", ", arg.c_str());
+			printf("%s%s", first ? "" : ", ", vlog_identifier(arg).c_str());
 			first = false;
 		}
 		printf(";\n");
@@ -835,9 +898,10 @@ void gen_verilogsim_cell(const LibertyAst *ast)
 		CHECK_NV(child->args.size(), == 1);
 		const LibertyAst *dir = find_non_null(child, "direction");
 		const LibertyAst *func = child->find("function");
-		printf("  %s %s;\n", dir->value.c_str(), child->args[0].c_str());
+		std::string var = vlog_identifier(child->args[0]);
+		printf("  %s %s;\n", dir->value.c_str(), var.c_str());
 		if (func != NULL)
-			printf("  assign %s = %s; // %s\n", child->args[0].c_str(), func2vl(func->value).c_str(), func->value.c_str());
+			printf("  assign %s = %s; // %s\n", var.c_str(), func2vl(func->value).c_str(), func->value.c_str());
 	}
 
 	for (auto child : ast->children)
@@ -845,8 +909,8 @@ void gen_verilogsim_cell(const LibertyAst *ast)
 		if (child->id != "ff" || child->args.size() != 2)
 			continue;
 
-		std::string iq_var = child->args[0];
-		std::string iqn_var = child->args[1];
+		std::string iq_var = vlog_identifier(child->args[0]);
+		std::string iqn_var = vlog_identifier(child->args[1]);
 
 		std::string clock_edge, clock_expr;
 		event2vl(child->find("clocked_on"), clock_edge, clock_expr);
@@ -909,8 +973,8 @@ void gen_verilogsim_cell(const LibertyAst *ast)
 		if (child->id != "latch" || child->args.size() != 2)
 			continue;
 
-		std::string iq_var = child->args[0];
-		std::string iqn_var = child->args[1];
+		std::string iq_var = vlog_identifier(child->args[0]);
+		std::string iqn_var = vlog_identifier(child->args[1]);
 
 		std::string enable_edge, enable_expr;
 		event2vl(child->find("enable"), enable_edge, enable_expr);

passes/techmap/libparse.h

@@ -63,7 +63,7 @@ namespace Yosys
 			}
 
 			std::string pin() {
-				auto length = s.find_first_of("\t()'!^*& +|");
+				auto length = s.find_first_of("\t()'!^*& +|\"");
 				if (length == std::string::npos) {
 					// nothing found so use size of s
 					length = s.size();
@@ -91,11 +91,13 @@ namespace Yosys
 		LibertyExpression() : kind(Kind::EMPTY) {}
 
 		static LibertyExpression parse(Lexer &s, int min_prio = 0);
-		void get_pin_names(pool<std::string>& names);
-		bool eval(dict<std::string, bool>& values);
-		std::string str(int indent = 0);
+		void get_pin_names(std::unordered_set<std::string>& names);
+		bool eval(std::unordered_map<std::string, bool>& values);
+		std::string sexpr_str(int indent = 0);
+		std::string vlog_str();
 	private:
-		static bool is_nice_binop(char c);
+		static bool char_is_nice_binop(char c);
+		bool is_binop();
 	};
 
 	class LibertyInputStream {
@@ -170,10 +172,12 @@ namespace Yosys
 		   'n': newline
 		   anything else is a single character.
 		*/
+		int lexer_inner(std::string &str);
 		int lexer(std::string &str);
 
 		void report_unexpected_token(int tok);
 		void parse_vector_range(int tok);
+		int consume_wrecked_str(int tok, std::string& out_str);
 		LibertyAst *parse(bool top_level);
 		void error() const;
 		void error(const std::string &str) const;

tests/liberty/XNOR2X1.lib.verilogsim.ok

@@ -2,5 +2,5 @@ module XNOR2X1 (B, A, Y);
   input B;
   input A;
   output Y;
-  assign Y = !(B&!A|!B&A); // "!(B&!A|!B&A)"
+  assign Y = (~((B&(~A))|((~B)&A))); // "!(B&!A|!B&A)"
 endmodule

tests/liberty/dff.lib

@@ -5,7 +5,7 @@ library(dff) {
     area : 1;
     ff("IQ", "IQN") {
       next_state : "(D)";
-      clocked_on : "CLK";
+      clocked_on : (CLK);
     } 
     pin(D) {
       direction : input;
@@ -15,7 +15,7 @@ library(dff) {
     }
     pin(Q) {
       direction: output;
-      function : "IQ";   
+      function : IQ;   
     }
   }
 

tests/liberty/dff.lib.filtered.ok

@@ -3,7 +3,7 @@ library(dff) {
     area : 1 ;
     ff("IQ", "IQN") {
       next_state : "(D)" ;
-      clocked_on : "CLK" ;
+      clocked_on :  ( CLK ) ;
     }
     pin(D) {
       direction : input ;
@@ -13,7 +13,7 @@ library(dff) {
     }
     pin(Q) {
       direction : output ;
-      function : "IQ" ;
+      function : IQ ;
     }
   }
 }

tests/liberty/dff.lib.verilogsim.ok

@@ -1,12 +1,12 @@
 module dff (D, CLK, Q);
-  reg "IQ", "IQN";
+  reg IQ, IQN;
   input D;
   input CLK;
   output Q;
-  assign Q = IQ; // "IQ"
+  assign Q = IQ; // IQ
   always @(posedge CLK) begin
       // "(D)"
-      "IQ" <= (D);
-      "IQN" <= ~((D));
+      IQ <= D;
+      IQN <= ~(D);
   end
 endmodule

tests/liberty/normal.lib.verilogsim.ok

@@ -1,13 +1,13 @@
 module inv (A, Y);
   input A;
   output Y;
-  assign Y = ~A; // "A'"
+  assign Y = (~A); // "A'"
 endmodule
 module tri_inv (A, S, Z);
   input A;
   input S;
   output Z;
-  assign Z = ~A; // "A'"
+  assign Z = (~A); // "A'"
 endmodule
 module buffer (A, Y);
   input A;
@@ -18,29 +18,29 @@ module nand2 (A, B, Y);
   input A;
   input B;
   output Y;
-  assign Y = ~(A&B); // "(A * B)'"
+  assign Y = (~(A&B)); // "(A * B)'"
 endmodule
 module nor2 (A, B, Y);
   input A;
   input B;
   output Y;
-  assign Y = ~(A|B); // "(A + B)'"
+  assign Y = (~(A|B)); // "(A + B)'"
 endmodule
 module xor2 (A, B, Y);
   input A;
   input B;
   output Y;
-  assign Y = (A&~B)|(~A&B); // "(A *B') + (A' * B)"
+  assign Y = ((A&(~B))|((~A)&B)); // "(A *B') + (A' * B)"
 endmodule
 module imux2 (A, B, S, Y);
   input A;
   input B;
   input S;
   output Y;
-  assign Y = ~(&(A&S)|(B&~S)&); // "( (A * S) + (B * S') )'"
+  assign Y = (~((A&S)|(B&(~S)))); // "( (A * S) + (B * S') )'"
 endmodule
 module dff (D, CLK, RESET, PRESET, Q, QN);
-  reg "IQ", "IQN";
+  reg IQ, IQN;
   input D;
   input CLK;
   input RESET;
@@ -51,26 +51,26 @@ module dff (D, CLK, RESET, PRESET, Q, QN);
   assign QN = IQN; // "IQN"
   always @(posedge CLK, posedge RESET, posedge PRESET) begin
     if ((RESET) && (PRESET)) begin
-      "IQ" <= 0;
-      "IQN" <= 0;
+      IQ <= 0;
+      IQN <= 0;
     end
     else if (RESET) begin
-      "IQ" <= 0;
-      "IQN" <= 1;
+      IQ <= 0;
+      IQN <= 1;
     end
     else if (PRESET) begin
-      "IQ" <= 1;
-      "IQN" <= 0;
+      IQ <= 1;
+      IQN <= 0;
     end
     else begin
       // "D"
-      "IQ" <= D;
-      "IQN" <= ~(D);
+      IQ <= D;
+      IQN <= ~(D);
     end
   end
 endmodule
 module latch (D, G, Q, QN);
-  reg "IQ", "IQN";
+  reg IQ, IQN;
   input D;
   input G;
   output Q;
@@ -79,8 +79,8 @@ module latch (D, G, Q, QN);
   assign QN = IQN; // "IQN"
   always @* begin
     if (G) begin
-      "IQ" <= D;
-      "IQN" <= ~(D);
+      IQ <= D;
+      IQN <= ~(D);
     end
   end
 endmodule
@@ -89,14 +89,14 @@ module aoi211 (A, B, C, Y);
   input B;
   input C;
   output Y;
-  assign Y = ~((A&B)|C); // "((A * B) + C)'"
+  assign Y = (~((A&B)|C)); // "((A * B) + C)'"
 endmodule
 module oai211 (A, B, C, Y);
   input A;
   input B;
   input C;
   output Y;
-  assign Y = ~((A|B)&C); // "((A + B) * C)'"
+  assign Y = (~((A|B)&C)); // "((A + B) * C)'"
 endmodule
 module halfadder (A, B, C, Y);
   input A;
@@ -104,7 +104,7 @@ module halfadder (A, B, C, Y);
   output C;
   assign C = (A&B); // "(A * B)"
   output Y;
-  assign Y = (A&~B)|(~A&B); // "(A *B') + (A' * B)"
+  assign Y = ((A&(~B))|((~A)&B)); // "(A *B') + (A' * B)"
 endmodule
 module fulladder (A, B, CI, CO, Y);
   input A;

tests/liberty/unquoted.lib

@@ -0,0 +1,60 @@
+library(dff_unquoted) {
+  cell (dff1) {
+    area : 1;
+    ff("IQ", "IQN") {
+      next_state : !D;
+      clocked_on : (CLK);
+    }
+    pin(D) {
+      direction : input;
+    }
+    pin(CLK) {
+      direction : input;
+    }
+    pin(Q) {
+      direction: output;
+      function : IQ;
+    }
+  }
+  cell (dff2) {
+    area : 1;
+    ff(IQ, IQN) {
+      next_state : D';
+      clocked_on : CLK;
+    }
+    pin(D) {
+      direction : input;
+    }
+    pin(CLK) {
+      direction : input;
+    }
+    pin(Q) {
+      direction: output;
+      function : "IQ";
+    }
+  }
+  cell (dffe) {
+    area : 6;
+    ff("IQ", "IQN") {
+      next_state : (D&EN) | (IQ&!EN);
+      clocked_on : !CLK;
+    }
+    pin(D) {
+      direction : input;
+    }
+    pin(EN) {
+      direction : input;
+    }
+    pin(CLK) {
+      direction : input;
+    }
+    pin(Q) {
+      direction: output;
+      function : "IQ";
+    }
+    pin(QN) {
+      direction: output;
+      function : "IQN";
+    }
+  }
+}

tests/liberty/unquoted.lib.filtered.ok

@@ -0,0 +1,60 @@
+library(dff_unquoted) {
+  cell(dff1) {
+    area : 1 ;
+    ff("IQ", "IQN") {
+      next_state : !D ;
+      clocked_on :  ( CLK ) ;
+    }
+    pin(D) {
+      direction : input ;
+    }
+    pin(CLK) {
+      direction : input ;
+    }
+    pin(Q) {
+      direction : output ;
+      function : IQ ;
+    }
+  }
+  cell(dff2) {
+    area : 1 ;
+    ff(IQ, IQN) {
+      next_state : D ' ;
+      clocked_on : CLK ;
+    }
+    pin(D) {
+      direction : input ;
+    }
+    pin(CLK) {
+      direction : input ;
+    }
+    pin(Q) {
+      direction : output ;
+      function : "IQ" ;
+    }
+  }
+  cell(dffe) {
+    area : 6 ;
+    ff("IQ", "IQN") {
+      next_state :  ( D & EN ) | ( IQ & ! EN ) ;
+      clocked_on : !CLK ;
+    }
+    pin(D) {
+      direction : input ;
+    }
+    pin(EN) {
+      direction : input ;
+    }
+    pin(CLK) {
+      direction : input ;
+    }
+    pin(Q) {
+      direction : output ;
+      function : "IQ" ;
+    }
+    pin(QN) {
+      direction : output ;
+      function : "IQN" ;
+    }
+  }
+}

tests/liberty/unquoted.lib.verilogsim.ok

@@ -0,0 +1,39 @@
+module dff1 (D, CLK, Q);
+  reg IQ, IQN;
+  input D;
+  input CLK;
+  output Q;
+  assign Q = IQ; // IQ
+  always @(posedge CLK) begin
+      // !D
+      IQ <= (~D);
+      IQN <= ~((~D));
+  end
+endmodule
+module dff2 (D, CLK, Q);
+  reg IQ, IQN;
+  input D;
+  input CLK;
+  output Q;
+  assign Q = IQ; // "IQ"
+  always @(posedge CLK) begin
+      // D '
+      IQ <= (~D);
+      IQN <= ~((~D));
+  end
+endmodule
+module dffe (D, EN, CLK, Q, QN);
+  reg IQ, IQN;
+  input D;
+  input EN;
+  input CLK;
+  output Q;
+  assign Q = IQ; // "IQ"
+  output QN;
+  assign QN = IQN; // "IQN"
+  always @(negedge CLK) begin
+      //  ( D & EN ) | ( IQ & ! EN )
+      IQ <= ((D&EN)|(IQ&(~EN)));
+      IQN <= ~(((D&EN)|(IQ&(~EN))));
+  end
+endmodule

tests/svinterfaces/positional_args.ys

@@ -0,0 +1,33 @@
+read_verilog -sv << EOF
+interface simple_if;
+    logic receiver;
+    logic driver;
+endinterface
+
+module driver_mod(simple_if intf, input in);
+    assign intf.driver = in;
+endmodule
+
+module receiver_mod(simple_if intf);
+    assign intf.receiver = intf.driver;
+endmodule
+
+module top(
+    input logic [1:0] inputs,
+    output logic [1:0] outputs
+);
+    simple_if intf0();
+    simple_if intf1();
+
+    driver_mod d0(intf0, inputs[0]);
+    driver_mod d1(intf1, inputs[1]);
+
+    receiver_mod r0(intf0);
+    receiver_mod r1(intf1);
+
+    assign outputs = {intf0.receiver, intf1.receiver};
+endmodule
+EOF
+
+logger -expect error "Unable to connect.* with positional interface" 1
+hierarchy -top top

tests/svinterfaces/run-test.sh

@@ -5,3 +5,4 @@
 
 ./run_simple.sh load_and_derive
 ./run_simple.sh resolve_types
+./run_simple.sh positional_args

tests/unit/Makefile

@@ -30,11 +30,11 @@ TESTS := $(addprefix $(BINTEST)/, $(basename $(ALLTESTFILE:%Test.cc=%Test.o)))
 
 all: prepare $(TESTS) run-tests
 
-$(BINTEST)/%: $(OBJTEST)/%.o
+$(BINTEST)/%: $(OBJTEST)/%.o | prepare
 	$(CXX) -L$(ROOTPATH) $(RPATH) $(LINKFLAGS) -o $@ $^ $(LIBS) \
 		$(GTEST_LDFLAGS) $(EXTRAFLAGS)
 
-$(OBJTEST)/%.o: $(basename $(subst $(OBJTEST),.,%)).cc
+$(OBJTEST)/%.o: $(basename $(subst $(OBJTEST),.,%)).cc | prepare
 	$(CXX) -o $@ -c -I$(ROOTPATH) $(CPPFLAGS) $(CXXFLAGS) $(GTEST_CXXFLAGS) $^
 
 .PHONY: prepare run-tests clean

tests/unit/techmap/libparseTest.cc

@@ -13,7 +13,7 @@ namespace RTLIL {
 		void checkAll(std::initializer_list<std::string> expressions, std::string expected) {
 			for (const auto& e : expressions) {
 				auto helper = LibertyExpression::Lexer(e);
-				auto tree_s = LibertyExpression::parse(helper).str();
+				auto tree_s = LibertyExpression::parse(helper).sexpr_str();
 				EXPECT_EQ(tree_s, expected);
 			}
 		}
@@ -82,6 +82,11 @@ namespace RTLIL {
 		}, "(and (pin \"x\")\n"
 		   "     (not (pin \"y\")))"
 		);
+		checkAll({
+			"( D & EN )",
+		}, "(and (pin \"D\")\n"
+		   "     (pin \"EN\"))"
+		);
 	}
 }