From: whitequark <whitequark@whitequark.org>
Date: Wed, 2 Jan 2019 05:04:28 +0000 (+0000)
Subject: opt_expr: improve simplification of comparisons with large constants.
X-Git-Tag: yosys-0.9~347^2
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=bf8db55ef36f6839827cd8bc69f673fd3fd43cca;p=yosys.git

opt_expr: improve simplification of comparisons with large constants.

The idea behind this simplification is that a N-bit signal X being
compared with an M-bit constant where M>N and the constant has Nth
or higher bit set, it either always succeeds or always fails.

However, the existing implementation only worked with one-hot signals
for some reason. It also printed incorrect messages.

This commit adjusts the simplification to have as much power as
possible, and fixes other bugs.
---

diff --git a/passes/opt/opt_expr.cc b/passes/opt/opt_expr.cc
index 9abbdc6ac..76bcfd44f 100644
--- a/passes/opt/opt_expr.cc
+++ b/passes/opt/opt_expr.cc
@@ -259,6 +259,22 @@ bool is_one_or_minus_one(const Const &value, bool is_signed, bool &is_negative)
 	return last_bit_one;
 }
 
+int get_highest_hot_index(RTLIL::SigSpec signal)
+{
+	for (int i = GetSize(signal) - 1; i >= 0; i--)
+	{
+		if (signal[i] == RTLIL::State::S0)
+			continue;
+
+		if (signal[i] == RTLIL::State::S1)
+			return i;
+
+		break;
+	}
+
+	return -1;
+}
+
 // if the signal has only one bit set, return the index of that bit.
 // otherwise return -1
 int get_onehot_bit_index(RTLIL::SigSpec signal)
@@ -1345,9 +1361,6 @@ void replace_const_cells(RTLIL::Design *design, RTLIL::Module *module, bool cons
 		}
 
 		// simplify comparisons
