Merge pull request #2041 from PeterCrozier/struct

Implementation of  SV structs.

README.md

@@ -556,6 +556,8 @@ from SystemVerilog:
 - enums are supported (including inside packages)
 	- but are currently not strongly typed
 
+- packed structs and unions are supported.
+
 - SystemVerilog interfaces (SVIs) are supported. Modports for specifying whether
   ports are inputs or outputs are supported.
 

frontends/ast/ast.cc

@@ -171,6 +171,9 @@ std::string AST::type2str(AstNodeType type)
 	X(AST_PACKAGE)
 	X(AST_WIRETYPE)
 	X(AST_TYPEDEF)
+	X(AST_STRUCT)
+	X(AST_UNION)
+	X(AST_STRUCT_ITEM)
 #undef X
 	default:
 		log_abort();

frontends/ast/ast.h

@@ -143,7 +143,7 @@ namespace AST
 		AST_GENCASE,
 		AST_GENBLOCK,
 		AST_TECALL,
-		
+
 		AST_POSEDGE,
 		AST_NEGEDGE,
 		AST_EDGE,
@@ -156,7 +156,10 @@ namespace AST
 		AST_PACKAGE,
 
 		AST_WIRETYPE,
-		AST_TYPEDEF
+		AST_TYPEDEF,
+		AST_STRUCT,
+		AST_UNION,
+		AST_STRUCT_ITEM
 	};
 
 	struct AstSrcLocType {
@@ -306,6 +309,7 @@ namespace AST
 
 		// helpers for enum
 		void allocateDefaultEnumValues();
+		void annotateTypedEnums(AstNode *template_node);
 	};
 
 	// process an AST tree (ast must point to an AST_DESIGN node) and generate RTLIL code

frontends/ast/genrtlil.cc

@@ -991,6 +991,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 	case AST_MODPORT:
 	case AST_MODPORTMEMBER:
 	case AST_TYPEDEF:
