This

from nmigen import *
from nmigen.cli import main

m = Module()

a = Signal(1)
array = Array(x for x in range(2))

with m.If(0 == array[a]):
    m.d.sync += a.eq(0),        

if __name__ == "__main__":
    main(m, ports=[a])

gives a strange error (https://hastebin.com/cedapovure.sql) when running generate

while

from nmigen import *
from nmigen.cli import main

m = Module()

a = Signal(1)
array = Array(x for x in range(2))

dummy = Signal(1)
m.d.comb += a.eq(dummy)

with m.If(0 == array[a]):
    m.d.sync += dummy.eq(0),
        

if __name__ == "__main__":
    main(m, ports=[a])

works.

I am not sure how to interpret this error and it seems like a bug in nmigen.

Right now nMigen silently generates invalid Verilog for that. It should (eventually) auto-flatten the hierarchy, but at least this should be detected.

A huge number of issues in the old migen are caused by forgetting to assign something to xxx.submodules.

Is there any reason that we can't do something like this?

class Module:
    def __setattr__(self, k, v):
        if isinstance(v, Module):
           self.add_module(k, v)
        object.__setattr__(self, k, v)

Any of these cases are unreachable by definition. Should we at least warn?

I know @jordens expressed in the past a desire to simplify the boilerplate around .get_fragment. Also, there is currently both Module.lower and Module.get_fragment (aliased to one another), which is confusing and unnecessary. Also#2, Migen has a Module.get_fragment() and combining Migen modules with nMigen modules gets even more confusing with that.

The design I see for the function currently named .get_fragment() is as follows:

• There are 2 kinds of synthesizable entities in nMigen: fragments (which are directly synthesizable) and anything that has .get_fragment(platform) (which is iterated to a fixed point, i.e. a fragment, when a fragment is required).
• Instances, memory ports, etc, are synthesizable. Which class they actually are is an implementation detail that is subject to change. (Instances are currently just fragments, memory ports are significantly more complex due to some mismatch between Yosys and Migen representation of ports, etc).
• There is a Fragment.get(obj, platform) function that iterates an object to a fixed point with .get_fragment(platform) or fails in a nice way.

Modules are in a somewhat weird space here. Modules should be synthesizable so that you can replace:

sm = Module()
# ... operate on sm
m = Module()
m.submodules += sm

with:

class SubCircuit:
    def get_fragment(self, platform):
        m = Module()
        return m.lower(platform)
sm = SubCircuit()
m = Module()
m.submodules += sm

that is without having to do downstream changes after replacing a Module with a user class. This means that in principle you can directly return a module, like return m.

But this means two things:

1. The actual get_fragment call for that module is delayed. The conversion of a module to a fragment is mostly (but not entirely) error-free, so delaying this call means some errors lose context. I think it might be more useful to make Module.get_fragment actually never fail, and permit return m.
2. If a parent module directly calls get_fragment on a submodule, it gets a Module object with the DSL enabled, as opposed to a Fragment object. So it can potentially abuse the DSL that way. This isn't very likely so maybe we should just ignore it.

And finally, there is an important question of what to rename .get_fragment too. I don't want something like .build because it is ambiguous and prone to collisions. What about .synthesize?

See YosysHQ/nextpnr#280.

From IRC, when I run:

class PulseSynchronizerTestCase(FHDLTestCase):
    def test_basic(self):
        m = Module()
        m.domains += ClockDomain("one")
        m.domains += ClockDomain("two")
 
        with Simulator(m, vcd_file=open("test.vcd", "w")) as sim:
            sim.add_clock(2e-6, domain="one")
            sim.add_clock(3e-6, domain="one")
            def process():
                yield Tick(domain="one")
                yield Tick(domain="two")
            sim.add_process(process)
            sim.run()

The bug is sim.add_clock(3e-6, domain="one"), instead of domain="two". However, instead of a nice error, I get:

self = <nmigen.back.pysim.Simulator object at 0x7fb1836445f8>, run_passive = False

    def step(self, run_passive=False):
        # Are there any delta cycles we should run?
        if self._state.curr_dirty.any():
            # We might run some delta cycles, and we have simulator processes waiting on
            # a deadline. Take care to not exceed the closest deadline.
            if self._wait_deadline and \
                    (self._timestamp + self._delta) >= min(self._wait_deadline.values()):
                # Oops, we blew the deadline. We *could* run the processes now, but this is
                # virtually certainly a logic loop and a design bug, so bail out instead.d
>               raise DeadlineError("Delta cycles exceeded process deadline; combinatorial loop?")
E               nmigen.back.pysim.DeadlineError: Delta cycles exceeded process deadline; combinatorial loop?

nmigen/back/pysim.py:755: DeadlineError

May you accept a pull request to prevent potential issues when using with tox?
As described in [1] best practice is to use a structure like this if used with tox [2]:

setup.py
src/
    mypkg/
        __init__.py
        app.py
        view.py
tests/
    __init__.py
    foo/
        __init__.py
        test_view.py
    bar/
        __init__.py
        test_view.py

The proposed project structure is used for instance in [3].

[1] Pytest:: Choosing a test layout / import rules
[2] Welcome to the tox automation project
[3] migen-axi

These are:

• Part
• Array
• Record
• Instance
• Memory
• FSM
• Tristate

The following valid migen code...

from nmigen import *
from nmigen.cli import main


input = Signal()
output = Signal()

m = Module()
m.d.comb += output.eq(input)

main(m, ports=[input, output])

... produces the following invalid verilog (with python fail.py generate:

/* Generated by Yosys 0.8+143 (git sha1 c82aa49d, gcc 8.2.1 -march=x86-64 -mtune=generic -O2 -fno-plt -fPIC -Os) */

(* top =  1  *)
(* generator = "nMigen" *)
module top(output, input);
  (* src = "fail.py:6" *)
  reg \$next\output ;
  (* src = "fail.py:5" *)
  input input;
  (* src = "fail.py:6" *)
  output output;
  always @* begin
    \$next\output  = 1'h0;
    \$next\output  = input;
  end
  assign output = \$next\output ;
endmodule

I dont really know, whether this should be handled by yosys (since nMigen produces valid ir) or by nMigen itself.

There's no way to specify a clock domain for XDR pins, so they're effectively unusable. Also, unregistered I/O should use XDR 0, not XDR 1 as it currently is.

Example:

from nmigen import *
from nmigen.back import rtlil


class A(Elaboratable):
    def elaborate(self, platform):
        m = Module()

        o = Signal(2)
        m.d.comb += o[0].eq(1)
        m.submodules += Instance("FOO", o_O=o[1])

        return m


a = A()
print(rtlil.convert(a))

Output:

Traceback (most recent call last):
  File "/tmp/test.py", line 17, in <module>
    print(rtlil.convert(a))
  File "/home/jf/src/nmigen/nmigen/back/rtlil.py", line 886, in convert
    fragment = fragment.prepare(**kwargs)
  File "/home/jf/src/nmigen/nmigen/hdl/ir.py", line 492, in prepare
    fragment._propagate_ports(ports=(), all_undef_as_ports=True)
  File "/home/jf/src/nmigen/nmigen/hdl/ir.py", line 419, in _propagate_ports
    self._prepare_use_def_graph(parent, level, uses, defs, ios, self)
  File "/home/jf/src/nmigen/nmigen/hdl/ir.py", line 393, in _prepare_use_def_graph
    subfrag._prepare_use_def_graph(parent, level, uses, defs, ios, top)
  File "/home/jf/src/nmigen/nmigen/hdl/ir.py", line 374, in _prepare_use_def_graph
    add_defs(value._lhs_signals())
  File "/home/jf/src/nmigen/nmigen/hdl/ir.py", line 361, in add_defs
    assert defs[sig] is self
AssertionError

A memory with a read port but no write port leads to a mysterious KeyError when simulated. Small example:

from nmigen import Module, Memory
from nmigen.back import pysim


def test_mem():
    m = Module()
    mem = Memory(width=8, depth=4, init=[0xaa, 0x55])
    m.submodules.rdport = rdport = mem.read_port()
    # Uncomment to fix this test
    # m.submodules.wrport = mem.write_port()
    with pysim.Simulator(m.lower(None)) as sim:
        def process():
            yield rdport.addr.eq(1)
            yield
            yield
            assert (yield rdport.data) == 0x55
        sim.add_clock(1e-6)
        sim.add_sync_process(process)
        sim.run()


if __name__ == "__main__":
    test_mem()

As shown, this example results in the following traceback:

Traceback (most recent call last):
  File "rom.py", line 22, in <module>
    test_mem()
  File "rom.py", line 10, in test_mem
    with pysim.Simulator(m.lower(None)) as sim:
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 551, in __enter__
    funclet = compiler(statements)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 179, in __call__
    return self.on_statement(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 174, in on_statement
    return self.on_statements(stmt)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in on_statements
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in <listcomp>
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 172, in on_statement
    return self.on_Switch(stmt)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 319, in on_Switch
    cases.append((make_test(mask, value), self.on_statements(stmts)))
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in on_statements
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in <listcomp>
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 165, in on_statement
    return self.on_Assign(stmt)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 295, in on_Assign
    rhs   = self.rrhs_compiler(stmt.rhs)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 100, in __call__
    return self.on_value(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 92, in on_value
    new_value = self.on_ArrayProxy(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 201, in on_ArrayProxy
    elems  = list(map(self, value.elems))
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 100, in __call__
    return self.on_value(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 73, in on_value
    new_value = self.on_Signal(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 93, in on_Signal
    value_slot = self.signal_slots[value]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/ast.py", line 961, in __getitem__
    return self._storage[key]
KeyError: <nmigen.hdl.ast.SignalKey (sig mem(0))>

Uncommenting the write_port fixes the example which then works as expected.

When nMigen elaborates an asynchronous read port, it does not wire the latter to a clock signal.

The following nMigen code instantiates an asynchronous and transparent read port:

from nmigen import *
from nmigen.back import verilog


class Foo:
    def __init__(self):
        mem = Memory(4, 4)
        self.rp = mem.read_port(synchronous=False, transparent=True)

    def elaborate(self, platform):
        m = Module()
        m.submodules += self.rp
        return m


foo = Foo()
frag = foo.elaborate(platform=None)
print(verilog.convert(frag))

which produces the following (broken?) output:

/* Generated by Yosys 0.8+147 (git sha1 266511b2, gcc 8.2.1 -fPIC -Os) */

(* top =  1  *)
(* generator = "nMigen" *)
module top(mem_r_addr);
  (* src = "/home/jf/src/nmigen/nmigen/hdl/mem.py:86" *)
  input [1:0] mem_r_addr;
  (* src = "/home/jf/src/nmigen/nmigen/hdl/mem.py:88" *)
  wire [3:0] mem_r_data;
  reg [3:0] mem [3:0];
  initial begin
    mem[0] = 4'h0;
    mem[1] = 4'h0;
    mem[2] = 4'h0;
    mem[3] = 4'h0;
  end
  reg [1:0] _0_;
  always @(posedge 1'h0) begin
    _0_ <= mem_r_addr;
  end
  assign mem_r_data = mem[_0_];
endmodule

The Yosys Verilog backend seems to make transparent read ports rely on the presence of a clock input regardless of them being synchronous or not.
https://github.com/YosysHQ/yosys/blob/266511b29eb66486bd17210eb28454a2efee218a/backends/verilog/verilog_backend.cc#L1104-L1109

Removing the if self.synchronous else Const(0) in the mem.py snippet above seems to work, but it makes asynchronous read ports rely on a clock signal.

This behaviour can currently be encountered when instantiating a SyncFIFO with fwft=True.

I'm interested in how the If statement actually works. The m module seems to track the current "state" in some way?

        with m.If(self.a):
            m.d.comb += self.o.eq(self.sub.o)
        with m.If(self.b):
            m.d.comb += self.o.eq(self.add.o)
        with m.Else():
            m.d.comb += self.o.eq(self.rnd.o)
        return m.lower(platform)

How does it actually know that the m.Else() applies to the first m.If(?

When the YOSYS environment variable isn't defined, the fallback yosys is used as the name of the yosys executable (src). When this fails (i.e. the environment variable is not defined and yosys is not in the user's PATH), a file not found error is thrown.

Suggestion: add a more user-friendly error message that prompts the user to define/check the YOSYS environment variable or add yosys to their PATH

Recent changes mean that the clk and reset signals are not exported to the inputs of the modules
in examples

$ python ctr.py generate ctr.v

module top(o);
  wire [16:0] \$1 ;
  wire [16:0] \$2 ;
  (* src = "ctr.py:8" *)
  reg \$next\o ;
  (* src = "ctr.py:7" *)
  reg [15:0] \$next\v ;
  (* src = "/usr/local/lib/python3.7/dist-packages/nmigen-0.1-py3.7.egg/nmigen/hdl/ir.py:330" *)
  wire clk;
  (* src = "ctr.py:8" *)
  output o;
  (* src = "/usr/local/lib/python3.7/dist-packages/nmigen-0.1-py3.7.egg/nmigen/hdl/ir.py:330" *)
  wire rst;
  (* init = 16'hffff *)
  (* src = "ctr.py:7" *)

The clock and reset are wires rather than inputs to the module.

Migen left this undefined so we have wide latitude here. IMO the following should happen:

• In simulation, a hard crash, with a nice message. (This already happens, but without the nice message.)
• In synthesis, xs, to let the synthesizer discard useless logic. Migen's output causes waste in inferred multiplexers, and Migen's output already causes way too many multiplexers to be inferred.

Repro:

#!/usr/bin/env python3

from nmigen import *
from nmigen.cli import *

class Problem:
	def __init__(self):
		self.a = Signal()

	def elaborate(self, platform):
		m = Module()
		m.d.comb += self.a.eq(Signal()[:0].bool())
		return m

if __name__ == "__main__":
	p = Problem()
	main(p, ports=[p.a])

$ ./width.py generate > width.v
$ yosys -p "read_verilog width.v"
1. Executing Verilog-2005 frontend.
Parsing Verilog input from `width.v' to AST representation.
width.v:13: ERROR: syntax error, unexpected '}'

The generated file contains an empty concat list. On the other hand, this seems to work fine:

	def elaborate(self, platform):
		m = Module()
		m.d.comb += self.a.eq(Signal(0).bool())
		return m

This is with nMigen master and a very recent Yosys.

Designs doing this kind of slice simulate fine, and Signal(0).bool() == Signal(n)[:0].bool() == 0 which makes sense to me. (Is this written down anywhere? Verilog does an awful job of 0-width signals, but they're a useful generalisation, take a lot of edge cases out of your code)

I think there may be a typo on line 47 of Tristate.elaborate().

The import statement at the top is:

from .module import Module as CompatModule

I think that Module should be CompatModule. But, making that change and running locally results in the following error.

File nmigen/compat/fhdl/module.py", line 133, in __getattr__
    .format(type(self).__name__, name))
AttributeError: 'CompatModule' object has no attribute 'd'

But, that error is causing me to second guess this change. Any ideas on what's going on here?

The statement m.comb += ResetSignal().eq(0) can be used in principle to tie the reset signal of a clock domain low (and remove the rst signal from the top-level port list). However, as-is a backtrace similar to the following results:

William@William-THINK MINGW64 ~/src/migen_mercury/baseboard
$ python3 baseboard.py
Traceback (most recent call last):
  File "baseboard.py", line 373, in <module>
    m.comb += ResetSignal().eq(0)
  File "C:/msys64/home/william/src/nmigen\nmigen\tools.py", line 52, in wrapper
    return f(*args, **kwargs)
  File "C:/msys64/home/william/src/nmigen\nmigen\compat\fhdl\module.py", line 32, in __iadd__
    self._cm._module._add_statement(assigns, domain=None, depth=0, compat_mode=True)
  File "C:/msys64/home/william/src/nmigen\nmigen\hdl\dsl.py", line 248, in _add_statement
    for signal in assign._lhs_signals():
  File "C:/msys64/home/william/src/nmigen\nmigen\hdl\ast.py", line 813, in _lhs_signals
    return self.lhs._lhs_signals()
  File "C:/msys64/home/william/src/nmigen\nmigen\hdl\ast.py", line 194, in _lhs_signals
    raise TypeError("Value {!r} cannot be used in assignments".format(self))
TypeError: Value (rst sync) cannot be used in assignments

Workaround for the time being is to manually declare the clock domain with reset_less=True.

Leaving a Signal unassigned does not make it assume its reset value when used as the RHS of an assignment.

Consider the following snippet:

from nmigen import *
from nmigen.back import rtlil


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

        foo = Signal(reset=1)
        bar = Signal()
        m.d.comb += bar.eq(foo)

        return m


foo = Foo()
frag = foo.elaborate(platform=None)
print(rtlil.convert(frag))

which produces the following IR:

attribute \generator "nMigen"
attribute \top 1
module \top
  attribute \src "foo.py:9"
  wire width 1 input 0 \foo
  attribute \src "foo.py:10"
  wire width 1 \bar
  attribute \src "foo.py:10"
  wire width 1 $next\bar
  process $group_0
    assign $next\bar 1'0
    assign $next\bar \foo
    sync init
    sync always
      update \bar $next\bar
  end
end

Should \foo be set to its reset value instead of being propagated as an input ?

Shouldn't this line read:

raise LegalizeValue(value.offset, range(1 << len(value.offset)))

Currently this:

from nmigen import *
from nmigen.cli import main

m = Module()

p = Signal(4, reset=0b11)
a = Signal(1, reset = 1)

m.d.sync += p.part(a, 1).eq(0)

if __name__ == "__main__":
    main(m)

just generates one assignment:

/* Generated by Yosys 0.8+152 (git sha1 2a8e5bf9, gcc 8.2.0-r6 -fPIC -O3) */

(* top =  1  *)
(* generator = "nMigen" *)
module top(rst, clk, a);
  (* src = "cdc.py:6" *)
  reg [3:0] \$next\p ;
  (* src = "cdc.py:7" *)
  input a;
  (* src = "/home/robin/.local/lib/python3.7/site-packages/nmigen/hdl/ir.py:304" *)
  input clk;
  (* init = 4'h3 *)
  (* src = "cdc.py:6" *)
  reg [3:0] p = 4'h3;
  (* src = "/home/robin/.local/lib/python3.7/site-packages/nmigen/hdl/ir.py:304" *)
  input rst;
  always @(posedge clk)
      p <= \$next\p ;
  always @* begin
    \$next\p  = p;
    casez (a)
      1'hz:
          \$next\p [0] = 1'h0;
    endcase
    casez (rst)
      1'h1:
          \$next\p  = 4'h3;
    endcase
  end
endmodule

which does not seem equivalent to the described logic.

Hi, trying to learn nMigen here :)

I have run into a case where .. maybe it's as simple as I have some logic that is not used?

I get an error message like:

python oops.py generate -t v oops.v

Traceback (most recent call last):
File "oops.py", line 34, in main(tw, ports=[])
File "/opt/conda/lib/python3.6/site-packages/nmigen/cli.py", line 74, in main
main_runner(parser, parser.parse_args(), args, kwargs)
File "/opt/conda/lib/python3.6/site-packages/nmigen/cli.py", line 56, in main_runner output = verilog.convert(fragment, name=name, ports=ports)
File "/opt/conda/lib/python3.6/site-packages/nmigen/back/verilog.py", line 37, in convert
raise YosysError(error.strip())
nmigen.back.verilog.YosysError: Warning: Module engine contains unmapped RTLIL proccesses. RTLIL processes
can't always be mapped directly to Verilog always blocks. Unintended
changes in simulation behavior are possible! Use "proc" to convert
processes to logic networks and registers.Warning: Module top contains unmapped RTLIL proccesses. RTLIL process
es
can't always be mapped directly to Verilog always blocks. Unintended
changes in simulation behavior are possible! Use "proc" to convert
processes to logic networks and registers.ERROR: Assertion failed: selection is not empty: w: i: %a %d o: %a
%ci* %d c:* %co* %a %d n:$* %d

It's not immediately apparent to a user (especially if they change more than a few lines of code at a time) which one is unused or causing this to break..

(I partially manually reduced my example code for this example)
(I have a more complex example that I think I've wired up correctly, but for some reason I need to route some signals out to pins or else it gives this same error ! (kind of like having load bearing nops) I add another signal somewhere from some verilog code, and suddenly this breaks .. sometimes even when I do wire it up to a pin -- hopefully improving the error message here will help! * Ideally let me know which signal, show why it's unmapped? )

There are a lot of possible combinations of memory ports and many of them are not legal. This depends on the vendor. In this issue we collect such behaviors to eventually implement a sanitizer, either as a part of nMigen or as a Yosys pass.

When creating a Record, the signals inside it are named recordname_fieldname currently.

This makes it hard to see the hierarchy when using snake case names for records and fields as well. I.e. having a record named some_record with a field named some_field the resulting signal is called some_record_some_field which could also be the name of a signal in a nested record with depth 3.

I propose to use another character for separation (for example .), which would lead to some_record.some_field in that case.

(see: https://github.com/m-labs/nmigen/blob/master/nmigen/hdl/rec.py#L75)

This is currently not done because of a quirk (bug?) in Yosys: YosysHQ/yosys#760. This is not right and affects user designs as the en signal is a constant and cannot be assigned to:

For added fun, they cannot be used on CompatModules either.

10e6 is incorrect since 10e6 is 10.0*10^6 not 10^6.

Sometimes I want to use generators to build a simulation model of some module. In migen, this wasn't possible for modules that implement combinatorial logic.

Trying my hand at nmigen syntax, an example could be something like this:

class RoutingPolicy:
    def __init__(self, width, n):
        self.data = Signal(width)
        self.destination = Signal(max=n)

    # I haven't decided yet what I want this module to do, let's try some options
    def gen_destination(self):
        while True: # or something?
            data = (yield self.data)
            if (data == 23):
                yield self.destination.eq(0)
            else:
                yield self.destination.eq(self.data[:log2_int(n)])

class Router:
    def __init__(self, width, n):
        self.in_port = Signal(width)
        self.out_ports = Array(Signal(width) for _ in range(n))

        self.policy = RoutingPolicy(width, n)

    def get_fragment(self, platform):
        m = Module()
        m.submodules.policy = self.policy
        m.d.comb += [
            self.policy.data.eq(self.in_port),
            self.out_ports[self.policy.destination].eq(self.in_port)
        ]
        return m.lower(platform)

m = Router(8, 2)
run_simulation(m, m.policy.gen_destination())

There probably needs to be something to tell what constitutes one iteration of the generator, with while True it would just run indefinitely. while (yield input_values_changed()): ? Also the whole thing would need to be @passive or so since there's no predefined end point.
Another question is whether it's possible, or even makes sense, to mix combinatorial assignments and synchronous assignments in one generator.

When adding a submodule to a submodule, the hirarchy is not obvious from the generated verilog.

Given the first submodule is named parent and its child child, I would suggest to name the verilog submodule for the child parent__child instead of child only.

What do you think?

nMigen has a pretty strong dependence on Yosys and you have been fixing a lot of bugs in it as #53 shows.

Maybe nMigen should check the Yosys version is new enough?

Python standard practice is to use isinstance(v, C) instead of type(v) is C. This allows using user-defined classes that derive from nmigen types.
One place that needs to be fixed is

See
http://bugs.libre-riscv.org/show_bug.cgi?id=64#c10
http://bugs.libre-riscv.org/show_bug.cgi?id=66
https://stackoverflow.com/a/1549854

I noticed in my design that two memories, both defined the same way and in the same clock domain, seemed to have different write behaviours:

This first one is what I'd expect: one cycle after addr=2 and data=2, mem(2) gets the value 2:

This is the weird one: last cycle's data is written to this cycle's slot:

I made a small reproduction of the buggy case:

from nmigen import Module, Memory, Signal
from nmigen.back import pysim

def test_mem():
    m = Module()
    mem = Memory(8, 32)
    m.submodules.wrport = wrport = mem.write_port()
    ctr = Signal(4)
    m.d.sync += [
        wrport.data.eq(ctr + 0x30),
        wrport.addr.eq(ctr),
        wrport.en.eq(1),
        ctr.eq(ctr + 1),
    ]

    vcdf = open("membug.vcd", "w")
    with pysim.Simulator(m, vcd_file=vcdf) as sim:
        def testbench():
            for _ in range(5):
                yield
            for idx in range(5):
                print(idx, ":", hex((yield mem[idx])))
        sim.add_clock(1e-6)
        sim.add_sync_process(testbench())
        sim.run()

if __name__ == "__main__":
    test_mem()

Moving the assignments to wrport.addr and wrport.data into a comb block restores the expected behaviour, so I guess it's to do with the addr/data being driven from a sync block?

The following MWE generates invalid RTLIL and as such fails Verilog generation.

from nmigen import *
from nmigen.cli import main

class Test:
    def __init__(self):
        self.out = Signal(8)

    def get_fragment(self, platform):
        data = [0b100, 0b1000, 0b10000]
        arr = Array(data)
        idx = Signal(max=len(data))

        m = Module()
        m.d.comb += self.out.eq(arr[idx])
        m.d.sync += idx.eq(idx + 1)
        return m.lower(platform)

test = Test()
main(test, ports=[test.out])

The following error is produced:

D:\test> python test.py generate -t v test.v
Traceback (most recent call last):
  File "test.py", line 19, in <module>
    main(test, ports=[test.out])
  File "D:\Program Files\Python36\lib\site-packages\nmigen-0.1-py3.6.egg\nmigen\cli.py", line 74, in main
    main_runner(parser, parser.parse_args(), *args, **kwargs)
  File "D:\Program Files\Python36\lib\site-packages\nmigen-0.1-py3.6.egg\nmigen\cli.py", line 56, in main_runner
    output = verilog.convert(fragment, name=name, ports=ports)
  File "D:\Program Files\Python36\lib\site-packages\nmigen-0.1-py3.6.egg\nmigen\back\verilog.py", line 37, in convert
    raise YosysError(error.strip())
nmigen.back.verilog.YosysError: ERROR: Found error in internal cell \top.$5 ($pos) at kernel/rtlil.cc:704:
  attribute \src "test.py:14"
  cell $pos $5
    parameter \Y_WIDTH 8
    parameter \A_WIDTH 5
    parameter \A_SIGNED 0
    connect \Y $4
    connect \A 4'1000
  end

This error does not occur when the elements of the Array all have the same shape (i.e. [0b100, 0b101, 0b111, 0b110]).

In oMigen we had four kinds of things that in nMigen go to extras:

• Drive(x)
• IOStandard(x)
• Misc("K=V")
• Misc("K")

All of these with the exception of the last are essentially key-value pairs. The last one appears to be almost exclusively used as Misc("PULLUP") on Xilinx platforms, and it could as well be replaced with PULLUP=TRUE (this is done for Sayma).

To reduce the amount of ad-hoc mini-languages, let's just use a dict instead.

An Array is essentially a mux, and there are several equally valid ways to drive a mux. Currently only straight binary is supported. It would be nice to also have one-hot and priority encodings, since all of these are trivially representable directly in RTLIL and Verilog.

One thing to note is that one-hot and priority are one and the same unless parallel_case is used.

I am not sure if I am using DomainRenamer or ClockSignal wrong but I can't get them to work together.

from nmigen import *
from nmigen.cli import main
import traceback

def test1():
    m = Module()
    m.d.comb += ClockSignal().eq(0)
    m1 = DomainRenamer("other")(m)
    main(m1)

def test2():
    m = Module()
    m.d.comb += ClockSignal().eq(0)

    m = DomainRenamer("other")(m)

    # adding only one of the ClockDomains also fails 
    m.domains += [ClockDomain("sync"), ClockDomain("other")]
    main(m)

def test3():
    m = Module()
    m.d.comb += ClockSignal().eq(0)

    # this testcase doesn't fail if these two lines are omitted
    a = Signal()
    m.d.sync += a.eq(0)

    m_super = Module()
    m_super.domains += [ClockDomain("sync"), ClockDomain("other")]
    m_super.submodules.m = DomainRenamer("other")(m)

    main(m_super)

for t in [test1, test2, test3]:
    try:
        t()
    except Exception as e:
        print(t.__name__ + ":")
        traceback.print_exc()
        print()

prints this

Could we get an example of embedding existing Verilog code? With specials having gone, it's unclear to me how that works?

When generating the platform, it creates signals with double underscores and ignores the resource

zignig@noid:/opt/FPGA/nmigen/examples$ python blinky.py
Warning: net 'clk3p3_0_io' does not exist in design, ignoring clock constraint

in build/top.il

  attribute \src "/usr/local/lib/python3.6/dist-packages/nmigen/build/res.py:137"
  wire width 1 input 0 \clk3p3_0_io

and then

  wire width 1 \clk3p3_0__i
  attribute \src "/usr/local/lib/python3.6/dist-packages/nmigen/hdl/rec.py:98"
  wire width 1 $next\clk3p3_0__i
  process $group_0
    assign $next\clk 1'0
    assign $next\clk \clk3p3_0__i
    sync init
    sync always
      update \clk $next\clk
  end

I'm using nmigen d69a4e2 (master as of march 22 2019)

Test Case:

from nmigen import Module, Memory
from nmigen.back.pysim import Simulator, Delay, Tick
import unittest


class TestMemory(unittest.TestCase):
    def test(self) -> None:
        class TestModule:
            def __init__(self):
                self.mem = Memory(1, 2, name="mem", init=[0, 0])
                self.mem_rd0 = self.mem.read_port(synchronous=False)
                self.mem_rd1 = self.mem.read_port(synchronous=False)
                self.mem_wr0 = self.mem.write_port(priority=0)
                self.mem_wr1 = self.mem.write_port(priority=1)

            def elaborate(self, platform):
                m = Module()
                m.submodules.mem_rd0 = self.mem_rd0
                m.submodules.mem_rd1 = self.mem_rd1
                m.submodules.mem_wr0 = self.mem_wr0
                m.submodules.mem_wr1 = self.mem_wr1
                m.d.comb += self.mem_rd0.addr.eq(0)
                m.d.comb += self.mem_rd1.addr.eq(1)
                m.d.comb += self.mem_wr0.addr.eq(0)
                m.d.comb += self.mem_wr0.data.eq(1)
                m.d.comb += self.mem_wr0.en.eq(1)
                m.d.comb += self.mem_wr1.addr.eq(1)
                m.d.comb += self.mem_wr1.data.eq(1)
                m.d.comb += self.mem_wr1.en.eq(1)
                return m
        module = TestModule()
        with Simulator(module,
                       vcd_file=open("test.vcd", "w"),
                       gtkw_file=open("test.gtkw", "w"),
                       traces=[module.mem_rd0.data,
                               module.mem_rd1.data]) as sim:
            sim.add_clock(1e-6, 0.25e-6)

            def async_process():
                yield Delay(1e-7)
                self.assertEqual((yield module.mem_rd0.data), 0)
                self.assertEqual((yield module.mem_rd1.data), 0)
                yield Tick()
                yield Delay(1e-7)
                self.assertEqual((yield module.mem_rd0.data), 1)
                self.assertEqual((yield module.mem_rd1.data), 1)

            sim.add_process(async_process)
            sim.run()

I was browsing the code to learn the standard library, and bumped into this in cdc.py:

class MultiReg:
    def __init__(self, i, o, odomain="sync", n=2, reset=0):

     ...

        self._regs = [Signal(self.i.shape(), name="cdc{}".format(i),
                             reset=reset, reset_less=True, attrs={"no_retiming": True})
                      for i in range(n)]

I understand there are caveats with resets on CDC capture flops, but sometimes you need it. For example, a pulse-to-pulse synchroniser (or toggle synchroniser) contains a toggling NRZ signal crossing the domains, and the capture flops need a capture-domain reset to avoid spurious toggling observed by the logic when deasserting reset.

platform.get_multi_reg(self) may contain considerable magic for some synthesis flows, and should only need writing once; ideally, something like a pulse-to-pulse synchroniser should be implementable using the existing MultiReg.

A ligh-touch option could be to just add a reset_less=True to MultiReg.__init__, and pass this down.

Happy to be argued with, told to go implement it, or just told I'm wrong :)

Currently the maximum value of a Signal specified using the max parameter is rounded up to the next power of two.
Because of this something like the following fails with IndexError: Cannot end slice 10 bits into 9-bit value

class Test:
    def __init__(self):
        self.ctr = Signal(max=9)
        self.out = Signal(9)
        self.input = Signal()

    def elaborate(self, platform):
        m = Module()
        m.d.sync += self.out.part(self.ctr, 1).eq(self.input)

        with m.If(self.ctr == 9):
            m.d.sync += self.ctr.eq(0)
        with m.Else():
            m.d.sync += self.ctr.eq(self.ctr + 1)
        
        return m

While I understand the cause for this error, I don't think this is the best solution possible.
I can think of two possible ways to improve:

1. Adding a new kind of Signal that supports non power of two maximum values (and guarantees that it is not excceded). Guaranteeing this may come with a high cost, but would also simplify some code, like this counter.
2. Adding a new parameter to the Signal constructor which lets the user assure the specified maximum value is never exceded. Something like this could also be added to the new kind of Signal proposed.

Repro:

from nmigen import *
from nmigen.back.pysim import *


m = Module()
m.domains += ClockDomain("input")
m.domains += ClockDomain("output")

with Simulator(m, vcd_file=open("test.vcd", "w")) as sim:
    sim.add_clock(3e-6, domain="input")
    sim.add_clock(5e-6, domain="output")
    sim.run_until(1, run_passive=True)