Merge pull request #2491 from zachjs/port-bind-sign

Sign extend port connections where necessary

frontends/ast/genrtlil.cc

@@ -106,6 +106,7 @@ static RTLIL::SigSpec binop2rtlil(AstNode *that, IdString type, int result_width
 
 	RTLIL::Wire *wire = current_module->addWire(cell->name.str() + "_Y", result_width);
 	wire->attributes[ID::src] = stringf("%s:%d.%d-%d.%d", that->filename.c_str(), that->location.first_line, that->location.first_column, that->location.last_line, that->location.last_column);
+	wire->is_signed = that->is_signed;
 
 	for (auto &attr : that->attributes) {
 		if (attr.second->type != AST_CONSTANT)
@@ -1721,8 +1722,29 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 				}
 				if (child->type == AST_ARGUMENT) {
 					RTLIL::SigSpec sig;
-					if (child->children.size() > 0)
+					if (child->children.size() > 0) {
-						sig = child->children[0]->genRTLIL();
+						AstNode *arg = child->children[0];
+						int local_width_hint = -1;
+						bool local_sign_hint = false;
+						// don't inadvertently attempt to detect the width of interfaces
+						if (arg->type != AST_IDENTIFIER || !arg->id2ast || arg->id2ast->type != AST_CELL)
+							arg->detectSignWidth(local_width_hint, local_sign_hint);
+						sig = arg->genRTLIL(local_width_hint, local_sign_hint);
+						log_assert(local_sign_hint == arg->is_signed);
+						if (sig.is_wire()) {
+							// if the resulting SigSpec is a wire, its
+							// signedness should match that of the AstNode
+							log_assert(arg->is_signed == sig.as_wire()->is_signed);
+						} else if (arg->is_signed) {
+							// non-trivial signed nodes are indirected through
+							// signed wires to enable sign extension
+							RTLIL::IdString wire_name = NEW_ID;
+							RTLIL::Wire *wire = current_module->addWire(wire_name, arg->bits.size());
+							wire->is_signed = true;
+							current_module->connect(wire, sig);
+							sig = wire;
+						}
+					}
 					if (child->str.size() == 0) {
 						char buf[100];
 						snprintf(buf, 100, "$%d", ++port_counter);

passes/hierarchy/hierarchy.cc

@@ -1233,14 +1233,18 @@ struct HierarchyPass : public Pass {
 						{
 							int n = GetSize(conn.second) - GetSize(w);
 							if (!w->port_input && w->port_output)
-								module->connect(sig.extract(GetSize(w), n), Const(0, n));
+							{
+								RTLIL::SigSpec out = sig.extract(0, GetSize(w));
+								out.extend_u0(GetSize(sig), w->is_signed);
+								module->connect(sig.extract(GetSize(w), n), out.extract(GetSize(w), n));
+							}
 							sig.remove(GetSize(w), n);
 						}
 						else
 						{
 							int n = GetSize(w) - GetSize(conn.second);
 							if (w->port_input && !w->port_output)
-								sig.append(Const(0, n));
+								sig.extend_u0(GetSize(w), sig.is_wire() && sig.as_wire()->is_signed);
 							else
 								sig.append(module->addWire(NEW_ID, n));
 						}

passes/techmap/flatten.cc

@@ -180,12 +180,15 @@ struct FlattenWorker
 
 			RTLIL::Wire *tpl_wire = tpl->wire(port_name);
 			RTLIL::SigSig new_conn;
+			bool is_signed = false;
 			if (tpl_wire->port_output && !tpl_wire->port_input) {
 				new_conn.first = port_it.second;
 				new_conn.second = tpl_wire;
+				is_signed = tpl_wire->is_signed;
 			} else if (!tpl_wire->port_output && tpl_wire->port_input) {
 				new_conn.first = tpl_wire;
 				new_conn.second = port_it.second;
+				is_signed = new_conn.second.is_wire() && new_conn.second.as_wire()->is_signed;
 			} else {
 				SigSpec sig_tpl = tpl_wire, sig_mod = port_it.second;
 				for (int i = 0; i < GetSize(sig_tpl) && i < GetSize(sig_mod); i++) {
@@ -204,7 +207,7 @@ struct FlattenWorker
 			if (new_conn.second.size() > new_conn.first.size())
 				new_conn.second.remove(new_conn.first.size(), new_conn.second.size() - new_conn.first.size());
 			if (new_conn.second.size() < new_conn.first.size())
-				new_conn.second.append(RTLIL::SigSpec(RTLIL::State::S0, new_conn.first.size() - new_conn.second.size()));
+				new_conn.second.extend_u0(new_conn.first.size(), is_signed);
 			log_assert(new_conn.first.size() == new_conn.second.size());
 
 			if (sigmap(new_conn.first).has_const())

tests/various/port_sign_extend.v

@@ -0,0 +1,76 @@
+module GeneratorSigned1(out);
+	output wire signed out;
+	assign out = 1;
+endmodule
+
+module GeneratorUnsigned1(out);
+	output wire out;
+	assign out = 1;
+endmodule
+
+module GeneratorSigned2(out);
+	output wire signed [1:0] out;
+	assign out = 2;
+endmodule
+
+module GeneratorUnsigned2(out);
+	output wire [1:0] out;
+	assign out = 2;
+endmodule
+
+module PassThrough(a, b);
+	input wire [3:0] a;
+	output wire [3:0] b;
+	assign b = a;
+endmodule
+
+module act(o1, o2, o3, o4, o5, yay1, nay1, yay2, nay2);
+	output wire [3:0] o1, o2, o3, o4, o5;
+
+	// unsigned constant
+	PassThrough pt1(1'b1, o1);
+
+	// unsigned wire
+	wire tmp2;
+	assign tmp2 = 1'sb1;
+	PassThrough pt2(tmp2, o2);
+
+	// signed constant
+	PassThrough pt3(1'sb1, o3);
+
+	// signed wire
+	wire signed tmp4;
+	assign tmp4 = 1'sb1;
+	PassThrough pt4(tmp4, o4);
+
+	// signed expressions
+	wire signed [1:0] tmp5a = 2'b11;
+	wire signed [1:0] tmp5b = 2'b01;
+	PassThrough pt5(tmp5a ^ tmp5b, o5);
+
+	output wire [2:0] yay1, nay1;
+	GeneratorSigned1   os1(yay1);
+	GeneratorUnsigned1 ou1(nay1);
+
+	output wire [2:0] yay2, nay2;
+	GeneratorSigned2   os2(yay2);
+	GeneratorUnsigned2 ou2(nay2);
+endmodule
+
+module ref(o1, o2, o3, o4, o5, yay1, nay1, yay2, nay2);
+	output wire [3:0] o1, o2, o3, o4, o5;
+
+	assign o1 = 4'b0001;
+	assign o2 = 4'b0001;
+	assign o3 = 4'b1111;
+	assign o4 = 4'b1111;
+	assign o5 = 4'b1110;
+
+	output wire [2:0] yay1, nay1;
+	assign yay1 = 3'b111;
+	assign nay1 = 3'b001;
+
+	output wire [2:0] yay2, nay2;
+	assign yay2 = 3'b110;
+	assign nay2 = 3'b010;
+endmodule

tests/various/port_sign_extend.ys

@@ -0,0 +1,22 @@
+read_verilog port_sign_extend.v
+hierarchy
+flatten
+equiv_make ref act equiv
+equiv_simple
+equiv_status -assert
+
+delete
+
+read_verilog port_sign_extend.v
+flatten
+equiv_make ref act equiv
+equiv_simple
+equiv_status -assert
+
+delete
+
+read_verilog port_sign_extend.v
+hierarchy
+equiv_make ref act equiv
+prep -flatten -top equiv
+equiv_status -assert