nmigen icon indicating copy to clipboard operation
nmigen copied to clipboard

Improve readability of assignments and add contexts for clock domains

Open ildus opened this issue 3 years ago • 0 comments

HI, I started having a look to nmigen and I see a lot of eq statements which are decrease readability of clauses. I think there is a room of improvement. So I created a little of POC patch which adds contexts for clock domains. Please tell me if it was discussed before.

That patch is using slicing mechanism to assign new values to signals. The [:] construction is used to assign to whole variable, and slices with numbers to assign to the part of values. This works if the context was created for a clock domain. The modifed ALU example as a demonstration:

    def elaborate(self, platform):
        m = Module()

        with m.Domain(sync=True):
            with m.If(self.sel == 0b00):
                self.o[:] = self.a | self.b
            with m.Elif(self.sel == 0b01):
                self.o[0:5] = (self.a & self.b)[0:5]
            with m.Elif(self.sel == 0b10):
                self.o[:] = self.a ^ self.b
            with m.Else():
                cat = Cat(self.o, self.co)
                cat[:] = self.a - self.b

        return m

The patch itself:

diff --git a/nmigen/hdl/ast.py b/nmigen/hdl/ast.py
index af1d831..1f41db5 100644
--- a/nmigen/hdl/ast.py
+++ b/nmigen/hdl/ast.py
@@ -24,6 +24,21 @@ __all__ = [
 ]


+class CurrentClockDomain:
+    _current_clock_domain = None
+
+    @classmethod
+    def set_current(cls, domain):
+        if domain is not None and cls._current_clock_domain is not None:
+            raise SyntaxError("clock domain already set")
+
+        cls._current_clock_domain = domain
+
+    @classmethod
+    def current(cls):
+        return cls._current_clock_domain
+
+
 class DUID:
     """Deterministic Unique IDentifier."""
     __next_uid = 0
@@ -239,6 +254,15 @@ class Value(metaclass=ABCMeta):
         else:
             raise TypeError("Cannot index value with {}".format(repr(key)))

+    def __setitem__(self, key, value):
+        item = self.__getitem__(key)
+
+        domain = CurrentClockDomain.current()
+        if domain is None:
+            raise ValueError("not in clock domain context")
+
+        domain += Assign(item, value, src_loc_at=0)
+
     def as_unsigned(self):
         """Conversion to unsigned.

diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py
index 85ae472..0f35678 100644
--- a/nmigen/hdl/dsl.py
+++ b/nmigen/hdl/dsl.py
@@ -301,6 +301,19 @@ class Module(_ModuleBuilderRoot, Elaboratable):
             self._ctrl_context = None
         self._pop_ctrl()

+    @contextmanager
+    def Domain(self, domain=None, sync=True):
+        from nmigen.hdl.ast import CurrentClockDomain
+
+        if domain is None and not sync:
+            domain = self.d.comb
+        elif domain is None and sync:
+            domain = self.d.sync
+
+        CurrentClockDomain.set_current(domain)
+        yield
+        CurrentClockDomain.set_current(None)
+

The implementation could be different but I think the idea is shown.

ildus avatar Apr 05 '21 22:04 ildus