+	case AST_STRUCT:
+	case AST_UNION:
 		break;
 	case AST_INTERFACEPORT: {
 		// If a port in a module with unknown type is found, mark it with the attribute 'is_interface'

frontends/ast/simplify.cc

@@ -168,6 +168,197 @@ std::string AstNode::process_format_str(const std::string &sformat, int next_arg
 }
 
 
+void AstNode::annotateTypedEnums(AstNode *template_node)
+{
+	//check if enum
+	if (template_node->attributes.count(ID::enum_type)) {
+		//get reference to enum node:
+		std::string enum_type = template_node->attributes[ID::enum_type]->str.c_str();
+		//			log("enum_type=%s (count=%lu)\n", enum_type.c_str(), current_scope.count(enum_type));
+		//			log("current scope:\n");
+		//			for (auto &it : current_scope)
+		//				log("  %s\n", it.first.c_str());
+		log_assert(current_scope.count(enum_type) == 1);
+		AstNode *enum_node = current_scope.at(enum_type);
+		log_assert(enum_node->type == AST_ENUM);
+		//get width from 1st enum item:
+		log_assert(enum_node->children.size() >= 1);
+		AstNode *enum_item0 = enum_node->children[0];
+		log_assert(enum_item0->type == AST_ENUM_ITEM);
+		int width;
+		if (!enum_item0->range_valid)
+			width = 1;
+		else if (enum_item0->range_swapped)
+			width = enum_item0->range_right - enum_item0->range_left + 1;
+		else
+			width = enum_item0->range_left - enum_item0->range_right + 1;
+		log_assert(width > 0);
+		//add declared enum items:
+		for (auto enum_item : enum_node->children){
+			log_assert(enum_item->type == AST_ENUM_ITEM);
+			//get is_signed
+			bool is_signed;
+			if (enum_item->children.size() == 1){
+				is_signed = false;
+			} else if (enum_item->children.size() == 2){
+				log_assert(enum_item->children[1]->type == AST_RANGE);
+				is_signed = enum_item->children[1]->is_signed;
+			} else {
+				log_error("enum_item children size==%lu, expected 1 or 2 for %s (%s)\n",
+						  enum_item->children.size(),
+						  enum_item->str.c_str(), enum_node->str.c_str()
+				);
+			}
+			//start building attribute string
+			std::string enum_item_str = "\\enum_value_";
+			//get enum item value
+			if(enum_item->children[0]->type != AST_CONSTANT){
+				log_error("expected const, got %s for %s (%s)\n",
+						  type2str(enum_item->children[0]->type).c_str(),
+						  enum_item->str.c_str(), enum_node->str.c_str()
+						);
+			}
+			RTLIL::Const val = enum_item->children[0]->bitsAsConst(width, is_signed);
+			enum_item_str.append(val.as_string());
+			//set attribute for available val to enum item name mappings
+			attributes[enum_item_str.c_str()] = mkconst_str(enum_item->str);
+		}
+	}
+}
+
+static bool name_has_dot(const std::string &name, std::string &struct_name)
+{
+	// check if plausible struct member name \sss.mmm
+	std::string::size_type pos;
+	if (name.substr(0, 1) == "\\" && (pos = name.find('.', 0)) != std::string::npos) {
+		struct_name = name.substr(0, pos);
+		return true;
+	}
+	return false;
+}
+
+static AstNode *make_range(int left, int right, bool is_signed = false)
+{
+	// generate a pre-validated range node for a fixed signal range.
+	auto range = new AstNode(AST_RANGE);
+	range->range_left = left;
+	range->range_right = right;
+	range->range_valid = true;
+	range->children.push_back(AstNode::mkconst_int(left, true));
+	range->children.push_back(AstNode::mkconst_int(right, true));
+	range->is_signed = is_signed;
+	return range;
+}
+
+int size_packed_struct(AstNode *snode, int base_offset)
+{
+	// Struct members will be laid out in the structure contiguously from left to right.
+	// Union members all have zero offset from the start of the union.
+	// Determine total packed size and assign offsets.  Store these in the member node.
+	bool is_union = (snode->type == AST_UNION);
+	int offset = 0;
+	int packed_width = -1;
+	// examine members from last to first
+	for (auto it = snode->children.rbegin(); it != snode->children.rend(); ++it) {
+		auto node = *it;
+		int width;
+		if (node->type == AST_STRUCT || node->type == AST_UNION) {
+			// embedded struct or union
+			width = size_packed_struct(node, base_offset + offset);
+		}
+		else {
+			log_assert(node->type == AST_STRUCT_ITEM);
+			if (node->children.size() == 1 && node->children[0]->type == AST_RANGE) {
+				auto rnode = node->children[0];
+				width = (rnode->range_swapped ? rnode->range_right - rnode->range_left :
+								rnode->range_left - rnode->range_right) + 1;
+				// range nodes are now redundant
+				node->children.clear();
+			}
+			else if (node->range_left < 0) {
+				// 1 bit signal: bit, logic or reg
+				width = 1;
+			}
+			else {
+				// already resolved and compacted
+				width = node->range_left - node->range_right + 1;
+			}
+			if (is_union) {
+				node->range_right = base_offset;
+				node->range_left = base_offset + width - 1;
+			}
+			else {
+				node->range_right = base_offset + offset;
+				node->range_left = base_offset + offset + width - 1;
+			}
+			node->range_valid = true;
+		}
+		if (is_union) {
+			// check that all members have the same size
+			if (packed_width == -1) {
+				// first member
+				packed_width = width;
+			}
+			else {
+				if (packed_width != width) {
+
+					log_file_error(node->filename, node->location.first_line, "member %s of a packed union has %d bits, expecting %d\n", node->str.c_str(), width, packed_width);
+				}
+			}
+		}
+		else {
+			offset += width;
+		}
+	}
+	return (is_union ? packed_width : offset);
+}
+
+static void add_members_to_scope(AstNode *snode, std::string name)
+{
+	// add all the members in a struct or union to local scope
+	// in case later referenced in assignments
+	log_assert(snode->type==AST_STRUCT || snode->type==AST_UNION);
+	for (auto *node : snode->children) {
+		if (node->type != AST_STRUCT_ITEM) {
+			// embedded struct or union
+			add_members_to_scope(node, name + "." + node->str);
+		}
+		else {
+			auto member_name = name + "." + node->str;
+			current_scope[member_name] = node;
+		}
+	}
+}
+
+static int get_max_offset(AstNode *node)
+{
+	// get the width from the MS member in the struct
+	// as members are laid out from left to right in the packed wire
+	log_assert(node->type==AST_STRUCT || node->type==AST_UNION);
+	while (node->type != AST_STRUCT_ITEM) {
+		node = node->children[0];
+	}
+	return node->range_left;
+}
+
+static AstNode *make_packed_struct(AstNode *template_node, std::string &name)
+{
+	// create a wire for the packed struct
+	auto wnode = new AstNode(AST_WIRE);
+	wnode->str = name;
+	wnode->is_logic = true;
+	wnode->range_valid = true;
+	wnode->is_signed = template_node->is_signed;
+	int offset = get_max_offset(template_node);
+	auto range = make_range(offset, 0);
+	wnode->children.push_back(range);
+	// make sure this node is the one in scope for this name
+	current_scope[name] = wnode;
+	// add all the struct members to scope under the wire's name
+	add_members_to_scope(template_node, name);
+	return wnode;
+}
+
 // convert the AST into a simpler AST that has all parameters substituted by their
 // values, unrolled for-loops, expanded generate blocks, etc. when this function
 // is done with an AST it can be converted into RTLIL using genRTLIL().
@@ -567,6 +758,32 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 		}
 		break;
 