-		// currently, only replaces comparisons with an extreme of the signal range with a constant
-		// TODO: since, unlike the following optimization, this one does not assume that both inputs to the cell are constant,
-		// it is more robust; the following optimization should be folded into this one at some point
 		if (do_fine && (cell->type == "$lt" || cell->type == "$ge" || cell->type == "$gt" || cell->type == "$le"))
 		{
 			IdString cmp_type = cell->type;
@@ -1405,29 +1418,53 @@ void replace_const_cells(RTLIL::Design *design, RTLIL::Module *module, bool cons
 						replace = true;
 					}
 
-					int const_bit_set = get_onehot_bit_index(const_sig);
-					if (const_bit_set >= 0 && const_bit_set < var_width)
+					int const_bit_hot = get_onehot_bit_index(const_sig);
+					if (const_bit_hot >= 0 && const_bit_hot < var_width)
 					{
-						RTLIL::SigSpec var_high_sig(RTLIL::State::S0, var_width - const_bit_set);
-						for (int i = const_bit_set; i < var_width; i++) {
-							var_high_sig[i - const_bit_set] = var_sig[i];
+						RTLIL::SigSpec var_high_sig(RTLIL::State::S0, var_width - const_bit_hot);
+						for (int i = const_bit_hot; i < var_width; i++) {
+							var_high_sig[i - const_bit_hot] = var_sig[i];
 						}
 
 						if (cmp_type == "$lt")
 						{
 							condition   = stringf("unsigned X<%s", log_signal(const_sig));
-							replacement = stringf("!X[%d:%d]", var_width - 1, const_bit_set);
+							replacement = stringf("!X[%d:%d]", var_width - 1, const_bit_hot);
 							module->addLogicNot(NEW_ID, var_high_sig, cell->getPort("\\Y"));
 							remove = true;
 						}
 						if (cmp_type == "$ge")
 						{
 							condition   = stringf("unsigned X>=%s", log_signal(const_sig));
-							replacement = stringf("|X[%d:%d]", var_width - 1, const_bit_set);
+							replacement = stringf("|X[%d:%d]", var_width - 1, const_bit_hot);
 							module->addReduceOr(NEW_ID, var_high_sig, cell->getPort("\\Y"));
 							remove = true;
 						}
 					}
+
+					int const_bit_set = get_highest_hot_index(const_sig);
+					if(const_bit_set >= var_width)
+					{
+						string cmp_name;
+						if (cmp_type == "$lt" || cmp_type == "$le")
+						{
+							if (cmp_type == "$lt") cmp_name = "<";
+							if (cmp_type == "$le") cmp_name = "<=";
+							condition   = stringf("unsigned X[%d:0]%s%s", var_width - 1, cmp_name.c_str(), log_signal(const_sig));
+							replacement = "constant 1";
+							replace_sig[0] = State::S1;
+							replace = true;
+						}
+						if (cmp_type == "$gt" || cmp_type == "$ge")
+						{
+							if (cmp_type == "$gt") cmp_name = ">";
+							if (cmp_type == "$ge") cmp_name = ">=";
+							condition   = stringf("unsigned X[%d:0]%s%s", var_width - 1, cmp_name.c_str(), log_signal(const_sig));
+							replacement = "constant 0";
+							replace_sig[0] = State::S0;
+							replace = true;
+						}
+					}
 				}
 				else
 				{ /* signed */
@@ -1460,66 +1497,6 @@ void replace_const_cells(RTLIL::Design *design, RTLIL::Module *module, bool cons
 			}
 		}
 
-		// replace a<0 or a>=0 with the top bit of a
-		if (do_fine && (cell->type == "$lt" || cell->type == "$ge" || cell->type == "$gt" || cell->type == "$le"))
-		{
-			//used to decide whether the signal needs to be negated
-			bool is_lt = false;
-
-			//references the variable signal in the comparison
-			RTLIL::SigSpec sigVar;
-
-			//references the constant signal in the comparison
-			RTLIL::SigSpec sigConst;
-
-			// note that this signal must be constant for the optimization
-			// to take place, but it is not checked beforehand.
-			// If new passes are added, this signal must be checked for const-ness
-
-			//width of the variable port
-			int width;
-			int const_width;
-
-			bool var_signed;
-
-			if (cell->type == "$lt" || cell->type == "$ge") {
-				is_lt = cell->type == "$lt" ? 1 : 0;
-				sigVar = cell->getPort("\\A");
-				sigConst = cell->getPort("\\B");
-				width = cell->parameters["\\A_WIDTH"].as_int();
-				const_width = cell->parameters["\\B_WIDTH"].as_int();
-				var_signed = cell->parameters["\\A_SIGNED"].as_bool();
-			} else
-			if (cell->type == "$gt" || cell->type == "$le") {
-				is_lt = cell->type == "$gt" ? 1 : 0;
-				sigVar = cell->getPort("\\B");
-				sigConst = cell->getPort("\\A");
-				width = cell->parameters["\\B_WIDTH"].as_int();
-				const_width = cell->parameters["\\A_WIDTH"].as_int();
-				var_signed = cell->parameters["\\B_SIGNED"].as_bool();
-			} else
-				log_abort();
-
-			if (sigConst.is_fully_const() && sigConst.is_fully_def() && var_signed == false)
-			{
-				int const_bit_set = get_onehot_bit_index(sigConst);
-				if(const_bit_set >= width && const_bit_set >= 0){
-					RTLIL::SigSpec a_prime(RTLIL::State::S0, 1);
-					if(is_lt){
-						a_prime[0] = RTLIL::State::S1;
-						log("Replacing %s cell `%s' (implementing unsigned X[%d:0] < %s[%d:0]) with constant 0.\n", log_id(cell->type), log_id(cell), width-1, log_signal(sigConst),const_width-1);
-					}
-					else{
-						log("Replacing %s cell `%s' (implementing unsigned X[%d:0]>= %s[%d:0]) with constant 1.\n", log_id(cell->type), log_id(cell), width-1, log_signal(sigConst),const_width-1);
-					}
-					module->connect(cell->getPort("\\Y"), a_prime);
-					module->remove(cell);
-					did_something = true;
-					goto next_cell;
-				}
-			}
-		}
-
 	next_cell:;
 #undef ACTION_DO
 #undef ACTION_DO_Y
diff --git a/tests/opt/opt_expr_cmp.v b/tests/opt/opt_expr_cmp.v
index 500b15f1b..5aff4b80f 100644
--- a/tests/opt/opt_expr_cmp.v
+++ b/tests/opt/opt_expr_cmp.v
@@ -19,4 +19,22 @@ module top(...);
   output o3_2 = 4'b0100 <= a;
   output o3_3 = a <  4'b0100;
   output o3_4 = a >= 4'b0100;
+
+  output o4_1 = 5'b10000 >  a;
+  output o4_2 = 5'b10000 >= a;
+  output o4_3 = 5'b10000 <  a;
+  output o4_4 = 5'b10000 <= a;
+  output o4_5 = a <  5'b10000;
+  output o4_6 = a <= 5'b10000;
+  output o4_7 = a >  5'b10000;
+  output o4_8 = a >= 5'b10000;
+
+  output o5_1 = 5'b10100 >  a;
+  output o5_2 = 5'b10100 >= a;
+  output o5_3 = 5'b10100 <  a;
+  output o5_4 = 5'b10100 <= a;
+  output o5_5 = a <  5'b10100;
+  output o5_6 = a <= 5'b10100;
+  output o5_7 = a >  5'b10100;
+  output o5_8 = a >= 5'b10100;
 endmodule