From 68815bf4f32c5b3c36bdfd0b727ec3e6d1269d18 Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Mon, 9 May 2022 12:00:23 -0700 Subject: [PATCH 1/7] Support wiring Bit and Bits[1] Two implementation issues: 1. Should we support wiring T to Array[1, T] in general? Or have this be a Bits/Bit specific feature 2. Is there a way to avoid a circular import in this logic? The naive solution is to have Bit check if the driver is Bits[1], but this clearly requires a circular import. Perhaps we could use some sort of `T.convert_to(other_T)` API? --- magma/bit.py | 24 +++++++++++++++++++----- magma/bits.py | 9 ++++++++- tests/test_wire/test_wireable.py | 15 +++++++++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/magma/bit.py b/magma/bit.py index e31bc9eca..b89b6d864 100644 --- a/magma/bit.py +++ b/magma/bit.py @@ -3,7 +3,6 @@ * Subtype of the Digital type * Implementation of hwtypes.AbstractBit """ -import keyword import typing as tp import functools import hwtypes as ht @@ -15,10 +14,13 @@ from magma.compatibility import IntegerTypes from magma.debug import debug_wire from magma.family import get_family -from magma.interface import IO -from magma.language_utils import primitive_to_python -from magma.protocol_type import magma_type, MagmaProtocol +from magma.logging import root_logger +from magma.protocol_type import magma_value from magma.operator_utils import output_only +from magma.wire_container import WiringLog + + +_logger = root_logger() def bit_cast(fn: tp.Callable[['Bit', 'Bit'], 'Bit']) -> \ @@ -38,7 +40,15 @@ def wrapped(self: 'Bit', other: tp.Union['Bit', bool]) -> 'Bit': _IMPLICITLY_COERCED_ITE_TYPES = (int, ht.BitVector, ht.Bit) -class Bit(Digital, AbstractBit, metaclass=DigitalMeta): +class BitMeta(DigitalMeta): + def is_wireable(cls, rhs): + import magma as m + if issubclass(rhs, m.Array) and len(rhs) == 1: + return cls.is_wireable(rhs.T) + return super().is_wireable(rhs) + + +class Bit(Digital, AbstractBit, metaclass=BitMeta): __hash__ = Digital.__hash__ @staticmethod @@ -113,9 +123,13 @@ def ite(self, t_branch, f_branch): @debug_wire def wire(self, o, debug_info): + o = magma_value(o) # Cast to Bit here so we don't get a Digital instead if isinstance(o, (IntegerTypes, bool, ht.Bit)): o = Bit(o) + import magma as m + if isinstance(o, m.Array) and len(o) == 1: + o = o[0] return super().wire(o, debug_info) diff --git a/magma/bits.py b/magma/bits.py index 4fbd6a52b..f0b6e41fe 100644 --- a/magma/bits.py +++ b/magma/bits.py @@ -200,8 +200,11 @@ def is_wireable(cls, rhs): return True if issubclass(cls, UInt) and issubclass(rhs, SInt): return False - elif issubclass(cls, SInt) and issubclass(rhs, UInt): + if issubclass(cls, SInt) and issubclass(rhs, UInt): return False + if len(cls) == 1 and issubclass(rhs, Bit): + # TODO(leonardt): Should we make this work for general Array[1, T]? + return True return super().is_wireable(rhs) @@ -247,6 +250,10 @@ def wire(self, other, debug_info): f"(bit_length={other.bit_length()}) to Bits ({len(self)})") from .conversions import bits other = bits(other, len(self)) + if isinstance(other, Bit) and len(self) == 1: + # TODO(leonardt): Should we make this work for general Array[1, T]? + from .conversions import bits + other = bits(other, 1) super().wire(other, debug_info) @classmethod diff --git a/tests/test_wire/test_wireable.py b/tests/test_wire/test_wireable.py index 4735b3311..67f16086e 100644 --- a/tests/test_wire/test_wireable.py +++ b/tests/test_wire/test_wireable.py @@ -1,3 +1,4 @@ +import pytest import magma as m @@ -20,3 +21,17 @@ class Main2(m.Circuit): Cannot wire Main2.a (Out(SInt[16])) to Main2.b (In(UInt[16]))\ """ assert caplog.messages[1][-len(expected):] == expected + + +@pytest.mark.parametrize('Ts', [ + (m.Bit, m.Bits[1]), + (m.Bits[1], m.Bit), +]) +def test_bit_bits1(Ts): + class Main(m.Circuit): + io = m.IO(a=m.In(Ts[0]), b=m.Out(Ts[1])) + io.b @= io.a + + # NOTE: We call compile here to ensure a wiring error was not reported + # (otherwise it would raise an exception) + m.compile('build/Main', Main) From 0d696c2056e558dd1b5005da942d0e24b1016b7b Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Mon, 9 May 2022 12:02:39 -0700 Subject: [PATCH 2/7] Remove unused imports --- magma/bit.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/magma/bit.py b/magma/bit.py index b89b6d864..fe079d23a 100644 --- a/magma/bit.py +++ b/magma/bit.py @@ -14,13 +14,8 @@ from magma.compatibility import IntegerTypes from magma.debug import debug_wire from magma.family import get_family -from magma.logging import root_logger from magma.protocol_type import magma_value from magma.operator_utils import output_only -from magma.wire_container import WiringLog - - -_logger = root_logger() def bit_cast(fn: tp.Callable[['Bit', 'Bit'], 'Bit']) -> \ From 25598d49ba06f2b2d23926fddb09c2ea6b9e0bed Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Mon, 9 May 2022 12:19:19 -0700 Subject: [PATCH 3/7] Fix regressiong --- magma/bit.py | 17 ++++++++--------- tests/test_circuit/test_new_style_syntax.py | 10 +++++----- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/magma/bit.py b/magma/bit.py index fe079d23a..7dd6125f4 100644 --- a/magma/bit.py +++ b/magma/bit.py @@ -35,15 +35,7 @@ def wrapped(self: 'Bit', other: tp.Union['Bit', bool]) -> 'Bit': _IMPLICITLY_COERCED_ITE_TYPES = (int, ht.BitVector, ht.Bit) -class BitMeta(DigitalMeta): - def is_wireable(cls, rhs): - import magma as m - if issubclass(rhs, m.Array) and len(rhs) == 1: - return cls.is_wireable(rhs.T) - return super().is_wireable(rhs) - - -class Bit(Digital, AbstractBit, metaclass=BitMeta): +class Bit(Digital, AbstractBit, metaclass=DigitalMeta): __hash__ = Digital.__hash__ @staticmethod @@ -127,6 +119,13 @@ def wire(self, o, debug_info): o = o[0] return super().wire(o, debug_info) + @classmethod + def is_wireable(cls, rhs): + import magma as m + if issubclass(rhs, m.Array) and len(rhs) == 1: + return cls.is_wireable(rhs.T) + return DigitalMeta.is_wireable(cls, rhs) + BitIn = Bit[Direction.In] BitOut = Bit[Direction.Out] diff --git a/tests/test_circuit/test_new_style_syntax.py b/tests/test_circuit/test_new_style_syntax.py index 33acaccff..12723607a 100644 --- a/tests/test_circuit/test_new_style_syntax.py +++ b/tests/test_circuit/test_new_style_syntax.py @@ -99,7 +99,7 @@ def definition(io): def test_defn_wiring_error(caplog): class _Foo(m.Circuit): - io = m.IO(I=m.In(m.Bit), O=m.In(m.Bit), O1=m.Out(m.Bits[1])) + io = m.IO(I=m.In(m.Bit), O=m.In(m.Bit), O1=m.Out(m.Bits[2])) m.wire(io.I, io.O) m.wire(io.I, io.O1) @@ -108,13 +108,13 @@ class _Foo(m.Circuit): assert has_error(caplog, "Cannot wire _Foo.I (Out(Bit)) to _Foo.O (Out(Bit))") assert has_error(caplog, - "Cannot wire _Foo.I (Out(Bit)) to _Foo.O1 (In(Bits[1]))") + "Cannot wire _Foo.I (Out(Bit)) to _Foo.O1 (In(Bits[2]))") @wrap_with_context_manager(logging_level("DEBUG")) def test_inst_wiring_error(caplog): class _Bar(m.Circuit): - io = m.IO(I=m.In(m.Bits[1]), O=m.Out(m.Bits[1])) + io = m.IO(I=m.In(m.Bits[2]), O=m.Out(m.Bits[2])) class _Foo(m.Circuit): io = m.IO(I=m.In(m.Bit), O=m.Out(m.Bit)) @@ -125,10 +125,10 @@ class _Foo(m.Circuit): assert has_error( caplog, - "Cannot wire _Foo.I (Out(Bit)) to _Foo._Bar_inst0.I (In(Bits[1]))") + "Cannot wire _Foo.I (Out(Bit)) to _Foo._Bar_inst0.I (In(Bits[2]))") assert has_error( caplog, - "Cannot wire _Foo._Bar_inst0.O (Out(Bits[1])) to _Foo.O (In(Bit))") + "Cannot wire _Foo._Bar_inst0.O (Out(Bits[2])) to _Foo.O (In(Bit))") assert has_error(caplog, "_Foo.O not driven") assert has_debug(caplog, "_Foo.O: Unconnected") assert has_error(caplog, "_Foo._Bar_inst0.I not driven") From 83d6def1c2afe96dcc050f398fb9656f6b303c59 Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Tue, 10 May 2022 14:43:40 -0700 Subject: [PATCH 4/7] Remove todo --- magma/bits.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/magma/bits.py b/magma/bits.py index f0b6e41fe..1e03d1516 100644 --- a/magma/bits.py +++ b/magma/bits.py @@ -203,7 +203,6 @@ def is_wireable(cls, rhs): if issubclass(cls, SInt) and issubclass(rhs, UInt): return False if len(cls) == 1 and issubclass(rhs, Bit): - # TODO(leonardt): Should we make this work for general Array[1, T]? return True return super().is_wireable(rhs) @@ -240,6 +239,7 @@ def __int__(self): @debug_wire def wire(self, other, debug_info): + from .conversions import bits if isinstance(other, (IntegerTypes, BitVector)): N = (other.bit_length() if isinstance(other, IntegerTypes) @@ -248,11 +248,8 @@ def wire(self, other, debug_info): raise ValueError( f"Cannot convert integer {other} " f"(bit_length={other.bit_length()}) to Bits ({len(self)})") - from .conversions import bits other = bits(other, len(self)) if isinstance(other, Bit) and len(self) == 1: - # TODO(leonardt): Should we make this work for general Array[1, T]? - from .conversions import bits other = bits(other, 1) super().wire(other, debug_info) From 5115d02f3861c66afbca7975c26e3f5ae8841cf8 Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Tue, 10 May 2022 14:49:11 -0700 Subject: [PATCH 5/7] Remove circular dependency --- magma/bit.py | 6 ++---- magma/bits.py | 3 +++ magma/t.py | 3 +++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/magma/bit.py b/magma/bit.py index 7dd6125f4..a7f3ecc93 100644 --- a/magma/bit.py +++ b/magma/bit.py @@ -114,15 +114,13 @@ def wire(self, o, debug_info): # Cast to Bit here so we don't get a Digital instead if isinstance(o, (IntegerTypes, bool, ht.Bit)): o = Bit(o) - import magma as m - if isinstance(o, m.Array) and len(o) == 1: + if type(o).is_bits_1(): o = o[0] return super().wire(o, debug_info) @classmethod def is_wireable(cls, rhs): - import magma as m - if issubclass(rhs, m.Array) and len(rhs) == 1: + if rhs.is_bits_1(): return cls.is_wireable(rhs.T) return DigitalMeta.is_wireable(cls, rhs) diff --git a/magma/bits.py b/magma/bits.py index 1e03d1516..fdd22ba6e 100644 --- a/magma/bits.py +++ b/magma/bits.py @@ -206,6 +206,9 @@ def is_wireable(cls, rhs): return True return super().is_wireable(rhs) + def is_bits_1(cls): + return len(cls) == 1 + class Bits(Array, AbstractBitVector, metaclass=BitsMeta): __hash__ = Array.__hash__ diff --git a/magma/t.py b/magma/t.py index bf439512c..5d045813d 100644 --- a/magma/t.py +++ b/magma/t.py @@ -177,6 +177,9 @@ def undirected_t(cls): def is_directed(cls): return cls is not cls.qualify(Direction.Undirected) + def is_bits_1(self): + return False + @lru_cache() def In(T): From 024f6a2a92177ad9f5453171b293c83de0adafca Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Tue, 10 May 2022 14:51:13 -0700 Subject: [PATCH 6/7] Simplify --- magma/bit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/magma/bit.py b/magma/bit.py index a7f3ecc93..8fac3e267 100644 --- a/magma/bit.py +++ b/magma/bit.py @@ -121,7 +121,7 @@ def wire(self, o, debug_info): @classmethod def is_wireable(cls, rhs): if rhs.is_bits_1(): - return cls.is_wireable(rhs.T) + return True return DigitalMeta.is_wireable(cls, rhs) From acb8b8b88b00218db7a5beb8c6f979e507ec11bd Mon Sep 17 00:00:00 2001 From: Lenny Truong Date: Wed, 11 May 2022 13:48:06 -0700 Subject: [PATCH 7/7] Add isinstance check --- magma/bit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/magma/bit.py b/magma/bit.py index 8fac3e267..6731d7ff3 100644 --- a/magma/bit.py +++ b/magma/bit.py @@ -7,7 +7,7 @@ import functools import hwtypes as ht from hwtypes.bit_vector_abc import AbstractBit, TypeFamily -from .t import Direction +from .t import Direction, Type from .digital import Digital, DigitalMeta from .digital import VCC, GND # TODO(rsetaluri): only here for b.c. @@ -120,7 +120,7 @@ def wire(self, o, debug_info): @classmethod def is_wireable(cls, rhs): - if rhs.is_bits_1(): + if issubclass(rhs, Type) and rhs.is_bits_1(): return True return DigitalMeta.is_wireable(cls, rhs)