Merge pull request #4817 from povik/macc_v2-1

macc: Stop using the B port

Makefile

@@ -869,6 +869,7 @@ endif
 SH_ABC_TEST_DIRS =
 SH_ABC_TEST_DIRS += tests/memories
 SH_ABC_TEST_DIRS += tests/aiger
+SH_ABC_TEST_DIRS += tests/alumacc
 
 # seed-tests/ is a dummy string, not a directory
 .PHONY: seed-tests

kernel/consteval.h

@@ -315,9 +315,6 @@ struct ConstEval
 			Macc macc;
 			macc.from_cell(cell);
 
-			if (!eval(macc.bit_ports, undef, cell))
-				return false;
-
 			for (auto &port : macc.ports) {
 				if (!eval(port.in_a, undef, cell))
 					return false;

kernel/macc.h

@@ -30,14 +30,11 @@ struct Macc
 		RTLIL::SigSpec in_a, in_b;
 		bool is_signed, do_subtract;
 	};
-
 	std::vector<port_t> ports;
-	RTLIL::SigSpec bit_ports;
 
 	void optimize(int width)
 	{
 		std::vector<port_t> new_ports;
-		RTLIL::SigSpec new_bit_ports;
 		RTLIL::Const off(0, width);
 
 		for (auto &port : ports)
@@ -48,11 +45,6 @@ struct Macc
 			if (GetSize(port.in_a) < GetSize(port.in_b))
 				std::swap(port.in_a, port.in_b);
 
-			if (GetSize(port.in_a) == 1 && GetSize(port.in_b) == 0 && !port.is_signed && !port.do_subtract) {
-				bit_ports.append(port.in_a);
-				continue;
-			}
-
 			if (port.in_a.is_fully_const() && port.in_b.is_fully_const()) {
 				RTLIL::Const v = port.in_a.as_const();
 				if (GetSize(port.in_b))
@@ -79,12 +71,6 @@ struct Macc
 			new_ports.push_back(port);
 		}
 
-		for (auto &bit : bit_ports)
-			if (bit == State::S1)
-				off = const_add(off, RTLIL::Const(1, width), false, false, width);
-			else if (bit != State::S0)
-				new_bit_ports.append(bit);
-
 		if (off.as_bool()) {
 			port_t port;
 			port.in_a = off;
@@ -94,7 +80,6 @@ struct Macc
 		}
 
 		new_ports.swap(ports);
-		bit_ports = new_bit_ports;
 	}
 
 	void from_cell(RTLIL::Cell *cell)
@@ -102,7 +87,6 @@ struct Macc
 		RTLIL::SigSpec port_a = cell->getPort(ID::A);
 
 		ports.clear();
-		bit_ports = cell->getPort(ID::B);
 
 		auto config_bits = cell->getParam(ID::CONFIG);
 		int config_cursor = 0;
@@ -145,6 +129,9 @@ struct Macc
 				ports.push_back(this_port);
 		}
 
+		for (auto bit : cell->getPort(ID::B))
+			ports.push_back(port_t{{bit}, {}, false, false});
+
 		log_assert(config_cursor == config_width);
 		log_assert(port_a_cursor == GetSize(port_a));
 	}
@@ -190,11 +177,11 @@ struct Macc
 		}
 
 		cell->setPort(ID::A, port_a);
-		cell->setPort(ID::B, bit_ports);
+		cell->setPort(ID::B, {});
 		cell->setParam(ID::CONFIG, config_bits);
 		cell->setParam(ID::CONFIG_WIDTH, GetSize(config_bits));
 		cell->setParam(ID::A_WIDTH, GetSize(port_a));
-		cell->setParam(ID::B_WIDTH, GetSize(bit_ports));
+		cell->setParam(ID::B_WIDTH, 0);
 	}
 
 	bool eval(RTLIL::Const &result) const
@@ -219,19 +206,12 @@ struct Macc
 				result = const_add(result, summand, port.is_signed, port.is_signed, GetSize(result));
 		}
 
