Regression following 'Use "read" command' change

[shenki]

Steps to reproduce the issue

Build the microwatt soc. It is a mixed language implementation (vhdl and verilog), so you need ghdl and the ghdl plugin.

git clone http://github.com/antonblanchard/microwatt && cd microwatt
make microwatt.bit

Expected behavior

With 2156e20db5b88ac7e3530abd2f27d27e12a412b2 (yosys-0.12) the soc builds correctly.

Actual behavior

With b07ca8756a6ca4fafd725f008ac871a0e24d75e8 (latest as of today) it fails to build at the nextpnr step:

nextpnr-ecp5 --json microwatt.json --lpf constraints/orange-crab.lpf --textcfg microwatt_out.config.tmp --um5g-85k --freq 48 --package CSFBGA285
...
ERROR: cell type 'uart_top' is unsupported (instantiated as 'soc0.uart0_16550.uart0')

The json is not recongised:

        "soc0.uart0_16550.uart0": {
          "hide_name": 0,
          "type": "uart_top",
          "parameters": {
          },
          "attributes": {
            "hdlname": "soc0 uart0_16550.uart0"
          },

I did a bisect and the commit the caused the regression is 0cbdb42dcd74090296aa3fe6bf11db1c288d9962, "Use "read" command to parse HDL files from Yosys command-line". If I revert this change I can build the soc again.

I assume I need to update the build script for microwatt to account for the change, but I was unable to work out what the change requires.

The yosys command is:

yosys -m ghdl.so -p " ghdl --std=08 --no-formal -gMEMORY_SIZE=8192 -gRAM_INIT_FILE=hello_world/hello_world.hex -gRESET_LOW=true -gCLK_INPUT=48000000 -gCLK_FREQUENCY=48000000 -gICACHE_NUM_LINES=4 -gUSE_LITEDRAM=true decode_types.vhdl common.vhdl wishbone_types.vhdl fetch1.vhdl utils.vhdl plru.vhdl cache_ram.vhdl icache.vhdl decode1.vhdl helpers.vhdl insn_helpers.vhdl control.vhdl decode2.vhdl register_file.vhdl cr_file.vhdl crhelpers.vhdl ppc_fx_insns.vhdl rotator.vhdl logical.vhdl countzero.vhdl multiply.vhdl divider.vhdl execute1.vhdl loadstore1.vhdl mmu.vhdl dcache.vhdl writeback.vhdl core_debug.vhdl core.vhdl fpu.vhdl pmu.vhdl wishbone_arbiter.vhdl wishbone_bram_wrapper.vhdl sync_fifo.vhdl wishbone_debug_master.vhdl xics.vhdl syscon.vhdl gpio.vhdl soc.vhdl spi_rxtx.vhdl spi_flash_ctrl.vhdl litedram/extras/litedram-wrapper-l2.vhdl litedram/generated/orangecrab-85-0.2/litedram-initmem.vhdl fpga/soc_reset.vhdl fpga/pp_fifo.vhd fpga/pp_soc_uart.vhd fpga/main_bram.vhdl nonrandom.vhdl fpga/clk_gen_ecp5.vhd fpga/top-orangecrab0.2.vhdl dmi_dtm_dummy.vhdl -e toplevel; synth_ecp5 -nowidelut -json microwatt.json  -abc2 -abc9" uart16550/raminfr.v uart16550/uart_defines.v uart16550/uart_receiver.v uart16550/uart_regs.v uart16550/uart_rfifo.v uart16550/uart_sync_flops.v uart16550/uart_tfifo.v uart16550/uart_top.v uart16550/uart_transmitter.v uart16550/uart_wb.v valentyusb/generated/orangecrab-85-0.2/gateware/valentyusb.v litesdcard/generated/lattice/litesdcard_core.v litedram/generated/orangecrab-85-0.2/litedram_core.v

Simplified:

yosys -m ghdl.so -p " ghdl --std=08 --no-formal files.vhdl -e toplevel; synth_ecp5 -nowidelut -json microwatt.json  -abc2 -abc9" files.v

[mmicko]

Should be fixed with https://github.com/YosysHQ/yosys/commit/e1c7a9a64734d9fe81edec7d5bb9708b0ae88f26

[shenki]

Should be fixed with e1c7a9a

Thanks! I tested with latest 477eeefd9 which includes this change and I'm still seeing the issue.

[mmicko]

Sorry @shenki, complete flow was not set on my end. Managed to reproduce now. Will investigate.

[mmicko]

@shenki Maybe best way to solve this is to change line https://github.com/antonblanchard/microwatt/blob/master/Makefile#L214 to

$(YOSYS) -m $(GHDLSYNTH) -p "ghdl --std=08 --no-formal $(GHDL_IMAGE_GENERICS) $(synth_files) -e toplevel; read_verilog $(uart_files); synth_ecp5 -abc9 -nowidelut -json $@  $(SYNTH_ECP5_FLAGS)" 

This way you can do explicit read_verilog and keep old behavior. Since read is calling read_verilog with -defer parameter, and that changes how things do work.

[cr1901]

Can also confirm via bisect that 0cbdb42 broke my example MachXO2 bitstream demos:

William@DESKTOP-H0PMN4M MINGW64 ~/Projects/FPGA/nextpnr/machxo2/examples
$ ../../../yosys/yosys.exe -ql read.log -p "synth_machxo2 -json tinyfpga.json" tinyfpga.v
ERROR: Module top contains processes, which are not supported by JSON backend (run `proc` first).

Workaround

William@DESKTOP-H0PMN4M MINGW64 ~/Projects/FPGA/nextpnr/machxo2/examples
$ ../../../yosys/yosys.exe -p "read_verilog tinyfpga.v; synth_machxo2 -json tinyfpga.json"

Files

read.log

[whitequark]

Can you check if https://github.com/YosysHQ/yosys/pull/3269 fixes this?

[mmicko]

@whitequark this issue was not related to hierarchy pass, have undo Makefile change locally and run with latest oss-cad-suite build just to confirm. Issue here is that read now use read_verilog -defer and before it was just read_verilog, which creates this issue when using with GHDL. If you take latest code and add in line https://github.com/antonblanchard/microwatt/blob/master/Makefile#L236 defer option to read_verilog it will produce same issue.

[ghost]

The reason running with -defer breaks the design is that it reads modules in as abstract modules. Running hierarchy -auto-top should resolve these abstract modules to concrete ones that further passes can operate on, however due to the bug that you fixed in your patch, this was not occurring since modules not tagged with the top attribute were not identified automatically before this elaboration pass was run.

The synth_* passes already invoke the hierarchy pass with -auto-top by default. So this should not occur after the resolution of your pass. Something I tested.