+	case AST_STRUCT:
+	case AST_UNION:
+		if (!basic_prep) {
+			for (auto *node : children) {
+				// resolve any ranges
+				while (!node->basic_prep && node->simplify(true, false, false, stage, -1, false, false)) {
+					did_something = true;
+				}
+			}
+			// determine member offsets and widths
+			size_packed_struct(this, 0);
+
+			// instance rather than just a type in a typedef or outer struct?
+			if (!str.empty() && str[0] == '\\') {
+				// instance so add a wire for the packed structure
+				auto wnode = make_packed_struct(this, str);
+				log_assert(current_ast_mod);
+				current_ast_mod->children.push_back(wnode);
+			}
+			basic_prep = true;
+		}
+		break;
+
+	case AST_STRUCT_ITEM:
+		break;
+
 	case AST_ENUM:
 		//log("\nENUM %s: %d child %d\n", str.c_str(), basic_prep, children[0]->basic_prep);
 		if (!basic_prep) {
@@ -884,10 +1101,12 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 	// resolve typedefs
 	if (type == AST_TYPEDEF) {
 		log_assert(children.size() == 1);
-		log_assert(children[0]->type == AST_WIRE || children[0]->type == AST_MEMORY);
+		auto type_node = children[0];
-		while(children[0]->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param))
+		log_assert(type_node->type == AST_WIRE || type_node->type == AST_MEMORY || type_node->type == AST_STRUCT || type_node->type == AST_UNION);
+		while (type_node->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {
 			did_something = true;
-		log_assert(!children[0]->is_custom_type);
+		}
+		log_assert(!type_node->is_custom_type);
 	}
 
 	// resolve types of wires
@@ -895,100 +1114,57 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 		if (is_custom_type) {
 			log_assert(children.size() >= 1);
 			log_assert(children[0]->type == AST_WIRETYPE);
-			if (!current_scope.count(children[0]->str))
+			auto type_name = children[0]->str;
-				log_file_error(filename, location.first_line, "Unknown identifier `%s' used as type name\n", children[0]->str.c_str());
+			if (!current_scope.count(type_name)) {
-			AstNode *resolved_type = current_scope.at(children[0]->str);
+				log_file_error(filename, location.first_line, "Unknown identifier `%s' used as type name\n", type_name.c_str());
-			if (resolved_type->type != AST_TYPEDEF)
+			}
-				log_file_error(filename, location.first_line, "`%s' does not name a type\n", children[0]->str.c_str());
+			AstNode *resolved_type_node = current_scope.at(type_name);
-			log_assert(resolved_type->children.size() == 1);
+			if (resolved_type_node->type != AST_TYPEDEF)
-			AstNode *templ = resolved_type->children[0];
+				log_file_error(filename, location.first_line, "`%s' does not name a type\n", type_name.c_str());
+			log_assert(resolved_type_node->children.size() == 1);
+			AstNode *template_node = resolved_type_node->children[0];
+
+			// Ensure typedef itself is fully simplified
+			while (template_node->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {};
+
+			if (template_node->type == AST_STRUCT || template_node->type == AST_UNION) {
+				// replace with wire representing the packed structure
+				newNode = make_packed_struct(template_node, str);
+				current_scope[str] = this;
+				goto apply_newNode;
+			}
+
 			// Remove type reference
 			delete children[0];
 			children.erase(children.begin());
 
-			// Ensure typedef itself is fully simplified
-			while(templ->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {};
-
 			if (type == AST_WIRE)
-				type = templ->type;
+				type = template_node->type;
-			is_reg = templ->is_reg;
+			is_reg = template_node->is_reg;
-			is_logic = templ->is_logic;
+			is_logic = template_node->is_logic;
-			is_signed = templ->is_signed;
+			is_signed = template_node->is_signed;
-			is_string = templ->is_string;
+			is_string = template_node->is_string;
-			is_custom_type = templ->is_custom_type;
+			is_custom_type = template_node->is_custom_type;
 
-			range_valid = templ->range_valid;
+			range_valid = template_node->range_valid;
-			range_swapped = templ->range_swapped;
+			range_swapped = template_node->range_swapped;
-			range_left = templ->range_left;
+			range_left = template_node->range_left;
-			range_right = templ->range_right;
+			range_right = template_node->range_right;
-			attributes[ID::wiretype] = mkconst_str(resolved_type->str);
+
-			//check if enum
+			attributes[ID::wiretype] = mkconst_str(resolved_type_node->str);
-			if (templ->attributes.count(ID::enum_type)){
+
-				//get reference to enum node:
+			// if an enum then add attributes to support simulator tracing
-				const std::string &enum_type = templ->attributes[ID::enum_type]->str;
+			annotateTypedEnums(template_node);
-				// 				log("enum_type=%s (count=%lu)\n", enum_type.c_str(), current_scope.count(enum_type));
-				// 				log("current scope:\n");
-				// 				for (auto &it : current_scope)
-				// 					log("  %s\n", it.first.c_str());
-				log_assert(current_scope.count(enum_type) == 1);
-				AstNode *enum_node = current_scope.at(enum_type);
-				log_assert(enum_node->type == AST_ENUM);
-				//get width from 1st enum item:
-				log_assert(enum_node->children.size() >= 1);
-				AstNode *enum_item0 = enum_node->children[0];
-				log_assert(enum_item0->type == AST_ENUM_ITEM);
-				int width;
-				if (!enum_item0->range_valid)
-					width = 1;
-				else if (enum_item0->range_swapped)
-					width = enum_item0->range_right - enum_item0->range_left + 1;
-				else
-					width = enum_item0->range_left - enum_item0->range_right + 1;
-				log_assert(width > 0);
-				//add declared enum items:
-				for (auto enum_item : enum_node->children){
-					log_assert(enum_item->type == AST_ENUM_ITEM);
-					//get is_signed
-					bool is_signed;
-					if (enum_item->children.size() == 1){
-						is_signed = false;
-					} else if (enum_item->children.size() == 2){
-						log_assert(enum_item->children[1]->type == AST_RANGE);
-						is_signed = enum_item->children[1]->is_signed;
-					} else {
-						log_error("enum_item children size==%lu, expected 1 or 2 for %s (%s)\n",
-								  enum_item->children.size(),
-								  enum_item->str.c_str(), enum_node->str.c_str()
-						);
-					}
-					//start building attribute string
-					std::string enum_item_str = "\\enum_value_";
-					//get enum item value
-					if(enum_item->children[0]->type != AST_CONSTANT){
-						log_error("expected const, got %s for %s (%s)\n",
-								  type2str(enum_item->children[0]->type).c_str(),
-								  enum_item->str.c_str(), enum_node->str.c_str()
- 								);
-					}
-					RTLIL::Const val = enum_item->children[0]->bitsAsConst(width, is_signed);
-					enum_item_str.append(val.as_string());
-					//set attribute for available val to enum item name mappings
-					attributes[enum_item_str] = mkconst_str(enum_item->str);
-				}
-			}
 
 			// Insert clones children from template at beginning
-			for (int i  = 0; i < GetSize(templ->children); i++)
+			for (int i  = 0; i < GetSize(template_node->children); i++)
-				children.insert(children.begin() + i, templ->children[i]->clone());
+				children.insert(children.begin() + i, template_node->children[i]->clone());
 
 			if (type == AST_MEMORY && GetSize(children) == 1) {
 				// Single-bit memories must have [0:0] range
-				AstNode *rng = new AstNode(AST_RANGE);
+				AstNode *rng = make_range(0, 0);
-				rng->children.push_back(AstNode::mkconst_int(0, true));
-				rng->children.push_back(AstNode::mkconst_int(0, true));
 				children.insert(children.begin(), rng);
 			}
-
 			did_something = true;
 		}
 		log_assert(!is_custom_type);
@@ -1001,29 +1177,29 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 			log_assert(children[1]->type == AST_WIRETYPE);
 			if (!current_scope.count(children[1]->str))
 				log_file_error(filename, location.first_line, "Unknown identifier `%s' used as type name\n", children[1]->str.c_str());
-			AstNode *resolved_type = current_scope.at(children[1]->str);
+			AstNode *resolved_type_node = current_scope.at(children[1]->str);
-			if (resolved_type->type != AST_TYPEDEF)
+			if (resolved_type_node->type != AST_TYPEDEF)
 				log_file_error(filename, location.first_line, "`%s' does not name a type\n", children[1]->str.c_str());
-			log_assert(resolved_type->children.size() == 1);
+			log_assert(resolved_type_node->children.size() == 1);
-			AstNode *templ = resolved_type->children[0];
+			AstNode *template_node = resolved_type_node->children[0];
 			delete children[1];
 			children.pop_back();
 
 			// Ensure typedef itself is fully simplified
-			while(templ->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {};
+			while(template_node->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {};
 
-			if (templ->type == AST_MEMORY)
+			if (template_node->type == AST_MEMORY)
 				log_file_error(filename, location.first_line, "unpacked array type `%s' cannot be used for a parameter\n", children[1]->str.c_str());
-			is_signed = templ->is_signed;
+			is_signed = template_node->is_signed;
-			is_string = templ->is_string;
+			is_string = template_node->is_string;
-			is_custom_type = templ->is_custom_type;
+			is_custom_type = template_node->is_custom_type;
 
-			range_valid = templ->range_valid;
+			range_valid = template_node->range_valid;
-			range_swapped = templ->range_swapped;
+			range_swapped = template_node->range_swapped;
-			range_left = templ->range_left;
+			range_left = template_node->range_left;
-			range_right = templ->range_right;
+			range_right = template_node->range_right;
-			attributes[ID::wiretype] = mkconst_str(resolved_type->str);
+			attributes[ID::wiretype] = mkconst_str(resolved_type_node->str);
-			for (auto template_child : templ->children)
+			for (auto template_child : template_node->children)
 				children.push_back(template_child->clone());
 			did_something = true;
 		}
@@ -1217,7 +1393,29 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 	}
 
 	// annotate identifiers using scope resolution and create auto-wires as needed
+	if (type == AST_IDENTIFIER && !basic_prep) {
+		// check if a plausible struct member sss.mmmm
+		std::string sname;
+		if (name_has_dot(str, sname) && children.size() == 0) {
+			//dumpScope();
+			if (current_scope.count(str) > 0) {
+				auto item_node = current_scope[str];
+				if (item_node->type == AST_STRUCT_ITEM) {
+					//log("found struct item %s\n", item_node->str.c_str());
+					// structure member, rewrite this node to reference the packed struct wire
+					auto range = make_range(item_node->range_left, item_node->range_right);
+					newNode = new AstNode(AST_IDENTIFIER, range);
+					newNode->str = sname;
+					//newNode->dumpAst(NULL, "* ");
+					newNode->basic_prep = true;
+					goto apply_newNode;
+				}
+			}
+		}
+	}
 	if (type == AST_IDENTIFIER) {
+		//log("annotate ID %s, stage=%d cf=%d, ip=%d\n", str.c_str(), stage, const_fold, in_param);
+		//dumpScope();
 		if (current_scope.count(str) == 0) {
 			AstNode *current_scope_ast = (current_ast_mod == nullptr) ? current_ast : current_ast_mod;
 			for (auto node : current_scope_ast->children) {

frontends/verilog/verilog_lexer.l

@@ -263,7 +263,10 @@ static bool isUserType(std::string &s)
 "final"      { SV_KEYWORD(TOK_FINAL); }
 "logic"      { SV_KEYWORD(TOK_LOGIC); }
 "var"        { SV_KEYWORD(TOK_VAR); }
-"bit"        { SV_KEYWORD(TOK_REG); }
+"bit"        { SV_KEYWORD(TOK_LOGIC); }
+"int"        { SV_KEYWORD(TOK_INT); }
+"byte"       { SV_KEYWORD(TOK_BYTE); }
+"shortint"   { SV_KEYWORD(TOK_SHORTINT); }
 
 "eventually"   { if (formal_mode) return TOK_EVENTUALLY; SV_KEYWORD(TOK_EVENTUALLY); }
 "s_eventually" { if (formal_mode) return TOK_EVENTUALLY; SV_KEYWORD(TOK_EVENTUALLY); }
@@ -277,11 +280,15 @@ static bool isUserType(std::string &s)
 "reg"     { return TOK_REG; }
 "integer" { return TOK_INTEGER; }
 "signed"  { return TOK_SIGNED; }
+"unsigned" { SV_KEYWORD(TOK_UNSIGNED); }
 "genvar"  { return TOK_GENVAR; }
 "real"    { return TOK_REAL; }
 
 "enum"    { SV_KEYWORD(TOK_ENUM); }
 "typedef" { SV_KEYWORD(TOK_TYPEDEF); }
+"struct"  { SV_KEYWORD(TOK_STRUCT); }
+"union"   { SV_KEYWORD(TOK_UNION); }
+"packed"  { SV_KEYWORD(TOK_PACKED); }
 
 [0-9][0-9_]* {
 	yylval->string = new std::string(yytext);

frontends/verilog/verilog_parser.y

@@ -161,6 +161,23 @@ static bool isInLocalScope(const std::string *name)
 	return (user_types->count(*name) > 0);
 }
 
+static AstNode *getTypeDefinitionNode(std::string type_name)
+{
+	// return the definition nodes from the typedef statement
+	auto user_types = user_type_stack.back();
+	log_assert(user_types->count(type_name) > 0);
+	auto typedef_node = (*user_types)[type_name];
+	log_assert(typedef_node->type == AST_TYPEDEF);
+	return typedef_node->children[0];
+}
+
+static AstNode *copyTypeDefinition(std::string type_name)
+{
+	// return a copy of the template from a typedef definition
+	auto typedef_node = getTypeDefinitionNode(type_name);
+	return typedef_node->clone();
+}
+
 static AstNode *makeRange(int msb = 31, int lsb = 0, bool isSigned = true)
 {
 	auto range = new AstNode(AST_RANGE);
@@ -175,6 +192,35 @@ static void addRange(AstNode *parent, int msb = 31, int lsb = 0, bool isSigned =
 	auto range = makeRange(msb, lsb, isSigned);
 	parent->children.push_back(range);
 }
+
+static AstNode *checkRange(AstNode *type_node, AstNode *range_node)
+{
+	if (type_node->range_left >= 0 && type_node->range_right >= 0) {
+		// type already restricts the range
+		if (range_node) {
+			frontend_verilog_yyerror("integer/genvar types cannot have packed dimensions.");
+		}
+		else {
+			range_node = makeRange(type_node->range_left, type_node->range_right, false);
+		}
+	}
+	if (range_node && range_node->children.size() != 2) {
+		frontend_verilog_yyerror("wire/reg/logic packed dimension must be of the form: [<expr>:<expr>], [<expr>+:<expr>], or [<expr>-:<expr>]");
+	}
+	return range_node;
+}
+
+static void rewriteAsMemoryNode(AstNode *node, AstNode *rangeNode)
+{
+	node->type = AST_MEMORY;
+	if (rangeNode->type == AST_RANGE && rangeNode->children.size() == 1) {
+		// SV array size [n], rewrite as [n-1:0]
+		rangeNode->children[0] = new AstNode(AST_SUB, rangeNode->children[0], AstNode::mkconst_int(1, true));
+		rangeNode->children.push_back(AstNode::mkconst_int(0, false));
+	}
+	node->children.push_back(rangeNode);
+}
+
 %}
 
 %define api.prefix {frontend_verilog_yy}
@@ -223,14 +269,16 @@ static void addRange(AstNode *parent, int msb = 31, int lsb = 0, bool isSigned =
 %token TOK_POS_INDEXED TOK_NEG_INDEXED TOK_PROPERTY TOK_ENUM TOK_TYPEDEF
 %token TOK_RAND TOK_CONST TOK_CHECKER TOK_ENDCHECKER TOK_EVENTUALLY
 %token TOK_INCREMENT TOK_DECREMENT TOK_UNIQUE TOK_PRIORITY
+%token TOK_STRUCT TOK_PACKED TOK_UNSIGNED TOK_INT TOK_BYTE TOK_SHORTINT TOK_UNION 
 
 %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 integral_number
 %type <string> type_name
-%type <ast> opt_enum_init
+%type <ast> opt_enum_init enum_type struct_type non_wire_data_type
 %type <boolean> opt_signed opt_property unique_case_attr always_comb_or_latch always_or_always_ff
 %type <al> attr case_attr
+%type <ast> struct_union
 
 %type <specify_target_ptr> specify_target
 %type <specify_triple_ptr> specify_triple specify_opt_triple
@@ -520,9 +568,10 @@ package_body:
 	;
 
 package_body_stmt:
-	typedef_decl |
+	  typedef_decl
-	localparam_decl |
+	| localparam_decl
-	param_decl;
+	| param_decl
+	;
 
 interface:
 	TOK_INTERFACE {
@@ -582,6 +631,7 @@ wire_type_token_list:
 		astbuf3->is_custom_type = true;
 		astbuf3->children.push_back(new AstNode(AST_WIRETYPE));
 		astbuf3->children.back()->str = *$1;
+		delete $1;
 	};
 
 wire_type_token_io:
@@ -682,15 +732,9 @@ range_or_multirange:
 	non_opt_multirange { $$ = $1; };
 
 range_or_signed_int:
-	range {
+	  range 		{ $$ = $1; }
-		$$ = $1;
+	| TOK_INTEGER		{ $$ = makeRange(); }
-	} |
+	;
-	TOK_INTEGER {
-		$$ = new AstNode(AST_RANGE);
-		$$->children.push_back(AstNode::mkconst_int(31, true));
-		$$->children.push_back(AstNode::mkconst_int(0, true));
-		$$->is_signed = true;
-	};
 
 module_body:
 	module_body module_body_stmt |
@@ -700,7 +744,7 @@ module_body:
 
 module_body_stmt:
 	task_func_decl | specify_block | param_decl | localparam_decl | typedef_decl | defparam_decl | specparam_declaration | wire_decl | assign_stmt | cell_stmt |
-	enum_decl |
+	enum_decl | struct_decl |
 	always_stmt | TOK_GENERATE module_gen_body TOK_ENDGENERATE | defattr | assert_property | checker_decl | ignored_specify_block;
 
 checker_decl:
@@ -841,18 +885,7 @@ task_func_port:
 		}
 		albuf = $1;
 		astbuf1 = $2;
-		astbuf2 = $3;
+		astbuf2 = checkRange(astbuf1, $3);
-		if (astbuf1->range_left >= 0 && astbuf1->range_right >= 0) {
-			if (astbuf2) {
-				frontend_verilog_yyerror("integer/genvar types cannot have packed dimensions (task/function arguments)");
-			} else {
-				astbuf2 = new AstNode(AST_RANGE);
-				astbuf2->children.push_back(AstNode::mkconst_int(astbuf1->range_left, true));
-				astbuf2->children.push_back(AstNode::mkconst_int(astbuf1->range_right, true));
-			}
-		}
-		if (astbuf2 && astbuf2->children.size() != 2)
-			frontend_verilog_yyerror("task/function argument range must be of the form: [<expr>:<expr>], [<expr>+:<expr>], or [<expr>-:<expr>]");
 	} wire_name |
 	{
 		if (!astbuf1) {
@@ -1387,6 +1420,10 @@ single_defparam_decl:
 		ast_stack.back()->children.push_back(node);
 	};
 
+/////////
+// enum
+/////////
+
 enum_type: TOK_ENUM {
 		static int enum_count;
 		// create parent node for the enum
@@ -1397,31 +1434,40 @@ enum_type: TOK_ENUM {
 		// create the template for the names
 		astbuf1 = new AstNode(AST_ENUM_ITEM);
 		astbuf1->children.push_back(AstNode::mkconst_int(0, true));
-	 } param_signed enum_base_type '{' enum_name_list '}' {  // create template for the enum vars
+	 } enum_base_type '{' enum_name_list '}' {	// create template for the enum vars
-								auto tnode = astbuf1->clone();
+							auto tnode = astbuf1->clone();
-								delete astbuf1;
+							delete astbuf1;
-								astbuf1 = tnode;
+							astbuf1 = tnode;
-								tnode->type = AST_WIRE;
+							tnode->type = AST_WIRE;
-								tnode->attributes[ID::enum_type] = AstNode::mkconst_str(astbuf2->str);
+							tnode->attributes[ID::enum_type] = AstNode::mkconst_str(astbuf2->str);
-								// drop constant but keep any range
+							// drop constant but keep any range
-								delete tnode->children[0];
+							delete tnode->children[0];
-								tnode->children.erase(tnode->children.begin()); }
+							tnode->children.erase(tnode->children.begin());
+							$$ = astbuf1; }
 	 ;
 
-enum_base_type: int_vec param_range
+enum_base_type: type_atom type_signing
-	| int_atom
+	| type_vec type_signing range	{ if ($3) astbuf1->children.push_back($3); }
-	| /* nothing */		{astbuf1->is_reg = true; addRange(astbuf1); }
+	| /* nothing */			{ astbuf1->is_reg = true; addRange(astbuf1); }
 	;
 
-int_atom: TOK_INTEGER		{astbuf1->is_reg=true; addRange(astbuf1); }		// probably should do byte, range [7:0] here
+type_atom: TOK_INTEGER		{ astbuf1->is_reg = true; addRange(astbuf1); }		// 4-state signed
+	|  TOK_INT		{ astbuf1->is_reg = true; addRange(astbuf1); }		// 2-state signed
+	|  TOK_SHORTINT		{ astbuf1->is_reg = true; addRange(astbuf1, 15, 0); }	// 2-state signed
+	|  TOK_BYTE		{ astbuf1->is_reg = true; addRange(astbuf1,  7, 0); }	// 2-state signed
 	;
 
-int_vec: TOK_REG {astbuf1->is_reg = true;}
+type_vec: TOK_REG		{ astbuf1->is_reg   = true; }		// unsigned
-	| TOK_LOGIC  {astbuf1->is_logic = true;}
+	| TOK_LOGIC		{ astbuf1->is_logic = true; }		// unsigned
 	;
 
-enum_name_list:
+type_signing:
-	enum_name_decl
+	  TOK_SIGNED		{ astbuf1->is_signed = true; }
+	| TOK_UNSIGNED		{ astbuf1->is_signed = false; }
+	| // optional
+	;
+
+enum_name_list: enum_name_decl
 	| enum_name_list ',' enum_name_decl
 	;
 
@@ -1433,6 +1479,7 @@ enum_name_decl:
 		auto node = astbuf1->clone();
 		node->str = *$1;
 		delete $1;
+		SET_AST_NODE_LOC(node, @1, @1);
 		delete node->children[0];
 		node->children[0] = $2 ?: new AstNode(AST_NONE);
 		astbuf2->children.push_back(node);
@@ -1456,32 +1503,122 @@ enum_var: TOK_ID {
 		ast_stack.back()->children.push_back(node);
 		node->str = *$1;
 		delete $1;
+		SET_AST_NODE_LOC(node, @1, @1);
 		node->is_enum = true;
 	}
 	;
 
-enum_decl: enum_type enum_var_list ';'			{
+enum_decl: enum_type enum_var_list ';'		{ delete $1; }
-		//enum_type creates astbuf1 for use by typedef only
-		delete astbuf1;
-	}
 	;
 
+//////////////////
+// struct or union
+//////////////////
+
+struct_decl: struct_type struct_var_list ';' 	{ delete astbuf2; }
+	;
+
+struct_type: struct_union { astbuf2 = $1; } struct_body { $$ = astbuf2; }
+	;
+
+struct_union:
+	  TOK_STRUCT		{ $$ = new AstNode(AST_STRUCT); }
+	| TOK_UNION		{ $$ = new AstNode(AST_UNION); }
+	;
+
+struct_body: opt_packed '{' struct_member_list '}'
+	;
+
+opt_packed: TOK_PACKED opt_signed_struct
+	| { frontend_verilog_yyerror("Only PACKED supported at this time"); }
+	;
+
+opt_signed_struct:
+	  TOK_SIGNED		{ astbuf2->is_signed = true; }
+	| TOK_UNSIGNED		{ astbuf2->is_signed = false; }
+	| // default is unsigned
+	;
+
+struct_member_list: struct_member
+	| struct_member_list struct_member
+	;
+
+struct_member: struct_member_type member_name_list ';'		{ delete astbuf1; }
+	;
+
+member_name_list:
+	  member_name
+	| member_name_list ',' member_name
+	;
+
+member_name: TOK_ID {
+			astbuf1->str = $1->substr(1);
+			delete $1;
+			auto member_node = astbuf1->clone();
+			SET_AST_NODE_LOC(member_node, @1, @1);
+			astbuf2->children.push_back(member_node);
+		}
+	;
+
+struct_member_type: { astbuf1 = new AstNode(AST_STRUCT_ITEM); } member_type_token
+	;
+
+member_type_token:
+	  member_type 
+	| hierarchical_type_id {
+			// use a clone of the typedef definition nodes
+			auto template_node = copyTypeDefinition(*$1);
+			delete $1;
+			switch (template_node->type) {
+			case AST_WIRE:
+				template_node->type = AST_STRUCT_ITEM;
+				break;
+			case AST_STRUCT:
+			case AST_UNION:
+				break;
+			default:
+				frontend_verilog_yyerror("Invalid type for struct member: %s", type2str(template_node->type).c_str());
+			}
+			delete astbuf1;
+			astbuf1 = template_node;
+		}
+	| struct_union {
+			// stash state on ast_stack
+			ast_stack.push_back(astbuf2);
+			astbuf2 = $1;
+		} struct_body  {
+		        astbuf1 = astbuf2;
+			// recover state
+			astbuf2 = ast_stack.back();
+			ast_stack.pop_back();
+		}
+	;
+
+member_type: type_atom type_signing
+	| type_vec type_signing range	{ if ($3) astbuf1->children.push_back($3); }
+	;
+
+struct_var_list: struct_var
+	| struct_var_list ',' struct_var
+	;
+
+struct_var: TOK_ID	{	auto *var_node = astbuf2->clone();
+				var_node->str = *$1;
+				delete $1;
+				SET_AST_NODE_LOC(var_node, @1, @1);
+				ast_stack.back()->children.push_back(var_node);
+			}
+	;
+
+/////////
+// wire
+/////////
+
 wire_decl:
 	attr wire_type range {
 		albuf = $1;
 		astbuf1 = $2;
-		astbuf2 = $3;
+		astbuf2 = checkRange(astbuf1, $3);
-		if (astbuf1->range_left >= 0 && astbuf1->range_right >= 0) {
-			if (astbuf2) {
-				frontend_verilog_yyerror("integer/genvar types cannot have packed dimensions.");
-			} else {
-				astbuf2 = new AstNode(AST_RANGE);
-				astbuf2->children.push_back(AstNode::mkconst_int(astbuf1->range_left, true));
-				astbuf2->children.push_back(AstNode::mkconst_int(astbuf1->range_right, true));
-			}
-		}
-		if (astbuf2 && astbuf2->children.size() != 2)
-			frontend_verilog_yyerror("wire/reg/logic packed dimension must be of the form: [<expr>:<expr>], [<expr>+:<expr>], or [<expr>-:<expr>]");
 	} delay wire_name_list {
 		delete astbuf1;
 		if (astbuf2 != NULL)
@@ -1603,19 +1740,9 @@ wire_name:
 			if (node->is_input || node->is_output)
 				frontend_verilog_yyerror("input/output/inout ports cannot have unpacked dimensions.");
 			if (!astbuf2 && !node->is_custom_type) {
-				AstNode *rng = new AstNode(AST_RANGE);
+				addRange(node, 0, 0, false);
-				rng->children.push_back(AstNode::mkconst_int(0, true));
-				rng->children.push_back(AstNode::mkconst_int(0, true));
-				node->children.push_back(rng);
 			}
-			node->type = AST_MEMORY;
+			rewriteAsMemoryNode(node, $2);
-			auto *rangeNode = $2;
-			if (rangeNode->type == AST_RANGE && rangeNode->children.size() == 1) {
-				// SV array size [n], rewrite as [n-1:0]
-				rangeNode->children[0] = new AstNode(AST_SUB, rangeNode->children[0], AstNode::mkconst_int(1, true));
-				rangeNode->children.push_back(AstNode::mkconst_int(0, false));
-			}
-			node->children.push_back(rangeNode);
 		}
 		if (current_function_or_task == NULL) {
 			if (do_not_require_port_stubs && (node->is_input || node->is_output) && port_stubs.count(*$1) == 0) {
@@ -1663,42 +1790,23 @@ type_name: TOK_ID		// first time seen
 typedef_decl:
 	TOK_TYPEDEF wire_type range type_name range_or_multirange ';' {
 		astbuf1 = $2;
-		astbuf2 = $3;
+		astbuf2 = checkRange(astbuf1, $3);
-		if (astbuf1->range_left >= 0 && astbuf1->range_right >= 0) {
-			if (astbuf2) {
-				frontend_verilog_yyerror("integer/genvar types cannot have packed dimensions.");
-			} else {
-				astbuf2 = new AstNode(AST_RANGE);
-				astbuf2->children.push_back(AstNode::mkconst_int(astbuf1->range_left, true));
-				astbuf2->children.push_back(AstNode::mkconst_int(astbuf1->range_right, true));
-			}
-		}
-		if (astbuf2 && astbuf2->children.size() != 2)
-			frontend_verilog_yyerror("wire/reg/logic packed dimension must be of the form: [<expr>:<expr>], [<expr>+:<expr>], or [<expr>-:<expr>]");
 		if (astbuf2)
 			astbuf1->children.push_back(astbuf2);
 
 		if ($5 != NULL) {
 			if (!astbuf2) {
-				AstNode *rng = new AstNode(AST_RANGE);
+				addRange(astbuf1, 0, 0, false);
-				rng->children.push_back(AstNode::mkconst_int(0, true));
-				rng->children.push_back(AstNode::mkconst_int(0, true));
-				astbuf1->children.push_back(rng);
 			}
-			astbuf1->type = AST_MEMORY;
+			rewriteAsMemoryNode(astbuf1, $5);
-			auto *rangeNode = $5;
-			if (rangeNode->type == AST_RANGE && rangeNode->children.size() == 1) {
-				// SV array size [n], rewrite as [n-1:0]
-				rangeNode->children[0] = new AstNode(AST_SUB, rangeNode->children[0], AstNode::mkconst_int(1, true));
-				rangeNode->children.push_back(AstNode::mkconst_int(0, false));
-			}
-			astbuf1->children.push_back(rangeNode);
 		}
-		addTypedefNode($4, astbuf1);
+		addTypedefNode($4, astbuf1); }
-	} |
+	| TOK_TYPEDEF non_wire_data_type type_name ';'   { addTypedefNode($3, $2); }
-	TOK_TYPEDEF enum_type type_name ';' {
+	;
-		addTypedefNode($3, astbuf1);
+
-	}
+non_wire_data_type:
+	  enum_type
+	| struct_type
 	;
 
 cell_stmt:

tests/svtypes/struct_simple.sv

@@ -0,0 +1,48 @@
+module top;
+	localparam BITS=8;
+
+	struct packed {
+		logic a;
+		logic[BITS-1:0] b;
+		byte c;
+		logic x, y;
+	} s;
+
+	struct packed signed { 
+		integer a;
+		logic[15:0] b;
+		logic[7:0] c;
+		bit [7:0] d;
+	} pack1;
+
+	struct packed {
+		byte a;
+		struct packed {
+			byte x, y;
+		} b;
+	} s2;
+
+	assign s.a = '1;
+	assign s.b = '1;
+	assign s.c = 8'hAA;
+	assign s.x = '1;
+	logic[7:0] t;
+	assign t = s.b;
+	assign pack1.a = 42;
+	assign pack1.b = 16'hAAAA;
+	assign pack1.c = '1;
+	assign pack1.d = 8'h55;
+	assign s2.b.x = 'h42;
+
+	always_comb assert(s.a == 1'b1);
+	always_comb assert(s.c == 8'hAA);
+	always_comb assert(s.x == 1'b1);
+	always_comb assert(t == 8'hFF);
+	always_comb assert(pack1.a == 42);
+	always_comb assert(pack1.b == 16'hAAAA);
+	always_comb assert(pack1.c == 8'hFF);
+	always_comb assert(pack1[15:8] == 8'hFF);
+	always_comb assert(pack1.d == 8'h55);
+	always_comb assert(s2.b.x == 'h42);
+
+endmodule

tests/svtypes/typedef_struct.sv

@@ -0,0 +1,42 @@
+package p;
+
+typedef struct packed {
+	byte a;
+	byte b;
+} p_t;
+
+endpackage
+
+
+module top;
+
+	typedef logic[7:0] t_t;
+
+	typedef struct packed {
+		bit		a;
+		logic[7:0]	b;
+		t_t		t;
+	} s_t;
+
+	s_t s;
+	s_t s1;
+
+	p::p_t ps;
+
+	assign s.a = '1;
+	assign s.b = '1;
+	assign s.t = 8'h55;
+	assign s1 = s;
+	assign ps.a = 8'hAA;
+	assign ps.b = 8'h55;
+
+	always_comb begin
+		assert(s.a == 1'b1);
+		assert(s.b == 8'hFF);
+		assert(s.t == 8'h55);
+		assert(s1.t == 8'h55);
+		assert(ps.a == 8'hAA);
+		assert(ps.b == 8'h55);
+	end
+
+endmodule

tests/svtypes/union_simple.sv

@@ -0,0 +1,72 @@
+module top;
+
+	typedef struct packed {
+		byte a,b,c,d;
+	} byte4_t;
+
+	typedef union packed {
+		int	x;
+		byte4_t	y;
+	} w_t;
+
+	w_t w;
+
+	assign w.x = 'h42;
+	always_comb begin
+		assert(w.y.d == 8'h42);
+	end
+
+	typedef logic[4:0] reg_addr_t;
+	typedef logic[6:0] opcode_t;
+
+	typedef struct packed {
+		bit [6:0]  func7;
+		reg_addr_t rs2;
+		reg_addr_t rs1;
+		bit [2:0]  func3;
+		reg_addr_t rd;
+		opcode_t   opcode;
+	} R_t;
+
+	typedef struct packed {
+		bit[11:0]  imm;
+		reg_addr_t rs1;
+		bit[2:0]   func3;
+		reg_addr_t rd;
+		opcode_t   opcode;
+	} I_t;
+
+	typedef struct packed {
+		bit[19:0]  imm;
+		reg_addr_t rd;
+		opcode_t   opcode;
+	} U_t;
+
+	typedef union packed {
+		R_t	r;
+		I_t	i;
+		U_t	u;
+	} instruction_t;
+
+	instruction_t ir1;
+	assign ir1 = 32'h0AA01EB7;          //	lui t4,0xAA01
+	always_comb begin
+		assert(ir1.u.opcode == 'h37);
+		assert(ir1.r.opcode == 'h37);
+		assert(ir1.u.rd == 'd29);
+		assert(ir1.r.rd == 'd29);
+		assert(ir1.u.imm == 'hAA01);
+	end
+
+	union packed {
+		int word;
+		struct packed {
+			byte a, b, c, d;
+		} byte4;
+	} u;
+	assign u.word = 'h42;
+	always_comb begin
+		assert(u.byte4.d == 'h42);
+	end
+
+endmodule