Skip to content

Improve readability of assignments and add contexts for clock domains #345

@ildus

Description

@ildus

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions