Merge pull request #1519 from YosysHQ/eddie/submod_po

submod: several bugfixes

passes/hierarchy/submod.cc

@@ -20,6 +20,7 @@
 #include "kernel/register.h"
 #include "kernel/celltypes.h"
 #include "kernel/log.h"
+#include "kernel/sigtools.h"
 #include <stdlib.h>
 #include <stdio.h>
 #include <set>
@@ -32,49 +33,56 @@ struct SubmodWorker
 	CellTypes ct;
 	RTLIL::Design *design;
 	RTLIL::Module *module;
+	SigMap sigmap;
 
 	bool copy_mode;
+	bool hidden_mode;
 	std::string opt_name;
 
 	struct SubModule
 	{
 		std::string name, full_name;
-		std::set<RTLIL::Cell*> cells;
+		pool<RTLIL::Cell*> cells;
 	};
 
 	std::map<std::string, SubModule> submodules;
 
 	struct wire_flags_t {
 		RTLIL::Wire *new_wire;
-		bool is_int_driven, is_int_used, is_ext_driven, is_ext_used;
+		RTLIL::Const is_int_driven;
-		wire_flags_t() : new_wire(NULL), is_int_driven(false), is_int_used(false), is_ext_driven(false), is_ext_used(false) { }
+		bool is_int_used, is_ext_driven, is_ext_used;
+		wire_flags_t(RTLIL::Wire* wire) : new_wire(NULL), is_int_driven(State::S0, GetSize(wire)), is_int_used(false), is_ext_driven(false), is_ext_used(false) { }
 	};
 	std::map<RTLIL::Wire*, wire_flags_t> wire_flags;
 	bool flag_found_something;
 
-	void flag_wire(RTLIL::Wire *wire, bool create, bool set_int_driven, bool set_int_used, bool set_ext_driven, bool set_ext_used)
+	void flag_wire(RTLIL::Wire *wire, bool create, bool set_int_used, bool set_ext_driven, bool set_ext_used)
 	{
 		if (wire_flags.count(wire) == 0) {
 			if (!create)
 				return;
-			wire_flags[wire] = wire_flags_t();
+			wire_flags.emplace(wire, wire);
 		}
-		if (set_int_driven)
-			wire_flags[wire].is_int_driven = true;
 		if (set_int_used)
-			wire_flags[wire].is_int_used = true;
+			wire_flags.at(wire).is_int_used = true;
 		if (set_ext_driven)
-			wire_flags[wire].is_ext_driven = true;
+			wire_flags.at(wire).is_ext_driven = true;
 		if (set_ext_used)
-			wire_flags[wire].is_ext_used = true;
+			wire_flags.at(wire).is_ext_used = true;
 		flag_found_something = true;
 	}
 
 	void flag_signal(const RTLIL::SigSpec &sig, bool create, bool set_int_driven, bool set_int_used, bool set_ext_driven, bool set_ext_used)
 	{
 		for (auto &c : sig.chunks())
-			if (c.wire != NULL)
+			if (c.wire != NULL) {
-				flag_wire(c.wire, create, set_int_driven, set_int_used, set_ext_driven, set_ext_used);
+				flag_wire(c.wire, create, set_int_used, set_ext_driven, set_ext_used);
+				if (set_int_driven)
+					for (int i = c.offset; i < c.offset+c.width; i++) {
+						wire_flags.at(c.wire).is_int_driven[i] = State::S1;
+						flag_found_something = true;
+					}
+			}
 	}
 
 	void handle_submodule(SubModule &submod)
@@ -127,27 +135,39 @@ struct SubmodWorker
 				flags.is_ext_driven = true;
 			if (wire->port_output)
 				flags.is_ext_used = true;
+			else {
+				auto sig = sigmap(wire);
+				for (auto c : sig.chunks())
+					if (c.wire && c.wire->port_output) {
+						flags.is_ext_used = true;
+						break;
+					}
+			}
 
 			bool new_wire_port_input = false;
 			bool new_wire_port_output = false;
 
