From 5dab327b30cb1d864297b22a15f0fce4b374a841 Mon Sep 17 00:00:00 2001
From: Clifford Wolf <clifford@clifford.at>
Date: Tue, 9 Jul 2013 23:41:28 +0200
Subject: [PATCH] More fixes in ast expression sign/width handling

---
 frontends/ast/genrtlil.cc | 17 ++++++++---------
 tests/simple/vloghammer.v | 28 +++++++++++++++-------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc
index 39f6e90ef..830778227 100644
--- a/frontends/ast/genrtlil.cc
+++ b/frontends/ast/genrtlil.cc
@@ -530,7 +530,7 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint)
 		break;
 
 	case AST_TO_UNSIGNED:
-		children.at(0)->detectSignWidthWorker(width_hint, sign_hint);
+		children.at(0)->detectSignWidthWorker(width_hint, dummy_sign_hint);
 		sign_hint = false;
 		break;
 
@@ -593,8 +593,8 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint)
 	case AST_LOGIC_AND:
 	case AST_LOGIC_OR:
 	case AST_LOGIC_NOT:
-		for (auto child : children)
-			child->detectSignWidthWorker(width_hint, sign_hint);
+		width_hint = std::max(width_hint, 1);
+		sign_hint = false;
 		break;
 
 	case AST_TERNARY:
@@ -835,7 +835,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 	// just pass thru the signal. the parent will evaluated the is_signed property and inperpret the SigSpec accordingly
 	case AST_TO_SIGNED:
 	case AST_TO_UNSIGNED: {
-			RTLIL::SigSpec sig = children[0]->genRTLIL(width_hint, sign_hint);
+			RTLIL::SigSpec sig = children[0]->genRTLIL();
 			is_signed = type == AST_TO_SIGNED;
 			return sig;
 	}
@@ -889,6 +889,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 	if (0) { case AST_BIT_XOR:  type_name = "$xor"; }
 	if (0) { case AST_BIT_XNOR: type_name = "$xnor"; }
 		{
+			if (width_hint < 0)
+				detectSignWidth(width_hint, sign_hint);
 			RTLIL::SigSpec left = children[0]->genRTLIL(width_hint, sign_hint);
 			RTLIL::SigSpec right = children[1]->genRTLIL(width_hint, sign_hint);
 			int width = std::max(left.width, right.width);
@@ -965,12 +967,9 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 			if (width > width_hint && width_hint > 0)
 				width = width_hint;
 			if (width < width_hint) {
-				if (type == AST_ADD || type == AST_SUB) {
+				if (type == AST_ADD || type == AST_SUB)
 					width++;
-					if (width < width_hint && children[0]->is_signed != children[1]->is_signed)
-						width++;
-				}
-				if (type == AST_SUB && !children[0]->is_signed && !children[1]->is_signed)
+				if (type == AST_SUB && (!children[0]->is_signed || !children[1]->is_signed))
 					width = width_hint;
 				if (type == AST_MUL)
 					width = std::min(left.width + right.width, width_hint);
diff --git a/tests/simple/vloghammer.v b/tests/simple/vloghammer.v
index 9269ff050..09987e7c9 100644
--- a/tests/simple/vloghammer.v
+++ b/tests/simple/vloghammer.v
@@ -27,19 +27,21 @@ module test04(a, y);
   assign y = ~(a - 1'b0);
 endmodule
 
-module test05(a, y);
-  input a;
-  output y;
-  assign y = 12345 >> {a, 32'd0};
-endmodule
-
-module test06(a, b, c, y);
-  input signed [3:0] a;
-  input signed [1:0] b;
-  input signed [1:0] c;
-  output [5:0] y;
-  assign y = (a >> b) >>> c;
-endmodule
+// .. this test triggers a bug in xilinx isim.
+// module test05(a, y);
+//   input a;
+//   output y;
+//   assign y = 12345 >> {a, 32'd0};
+// endmodule
+
+// .. this test triggers a bug in icarus verilog.
+// module test06(a, b, c, y);
+//   input signed [3:0] a;
+//   input signed [1:0] b;
+//   input signed [1:0] c;
+//   output [5:0] y;
+//   assign y = (a >> b) >>> c;
+// endmodule
 
 module test07(a, b, y);
   input signed [1:0] a;
-- 
2.30.2