-		for (auto bit : bit_ports) {
-			if (bit.wire)
-				return false;
-			result = const_add(result, bit.data, false, false, GetSize(result));
-		}
-
 		return true;
 	}
 
 	bool is_simple_product()
 	{
-		return bit_ports.empty() &&
+		return ports.size() == 1 &&
-				ports.size() == 1 &&
 				!ports[0].in_b.empty() &&
 				!ports[0].do_subtract;
 	}

kernel/satgen.cc

@@ -743,7 +743,6 @@ bool SatGen::importCell(RTLIL::Cell *cell, int timestep)
 	if (cell->type == ID($macc))
 	{
 		std::vector<int> a = importDefSigSpec(cell->getPort(ID::A), timestep);
-		std::vector<int> b = importDefSigSpec(cell->getPort(ID::B), timestep);
 		std::vector<int> y = importDefSigSpec(cell->getPort(ID::Y), timestep);
 
 		Macc macc;
@@ -785,12 +784,6 @@ bool SatGen::importCell(RTLIL::Cell *cell, int timestep)
 			}
 		}
 
-		for (int i = 0; i < GetSize(b); i++) {
-			std::vector<int> val(GetSize(y), ez->CONST_FALSE);
-			val.at(0) = b.at(i);
-			tmp = ez->vec_add(tmp, val);
-		}
-
 		if (model_undef)
 		{
 			std::vector<int> undef_a = importUndefSigSpec(cell->getPort(ID::A), timestep);

passes/opt/share.cc

@@ -117,7 +117,7 @@ struct ShareWorker
 
 	static int bits_macc(const Macc &m, int width)
 	{
-		int bits = GetSize(m.bit_ports);
+		int bits = 0;
 		for (auto &p : m.ports)
 			bits += bits_macc_port(p, width);
 		return bits;

passes/techmap/alumacc.cc

@@ -283,7 +283,7 @@ struct AlumaccWorker
 		for (auto &it : sig_macc)
 		{
 			auto n = it.second;
-			RTLIL::SigSpec A, B, C = n->macc.bit_ports;
+			RTLIL::SigSpec A, B, C;
 			bool a_signed = false, b_signed = false;
 			bool subtract_b = false;
 			alunode_t *alunode;

passes/techmap/maccmap.cc

@@ -286,19 +286,23 @@ void maccmap(RTLIL::Module *module, RTLIL::Cell *cell, bool unmap)
 			log("  %s %s * %s (%dx%d bits, %s)\n", port.do_subtract ? "sub" : "add", log_signal(port.in_a), log_signal(port.in_b),
 					GetSize(port.in_a), GetSize(port.in_b), port.is_signed ? "signed" : "unsigned");
 
-	if (GetSize(macc.bit_ports) != 0)
-		log("  add bits %s (%d bits)\n", log_signal(macc.bit_ports), GetSize(macc.bit_ports));
-
 	if (unmap)
 	{
 		typedef std::pair<RTLIL::SigSpec, bool> summand_t;
 		std::vector<summand_t> summands;
 
+		RTLIL::SigSpec bit_ports;
+
 		for (auto &port : macc.ports) {
 			summand_t this_summand;
 			if (GetSize(port.in_b)) {
 				this_summand.first = module->addWire(NEW_ID, width);
 				module->addMul(NEW_ID, port.in_a, port.in_b, this_summand.first, port.is_signed);
+			} else if (GetSize(port.in_a) == 1 && GetSize(port.in_b) == 0 && !port.is_signed && !port.do_subtract) {
+				// Mimic old 'bit_ports' treatment in case it's relevant for performance,
+				// i.e. defer single-bit summands to be the last ones
+				bit_ports.append(port.in_a);
+				continue;
 			} else if (GetSize(port.in_a) != width) {
 				this_summand.first = module->addWire(NEW_ID, width);
 				module->addPos(NEW_ID, port.in_a, this_summand.first, port.is_signed);
@@ -309,7 +313,7 @@ void maccmap(RTLIL::Module *module, RTLIL::Cell *cell, bool unmap)
 			summands.push_back(this_summand);
 		}
 
-		for (auto &bit : macc.bit_ports)
+		for (auto &bit : bit_ports)
 			summands.push_back(summand_t(bit, false));
 
 		if (GetSize(summands) == 0)
@@ -346,14 +350,20 @@ void maccmap(RTLIL::Module *module, RTLIL::Cell *cell, bool unmap)
 	else
 	{
 		MaccmapWorker worker(module, width);
+		RTLIL::SigSpec bit_ports;
 
-		for (auto &port : macc.ports)
+		for (auto &port : macc.ports) {
-			if (GetSize(port.in_b) == 0)
+			// Mimic old 'bit_ports' treatment in case it's relevant for performance,
+			// i.e. defer single-bit summands to be the last ones
+			if (GetSize(port.in_a) == 1 && GetSize(port.in_b) == 0 && !port.is_signed && !port.do_subtract)
+				bit_ports.append(port.in_a);
+			else if (GetSize(port.in_b) == 0)
 				worker.add(port.in_a, port.is_signed, port.do_subtract);
 			else
 				worker.add(port.in_a, port.in_b, port.is_signed, port.do_subtract);
+		}
 
-		for (auto &bit : macc.bit_ports)
+		for (auto bit : bit_ports)
 			worker.add(bit, 0);
 
 		module->connect(cell->getPort(ID::Y), worker.synth());

passes/tests/test_cell.cc

@@ -201,18 +201,19 @@ static RTLIL::Cell* create_gold_module(RTLIL::Design *design, RTLIL::IdString ce
 			this_port.do_subtract = xorshift32(2) == 1;
 			macc.ports.push_back(this_port);
 		}
-
+		// Macc::to_cell sets the input ports
-		wire = module->addWire(ID::B);
+		macc.to_cell(cell);
-		wire->width = xorshift32(mulbits_a ? xorshift32(4)+1 : xorshift32(16)+1);
-		wire->port_input = true;
-		macc.bit_ports = wire;
 
 		wire = module->addWire(ID::Y);
 		wire->width = width;
 		wire->port_output = true;
 		cell->setPort(ID::Y, wire);
 
-		macc.to_cell(cell);
+		// override the B input (macc helpers always sets an empty vector)
+		wire = module->addWire(ID::B);
+		wire->width = xorshift32(mulbits_a ? xorshift32(4)+1 : xorshift32(16)+1);
+		wire->port_input = true;
+		cell->setPort(ID::B, wire);
 	}
 
 	if (cell_type == ID($lut))

tests/alumacc/macc_b_port_compat.ys

@@ -0,0 +1,50 @@
+# We don't set the B port on $macc cells anymore,
+# test compatibility with older RTLIL files which can
+# have the B port populated
+
+read_verilog <<EOF
+module gold(input signed [2:0] a1, input signed [2:0] b1,
+			input [1:0] a2, input [3:0] b2, input c, input d, output signed [3:0] y);
+  wire signed [3:0] ab1;
+  assign ab1 = a1 * b1;
+  assign y = ab1 + a2*b2 + c + d + 1;
+endmodule
+EOF
+prep
+
+# test the model for $macc including the retired B parameter
+# matches the gold module
+read_rtlil <<EOF
+attribute \src "<<EOF:1.1-4.10"
+module \gate
+  wire width 3 input 1 signed \a1
+  wire width 2 input 3 \a2
+  wire width 3 input 2 signed \b1
+  wire width 4 input 4 \b2
+  wire input 5 \c
+  wire input 6 \d
+  wire width 4 output 7 signed \y
+
+  cell $macc $1
+    parameter \A_WIDTH 12
+    parameter \B_WIDTH 3
+    parameter \CONFIG 20'01010000011011010011
+    parameter \CONFIG_WIDTH 20
+    parameter \Y_WIDTH 4
+    connect \A { \a2 \b2 \b1 \a1 }
+    connect \B { \d \c 1'1 }
+    connect \Y \y
+  end
+end
+EOF
+
+design -save gold_gate
+equiv_make gold gate equiv
+equiv_induct equiv
+equiv_status -assert equiv
+
+design -load gold_gate
+maccmap gate
+equiv_make gold gate equiv
+equiv_induct equiv
+equiv_status -assert equiv

tests/alumacc/run-test.sh

@@ -0,0 +1,6 @@
+#!/usr/bin/env bash
+set -e
+for x in *.ys; do
+  echo "Running $x.."
+  ../../yosys -ql ${x%.ys}.log $x
+done