-			if (flags.is_int_driven && flags.is_ext_used)
+			if (!flags.is_int_driven.is_fully_zero() && flags.is_ext_used)
 				new_wire_port_output = true;
 			if (flags.is_ext_driven && flags.is_int_used)
 				new_wire_port_input = true;
 
-			if (flags.is_int_driven && flags.is_ext_driven)
+			if (!flags.is_int_driven.is_fully_zero() && flags.is_ext_driven)
 				new_wire_port_input = true, new_wire_port_output = true;
 
 			std::string new_wire_name = wire->name.str();
 			if (new_wire_port_input || new_wire_port_output) {
-				while (new_wire_name[0] == '$') {
+				if (new_wire_name[0] == '$')
-					std::string next_wire_name = stringf("\\n%d", auto_name_counter++);
+					while (1) {
-					if (all_wire_names.count(next_wire_name) == 0) {
+						std::string next_wire_name = stringf("%s\\n%d", hidden_mode ? "$submod" : "", auto_name_counter++);
-						all_wire_names.insert(next_wire_name);
+						if (all_wire_names.count(next_wire_name) == 0) {
-						new_wire_name = next_wire_name;
+							all_wire_names.insert(next_wire_name);
+							new_wire_name = next_wire_name;
+							break;
+						}
 					}
-				}
+				else if (hidden_mode)
+					new_wire_name = stringf("$submod%s", new_wire_name.c_str());
 			}
 
 			RTLIL::Wire *new_wire = new_mod->addWire(new_wire_name, wire->width);
@@ -155,6 +175,22 @@ struct SubmodWorker
 			new_wire->port_output = new_wire_port_output;
 			new_wire->start_offset = wire->start_offset;
 			new_wire->attributes = wire->attributes;
+			if (!flags.is_int_driven.is_fully_zero()) {
+				new_wire->attributes.erase(ID(init));
+				auto sig = sigmap(wire);
+				for (int i = 0; i < GetSize(sig); i++) {
+					if (flags.is_int_driven[i] == State::S0)
+						continue;
+					if (!sig[i].wire)
+						continue;
+					auto it = sig[i].wire->attributes.find(ID(init));
+					if (it != sig[i].wire->attributes.end()) {
+						auto jt = new_wire->attributes.insert(std::make_pair(ID(init), Const(State::Sx, GetSize(sig)))).first;
+						jt->second[i] = it->second[sig[i].offset];
+						it->second[sig[i].offset] = State::Sx;
+					}
+				}
+			}
 
 			if (new_wire->port_input && new_wire->port_output)
 				log("  signal %s: inout %s\n", wire->name.c_str(), new_wire->name.c_str());
@@ -177,7 +213,7 @@ struct SubmodWorker
 				for (auto &bit : conn.second)
 					if (bit.wire != NULL) {
 						log_assert(wire_flags.count(bit.wire) > 0);
-						bit.wire = wire_flags[bit.wire].new_wire;
+						bit.wire = wire_flags.at(bit.wire).new_wire;
 					}
 			log("  cell %s (%s)\n", new_cell->name.c_str(), new_cell->type.c_str());
 			if (!copy_mode)
@@ -189,16 +225,27 @@ struct SubmodWorker
 			RTLIL::Cell *new_cell = module->addCell(submod.full_name, submod.full_name);
 			for (auto &it : wire_flags)
 			{
-				RTLIL::Wire *old_wire = it.first;
+				RTLIL::SigSpec old_sig = sigmap(it.first);
 				RTLIL::Wire *new_wire = it.second.new_wire;
-				if (new_wire->port_id > 0)
+				if (new_wire->port_id > 0) {
-					new_cell->setPort(new_wire->name, RTLIL::SigSpec(old_wire));
+					if (new_wire->port_output)
+						for (int i = 0; i < GetSize(old_sig); i++) {
+							auto &b = old_sig[i];
+							// Prevents "ERROR: Mismatch in directionality ..." when flattening
+							if (!b.wire)
+								b = module->addWire(NEW_ID);
+							// Prevents "Warning: multiple conflicting drivers ..."
+							else if (!it.second.is_int_driven[i])
+								b = module->addWire(NEW_ID);
+						}
+					new_cell->setPort(new_wire->name, old_sig);
+				}
 			}
 		}
 	}
 
