hdl.ir: rework named port handling for Instances.

The main purpose of this rework is cleanup, to avoid specifying
the direction of input ports in an implicit, ad-hoc way using
the named ports and ports dictionaries.

While working on this I realized that output ports can be connected
to anything that is valid on LHS, so this is now supported too.

diff --git a/nmigen/back/rtlil.py b/nmigen/back/rtlil.py
index 8ec4f562dbacbc4bf35234cb36877c8f482a8f68..b5a0efc1fe8981a3389a7f08d06d77972e66d47a 100644 (file)
--- a/nmigen/back/rtlil.py
+++ b/nmigen/back/rtlil.py
@@ -669,7 +669,7 @@ class _StatementCompiler(xfrm.StatementVisitor):
 def convert_fragment(builder, fragment, hierarchy):
     if isinstance(fragment, ir.Instance):
         port_map = OrderedDict()
-        for port_name, value in fragment.named_ports.items():
+        for port_name, (value, dir) in fragment.named_ports.items():
             port_map["\\{}".format(port_name)] = value
 
         if fragment.type[0] == "$":

diff --git a/nmigen/hdl/ir.py b/nmigen/hdl/ir.py
index 164f5582ba696a95c08306d92f5d936706cbc48e..c5879690ac3fd7ae217899d7ca76f90d5d181021 100644 (file)
--- a/nmigen/hdl/ir.py
+++ b/nmigen/hdl/ir.py
@@ -344,16 +344,19 @@ class Fragment:
         # Collect all signals we're driving (on LHS of statements), and signals we're using
         # (on RHS of statements, or in clock domains).
         if isinstance(self, Instance):
-            # Named ports contain signals for input, output and bidirectional ports. Output
-            # and bidirectional ports are already added to the main port dict, however, for
-            # input ports this has to be done lazily as any expression is valid there, including
-            # ones with deferred resolution to signals, such as ClockSignal().
             self_driven = SignalSet()
             self_used   = SignalSet()
-            for named_port_used in union((p._rhs_signals() for p in self.named_ports.values()),
-                                         start=SignalSet()):
-                if named_port_used not in self.ports:
-                    self_used.add(named_port_used)
+            for port_name, (value, dir) in self.named_ports.items():
+                if dir == "i":
+                    for signal in value._rhs_signals():
+                        self_used.add(signal)
+                        self.add_ports(signal, dir="i")
+                if dir == "o":
+                    for signal in value._lhs_signals():
+                        self_driven.add(signal)
+                        self.add_ports(signal, dir="o")
+                if dir == "io":
+                    self.add_ports(value, dir="io")
         else:
             self_driven = union((s._lhs_signals() for s in self.statements), start=SignalSet())
             self_used   = union((s._rhs_signals() for s in self.statements), start=SignalSet())
@@ -415,24 +418,19 @@ class Instance(Fragment):
     def __init__(self, type, **kwargs):
         super().__init__()
 
-        self.type = type
-        self.parameters = OrderedDict()
+        self.type        = type
+        self.parameters  = OrderedDict()
         self.named_ports = OrderedDict()
 
         for kw, arg in kwargs.items():
             if kw.startswith("p_"):
                 self.parameters[kw[2:]] = arg
             elif kw.startswith("i_"):
-                self.named_ports[kw[2:]] = arg
-                # Unlike with "o_" and "io_", "i_" ports can be assigned an arbitrary value;
-                # this includes unresolved ClockSignals etc. We rely on Fragment.prepare to
-                # populate fragment ports for these named ports.
+                self.named_ports[kw[2:]] = (arg, "i")
             elif kw.startswith("o_"):
-                self.named_ports[kw[2:]] = arg
-                self.add_ports(arg, dir="o")
+                self.named_ports[kw[2:]] = (arg, "o")
             elif kw.startswith("io_"):
-                self.named_ports[kw[3:]] = arg
-                self.add_ports(arg, dir="io")
+                self.named_ports[kw[3:]] = (arg, "io")
             else:
                 raise NameError("Instance argument '{}' does not start with p_, i_, o_, or io_"
                                 .format(arg))

diff --git a/nmigen/hdl/xfrm.py b/nmigen/hdl/xfrm.py
index c554bc6d9678111412e56a1e31b91918b8a7a7a8..7b667646411f4810ce1ce867b0add6c8d20a75bb 100644 (file)
--- a/nmigen/hdl/xfrm.py
+++ b/nmigen/hdl/xfrm.py
@@ -244,8 +244,8 @@ class FragmentTransformer:
 
     def map_named_ports(self, fragment, new_fragment):
         if hasattr(self, "on_value"):
-            for name, value in fragment.named_ports.items():
-                new_fragment.named_ports[name] = self.on_value(value)
+            for name, (value, dir) in fragment.named_ports.items():
+                new_fragment.named_ports[name] = self.on_value(value), dir
         else:
             new_fragment.named_ports = OrderedDict(fragment.named_ports.items())
 

diff --git a/nmigen/test/test_hdl_ir.py b/nmigen/test/test_hdl_ir.py
index 4c59917520ddd13cb9d2c186111d9fe8a36da6e1..f9f6ab8f3bbb7614609eee41eb381cdb2da9ef51 100644 (file)
--- a/nmigen/test/test_hdl_ir.py
+++ b/nmigen/test/test_hdl_ir.py
@@ -531,11 +531,14 @@ class InstanceTestCase(FHDLTestCase):
         self.rst = Signal()
         self.stb = Signal()
         self.pins = Signal(8)
+        self.datal = Signal(4)
+        self.datah = Signal(4)
         self.inst = Instance("cpu",
             p_RESET=0x1234,
             i_clk=ClockSignal(),
             i_rst=self.rst,
             o_stb=self.stb,
+            o_data=Cat(self.datal, self.datah),
             io_pins=self.pins
         )
 
@@ -544,22 +547,18 @@ class InstanceTestCase(FHDLTestCase):
         f = self.inst
         self.assertEqual(f.type, "cpu")
         self.assertEqual(f.parameters, OrderedDict([("RESET", 0x1234)]))
-        self.assertEqual(list(f.named_ports.keys()), ["clk", "rst", "stb", "pins"])
-        self.assertEqual(f.ports, SignalDict([
-            (self.stb, "o"),
-            (self.pins, "io"),
-        ]))
+        self.assertEqual(list(f.named_ports.keys()), ["clk", "rst", "stb", "data", "pins"])
+        self.assertEqual(f.ports, SignalDict([]))
 
     def test_prepare(self):
         self.setUp_cpu()
         f = self.inst.prepare()
         clk = f.domains["sync"].clk
-        self.assertEqual(f.type, "cpu")
-        self.assertEqual(f.parameters, OrderedDict([("RESET", 0x1234)]))
-        self.assertEqual(list(f.named_ports.keys()), ["clk", "rst", "stb", "pins"])
         self.assertEqual(f.ports, SignalDict([
             (clk, "i"),
             (self.rst, "i"),
             (self.stb, "o"),
+            (self.datal, "o"),
+            (self.datah, "o"),
             (self.pins, "io"),
         ]))