amaranth-lang / amaranth

A modern hardware definition language and toolchain based on Python
https://amaranth-lang.org/docs/amaranth/
BSD 2-Clause "Simplified" License
1.57k stars 174 forks source link

Undriven signal ends up as module port #630

Closed antonblanchard closed 2 years ago

antonblanchard commented 3 years ago

If a signal has no driver, it ends up as a port:

from nmigen import *
from nmigen.back import verilog

class Test(Elaboratable):
    def __init__(self):
        self.val_out = Signal()

    def elaborate(self, platform):
        m = Module()
        int_sig = Signal(8)
        m.d.comb += self.val_out.eq(int_sig)

        return m

if __name__ == "__main__":
    top = Test()
    with open("test.v", "w") as f:
        f.write(verilog.convert(top))
module top(int_sig);
  input [7:0] int_sig;
  wire val_out;
  assign val_out = int_sig[0];
endmodule

This is pretty confusing. Reporting an error here would be great.

BracketMaster commented 3 years ago

You're really supposed to provide an explicit port list iirc.

from nmigen import *
from nmigen.back import verilog

class Test(Elaboratable):
    def __init__(self):
        self.val_out = Signal()

    def elaborate(self, platform):
        m = Module()
        int_sig = Signal(8)
        m.d.comb += self.val_out.eq(int_sig)

        return m

if __name__ == "__main__":
    top = Test()
    with open("test.v", "w") as f:
        f.write(verilog.convert(top, ports=[top.val_out]))
/* Generated by Yosys 0.9+3755 (git sha1 442d19f6, clang 12.0.0 -fPIC -Os) */

(* \nmigen.hierarchy  = "top" *)
(* top =  1  *)
(* generator = "nMigen" *)
module top(val_out);
  wire [7:0] int_sig;
  output val_out;
  assign int_sig = 8'h00;
  assign val_out = 1'h0;
endmodule
antonblanchard commented 3 years ago

Thanks @BracketMaster, that does fix the issue at the top level.

I think there are a couple of usability issues here:

whitequark commented 2 years ago

Let's deprecate optional ports= in 0.3 and remove it in 0.4.