Prototype a new CLI

[whitequark]

Demo:

$ cat pyproject.toml
[project]
name = "efbutils"
version = "0"

requires-python = "~=3.8"
dependencies = [
  "amaranth @ file:///${PROJECT_ROOT}/../amaranth"
]

[build-system]
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.build]
includes = ["efbutils/"]
$ cat Makefile
-include page_buffer.v.d
page_buffer.v:
        pdm run amaranth generate efbutils.ufm.reader.page_buffer:PageBuffer verilog -v $@ -d [email protected]
$ make page_buffer.v
pdm run amaranth generate efbutils.ufm.reader.page_buffer:PageBuffer verilog -v page_buffer.v -d page_buffer.v.d
$ make page_buffer.v
make: 'page_buffer.v' is up to date.
$ touch efbutils/__init__.py
$ make page_buffer.v
pdm run amaranth generate efbutils.ufm.reader.page_buffer:PageBuffer verilog -v page_buffer.v -d page_buffer.v.d

[codecov]

Codecov Report

Merging #904 (1fd9388) into main (d32ca04) will decrease coverage by 0.02%.
Report is 15 commits behind head on main.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main     #904      +/-   ##
==========================================
- Coverage   83.75%   83.74%   -0.02%     
==========================================
  Files          54       54              
  Lines        7678     7684       +6     
  Branches     1867     1869       +2     
==========================================
+ Hits         6431     6435       +4     
- Misses       1048     1049       +1     
- Partials      199      200       +1     

Files	Coverage Δ
amaranth/back/rtlil.py	77.30% <100.00%> (-0.04%)	⬇️
amaranth/lib/fifo.py	93.71% <87.50%> (-0.37%)	⬇️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[cr1901]

I think the verilog.convert invocation should include name field (either as an input arg to main or maybe derive it from the class name? I prefer the former slightly to avoid accidental conflicts with primitive names in the Verilog synthesizer). In projects where you generate multiple Verilog modules from Amaranth, chances are the synthesizer is not going to like them all named top.

[whitequark]

The problem is that you'll still get duplicate names for the submodules in that case, so it's not enough.

[cr1901]

I agree, per IRC, that this is a problem that needs to be solved. For the purposes of testing/finishing the refactor of my code for the time being, would you be willing to expose top-level module renaming from the CLI? It would allow me to write the FuseSoC generation in terms of the Amaranth CLI.

[whitequark]

Try the updated version.

[cr1901]

This correctly creates the hiearchy:

pdm run amaranth generate efbutils.ufm.reader.demo:Demo -p num_leds 2         -p efb_config '{ "dev_density": "7000L", "wb_clk_freq": 12.08 }' -p ufm_config '{ "init_mem": null, "zero_mem": true, "start_page": 0, "num_pages": 2046}' verilog -v - | grep "module "
module top(rx, tx, leds);
module \top.efb (bus__adr, bus__cyc, bus__dat_r, bus__dat_w, bus__irq, bus__stb, bus__we, rst, clk, bus__ack);
module \top.por (clk, por_clk, rst);
module \top.reader (bus__data, bus__read_en, bus__valid, efb__ack, efb__adr, efb__cyc, efb__dat_r, efb__dat_w, efb__irq, efb__stb, efb__we, rst, clk, bus__addr);
module \top.reader.pagemod (clk, seq__data, seq__addr, seq__stb, seq__ack, rand__data, rand__addr, rand__read_en, rand__valid, rst);
module \top.reader.streammod (clk, efb__ack, efb__adr, efb__cyc, efb__dat_r, efb__dat_w, efb__irq, efb__stb, efb__we, stream__data, stream__addr, stream__stb, stream__ack, stream__ready, rst);
module \top.reader.streammod.seqmod (clk, ctl__cmd, ctl__data_len, ctl__done, ctl__op_len, ctl__rd__data, ctl__rd__stb, ctl__req, ctl__wr__data, ctl__xfer_is_wr, efb__ack, efb__adr, efb__cyc, efb__dat_r, efb__dat_w, efb__stb, efb__we, rst);
module \top.uart (i, data, ack, rdy, rst, clk, o);
module \top.uart.rx (rst, clk, divisor, i);
module \top.uart.tx (data, ack, rdy, rst, clk, divisor, o);
module \top.wait (rst, clk, stb);

Once I'm able to rename the top prefix (probably via an -n [NAME] argument to the CLI?), I can finish the refactor, and wait for other ppl to test.

[whitequark]

Try the updated version.

[cr1901]

It took me a while to realize that by "object reference", the error means "your input argument fails to match a regex", rather than "Python couldn't find an object with that name" (which is what the ImportError exception handler is for).

Perhaps this else branch could have "expected format is path.to.mod:Obj" or something?

[whitequark]

Yep this could be improved!!

[whitequark]

Care to submit a PR against a PR?

[cr1901]

Will do in the coming days (my response time will be sporadic until possibly this Monday afternoon).

[cr1901]

PR is done: whitequark#1

[cr1901]

I'm quite happy with this PR at this point. But I don't want it merged before other people have tested it. Any chance we could bring it up at the next meeting to let other ppl know to try it out and report issues as a "final comment period"?

[whitequark]

Any chance we could bring it up at the next meeting to let other ppl know to try it out and report issues as a "final comment period"?

It's still a draft in my thoughts--I just prototyped it for you. Merging it needs an RFC first.