-	SubmodWorker(RTLIL::Design *design, RTLIL::Module *module, bool copy_mode = false, std::string opt_name = std::string()) :
+	SubmodWorker(RTLIL::Design *design, RTLIL::Module *module, bool copy_mode = false, bool hidden_mode = false, std::string opt_name = std::string()) :
-			design(design), module(module), copy_mode(copy_mode), opt_name(opt_name)
+			design(design), module(module), sigmap(module), copy_mode(copy_mode), hidden_mode(hidden_mode), opt_name(opt_name)
 	{
 		if (!design->selected_whole_module(module->name) && opt_name.empty())
 			return;
@@ -219,6 +266,12 @@ struct SubmodWorker
 		ct.setup_stdcells_mem();
 		ct.setup_design(design);
 
+		for (auto port : module->ports) {
+			auto wire = module->wire(port);
+			if (wire->port_output)
+				sigmap.add(wire);
+		}
+
 		if (opt_name.empty())
 		{
 			for (auto &it : module->wires_)
@@ -273,7 +326,7 @@ struct SubmodPass : public Pass {
 	{
 		//   |---v---|---v---|---v---|---v---|---v---|---v---|---v---|---v---|---v---|---v---|
 		log("\n");
-		log("    submod [-copy] [selection]\n");
+		log("    submod [options] [selection]\n");
 		log("\n");
 		log("This pass identifies all cells with the 'submod' attribute and moves them to\n");
 		log("a newly created module. The value of the attribute is used as name for the\n");
@@ -285,16 +338,20 @@ struct SubmodPass : public Pass {
 		log("This pass only operates on completely selected modules with no processes\n");
 		log("or memories.\n");
 		log("\n");
+		log("    -copy\n");
+		log("        by default the cells are 'moved' from the source module and the source\n");
+		log("        module will use an instance of the new module after this command is\n");
+		log("        finished. call with -copy to not modify the source module.\n");
 		log("\n");
-		log("    submod -name <name> [-copy] [selection]\n");
+		log("    -name <name>\n");
+		log("        don't use the 'submod' attribute but instead use the selection. only\n");
+		log("        objects from one module might be selected. the value of the -name option\n");
+		log("        is used as the value of the 'submod' attribute instead.\n");
 		log("\n");
-		log("As above, but don't use the 'submod' attribute but instead use the selection.\n");
+		log("    -hidden\n");
-		log("Only objects from one module might be selected. The value of the -name option\n");
+		log("        instead of creating submodule ports with public names, create ports with\n");
-		log("is used as the value of the 'submod' attribute above.\n");
+		log("        private names so that a subsequent 'flatten; clean' call will restore the\n");
-		log("\n");
+		log("        original module with original public names.\n");
-		log("By default the cells are 'moved' from the source module and the source module\n");
-		log("will use an instance of the new module after this command is finished. Call\n");
-		log("with -copy to not modify the source module.\n");
 		log("\n");
 	}
 	void execute(std::vector<std::string> args, RTLIL::Design *design) YS_OVERRIDE
@@ -304,6 +361,7 @@ struct SubmodPass : public Pass {
 
 		std::string opt_name;
 		bool copy_mode = false;
+		bool hidden_mode = false;
 
 		size_t argidx;
 		for (argidx = 1; argidx < args.size(); argidx++) {
@@ -315,6 +373,10 @@ struct SubmodPass : public Pass {
 				copy_mode = true;
 				continue;
 			}
+			if (args[argidx] == "-hidden") {
+				hidden_mode = true;
+				continue;
+			}
 			break;
 		}
 		extra_args(args, argidx, design);
@@ -335,7 +397,7 @@ struct SubmodPass : public Pass {
 						queued_modules.push_back(mod_it.first);
 				for (auto &modname : queued_modules)
 					if (design->modules_.count(modname) != 0) {
-						SubmodWorker worker(design, design->modules_[modname], copy_mode);
+						SubmodWorker worker(design, design->modules_[modname], copy_mode, hidden_mode);
 						handled_modules.insert(modname);
 						did_something = true;
 					}
@@ -358,7 +420,7 @@ struct SubmodPass : public Pass {
 			else {
 				Pass::call_on_module(design, module, "opt_clean");
 				log_header(design, "Continuing SUBMOD pass.\n");
-				SubmodWorker worker(design, module, copy_mode, opt_name);
+				SubmodWorker worker(design, module, copy_mode, hidden_mode, opt_name);
 			}
 		}
 

tests/various/submod.ys

@@ -0,0 +1,124 @@
+read_verilog <<EOT
+module top(input a, output b);
+wire c;
+(* submod="bar" *) sub s1(a, c);
+assign b = c;
+endmodule
+
+module sub(input a, output c);
+assign c = a;
+endmodule
+EOT
+
+hierarchy -top top
+proc
+design -save gold
+
+submod
+check -assert
+design -stash gate
+
+design -import gold -as gold
+design -import gate -as gate
+
+miter -equiv -flatten -make_assert -make_outputs gold gate miter
+sat -verify -prove-asserts -show-ports miter
+
+
+design -reset
+read_verilog <<EOT
+module top(input a, output [1:0] b);
+(* submod="bar" *) sub s1(a, b[1]);
+assign b[0] = 1'b0;
+endmodule
+
+module sub(input a, output c);
+assign c = a;
+endmodule
+EOT
+
+hierarchy -top top
+proc
+design -save gold
+
+submod
+check -assert top
+design -stash gate
+
+design -import gold -as gold
+design -import gate -as gate
+
+miter -equiv -flatten -make_assert -make_outputs gold gate miter
+sat -verify -prove-asserts -show-ports miter
+
+
+design -reset
+read_verilog <<EOT
+module top(input a, output [1:0] b, c);
+(* submod="bar" *) sub s1(a, b[0]);
+(* submod="bar" *) sub s2(a, c[1]);
+assign c = b;
+endmodule
+
+module sub(input a, output c);
+assign c = a;
+endmodule
+EOT
+
+hierarchy -top top
+proc
+design -save gold
+
+submod
+check -assert top
+design -stash gate
+
+design -import gold -as gold
+design -import gate -as gate
+
+miter -equiv -flatten -make_assert -make_outputs gold gate miter
+sat -verify -prove-asserts -show-ports miter
+
+
+design -reset
+read_verilog <<EOT
+module top(input d, c, (* init = 3'b011 *) output reg [2:0] q);
+(* submod="bar" *) DFF s1(.D(d), .C(c), .Q(q[1]));
+DFF s2(.D(d), .C(c), .Q(q[0]));
+DFF s3(.D(d), .C(c), .Q(q[2]));
+endmodule
+
+module DFF(input D, C, output Q);
+parameter INIT = 1'b0;
+endmodule
+EOT
+
+hierarchy -top top
+proc
+
+submod
+dffinit -ff DFF Q INIT
+check -noinit -assert
+
+
+design -reset
+read_verilog <<EOT
+module top(input d, c, output reg [2:0] q);
+(* submod="bar" *) DFF s1(.D(d), .C(c), .Q(q[1]));
+DFF s2(.D(d), .C(c), .Q(q[0]));
+DFF s3(.D(d), .C(c), .Q(q[2]));
+endmodule
+EOT
+
+hierarchy -top top
+proc
+
+submod
+flatten
+
+read_verilog <<EOT
+module DFF(input D, C, output Q);
+endmodule
+EOT
+
+check -assert