pysim: testbench-only signals are not placed at root level on the VCD file

[cestrauss]

Consider:

from nmigen import Signal, Module
from nmigen.sim import Simulator

m = Module()
s = Signal()
t = Signal()
m.d.sync += s.eq(1)

def process():
    yield t.eq(1)

for engine in ["pysim", "cxxsim"]:
    sim = Simulator(m, engine=engine)
    sim.add_clock(1e-6)
    sim.add_sync_process(process)
    with sim.write_vcd(f"bug17.{engine}.vcd", traces=[s, t]):
        sim.run()

On PySim, both s and t are placed under the topmodule. On CXXSim, t is at the root.

The generated GTKWave document (if any) correctly display the trace, but an already existing manually-generated document would need modification, which is inconvenient.

[whitequark]

I've implemented this behavior intentionally for CXXSim; the idea is that this way, it's easier to figure out which signals come from the testbench, and which come from the netlist. I actually thought that PySim already worked that way, but it seems like I misremembered.

[cestrauss]

Sure, makes sense. I've just checked nMigen 0.2, and its behavior does matches CXXSim, differently from the current PySim. So, it actually changed some time along the way. No harm done in changing it back, then.

[Lunaphied]

I started working on the code required to implement this functionality (testbench signals on root level), and I realized that this might not be the most desirable behavior.

The way GTKWave handles root level signals is somewhat annoying; for one it's somewhat confusing to even recognize that there are root level signals in the first place. Selecting them if you click another module requires clicking that module again which is counterintuitive. Furthermore, you can only recursively import signals from a named hierarchy level. This means if you have a number of top-level testbench signals, you cannot simply recursively import all signals in the design (which is useful for smaller designs).

Given these patterns, I suggest we instead create our own "root" level module but give it a name such as "testbench", this has a much less obscure UX pattern. I have the code to do this already implemented, but I thought it would be worth discussing the behavior change first.