From de34728bf8740df4169c06ff562716e5c8699f40 Mon Sep 17 00:00:00 2001 From: whitequark Date: Fri, 4 Oct 2019 07:49:24 +0000 Subject: [PATCH] hdl.ast: prohibit signed divisors. See #238. --- nmigen/hdl/ast.py | 22 ++++++++++++++++++---- nmigen/test/test_hdl_ast.py | 21 +++++++++++++++++++-- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/nmigen/hdl/ast.py b/nmigen/hdl/ast.py index 6175300..2c997be 100644 --- a/nmigen/hdl/ast.py +++ b/nmigen/hdl/ast.py @@ -79,18 +79,34 @@ class Value(metaclass=ABCMeta): return Operator("-", [self, other]) def __rsub__(self, other): return Operator("-", [other, self]) + def __mul__(self, other): return Operator("*", [self, other]) def __rmul__(self, other): return Operator("*", [other, self]) + + def __check_divisor(self): + width, signed = self.shape() + if signed: + # Python's division semantics and Verilog's division semantics differ for negative + # divisors (Python uses div/mod, Verilog uses quo/rem); for now, avoid the issue + # completely by prohibiting such division operations. + raise NotImplementedError("Division by a signed value is not supported") def __mod__(self, other): + other = Value.wrap(other) + other.__check_divisor() return Operator("%", [self, other]) def __rmod__(self, other): + self.__check_divisor() return Operator("%", [other, self]) def __floordiv__(self, other): + other = Value.wrap(other) + other.__check_divisor() return Operator("//", [self, other]) def __rfloordiv__(self, other): + self.__check_divisor() return Operator("//", [other, self]) + def __lshift__(self, other): return Operator("<<", [self, other]) def __rlshift__(self, other): @@ -475,10 +491,8 @@ class Operator(Value): return width + 1, signed if self.op == "*": return a_width + b_width, a_signed or b_signed - if self.op == "//": - # division by -1 can overflow - return a_width + b_signed, a_signed or b_signed - if self.op == "%": + if self.op in ("//", "%"): + assert not b_signed return a_width, a_signed if self.op in ("<", "<=", "==", "!=", ">", ">="): return 1, False diff --git a/nmigen/test/test_hdl_ast.py b/nmigen/test/test_hdl_ast.py index f90d444..0bca37a 100644 --- a/nmigen/test/test_hdl_ast.py +++ b/nmigen/test/test_hdl_ast.py @@ -181,17 +181,34 @@ class OperatorTestCase(FHDLTestCase): v5 = 10 * Const(0, 4) self.assertEqual(v5.shape(), (8, False)) + def test_mod(self): + v1 = Const(0, (4, False)) % Const(0, (6, False)) + self.assertEqual(repr(v1), "(% (const 4'd0) (const 6'd0))") + self.assertEqual(v1.shape(), (4, False)) + v3 = Const(0, (4, True)) % Const(0, (4, False)) + self.assertEqual(v3.shape(), (4, True)) + v5 = 10 % Const(0, 4) + self.assertEqual(v5.shape(), (4, False)) + + def test_mod_wrong(self): + with self.assertRaises(NotImplementedError, + msg="Division by a signed value is not supported"): + Const(0, (4, True)) % Const(0, (6, True)) + def test_floordiv(self): v1 = Const(0, (4, False)) // Const(0, (6, False)) self.assertEqual(repr(v1), "(// (const 4'd0) (const 6'd0))") self.assertEqual(v1.shape(), (4, False)) - v2 = Const(0, (4, True)) // Const(0, (6, True)) - self.assertEqual(v2.shape(), (5, True)) v3 = Const(0, (4, True)) // Const(0, (4, False)) self.assertEqual(v3.shape(), (4, True)) v5 = 10 // Const(0, 4) self.assertEqual(v5.shape(), (4, False)) + def test_floordiv_wrong(self): + with self.assertRaises(NotImplementedError, + msg="Division by a signed value is not supported"): + Const(0, (4, True)) // Const(0, (6, True)) + def test_and(self): v1 = Const(0, (4, False)) & Const(0, (6, False)) self.assertEqual(repr(v1), "(& (const 4'd0) (const 6'd0))") -- 2.30.2