byuccl / spydrnet

A flexible framework for analyzing and transforming FPGA netlists. Official repository.
https://byuccl.github.io/spydrnet
BSD 3-Clause "New" or "Revised" License
86 stars 20 forks source link

Verilog Composer Writing Module Header Port Concatenations #184

Closed jacobdbrown4 closed 1 year ago

jacobdbrown4 commented 1 year ago

I'm working on the VexRiscv design for the Lattice CrossLink NX Eval board. I've been running into errors in PAR that arise after running the netlist through SpyDrNet. I've tracked the issue down to the following:

When composing a module's top level ports, the Verilog composer will do something interesting. It will write out concatenations like the following:

module VexRiscv
(
    un1_main_basesoc_uart_tx_fifo_level0_0_d0,
    dsp_join_kb_0,
    builder_csr_bankarray_interface3_bank_bus_dat_r_11,
    main_basesoc_timer_value_status,
    builder_csr_bankarray_interface2_bank_bus_dat_r_11,
    storage_1_dat1,
    main_basesoc_uart_enable_storage,
    N_443_0,
    .builder_csr_bankarray_dat_r({builder_csr_bankarray_dat_r[1], builder_csr_bankarray_dat_r[2], builder_csr_bankarray_dat_r[3], builder_csr_bankarray_dat_r[4], builder_csr_bankarray_dat_r[5], builder_csr_bankarray_dat_r[6], builder_csr_bankarray_dat_r[7]}),
    builder_slave_sel_r_r_0_a2_0,
    dsp_join_kb,
    .main_basesoc_timer_value_7({main_basesoc_timer_value_7[1], main_basesoc_timer_value_7[2], main_basesoc_timer_value_7[3], main_basesoc_timer_value_7[4], main_basesoc_timer_value_7[5], main_basesoc_timer_value_7[6], main_basesoc_timer_value_7[7], main_basesoc_timer_value_7[8], main_basesoc_timer_value_7[9], main_basesoc_timer_value_7[10], main_basesoc_timer_value_7[11], main_basesoc_timer_value_7[12], main_basesoc_timer_value_7[13], main_basesoc_timer_value_7[14], main_basesoc_timer_value_7[15], main_basesoc_timer_value_7[16], main_basesoc_timer_value_7[17], main_basesoc_timer_value_7[18], main_basesoc_timer_value_7[19], main_basesoc_timer_value_7[20], main_basesoc_timer_value_7[21], main_basesoc_timer_value_7[22], main_basesoc_timer_value_7[23], main_basesoc_timer_value_7[24], main_basesoc_timer_value_7[25], main_basesoc_timer_value_7[26], main_basesoc_timer_value_7[27], main_basesoc_timer_value_7[28], main_basesoc_timer_value_7[29], main_basesoc_timer_value_7[30], main_basesoc_timer_value_7[31]}),
    builder_count_r_3,
    builder_count_r_0,
    builder_slave_sel_2_0,
    builder_basesoc_adr,
    N_792_0,
    main_basesoc_timer_reload_storage,
    builder_slave_sel_r,
    un1_main_basesoc_serial_tx_rs232phytx_next_value112_i_0,
    main_basesoc_rx_data,
    main_basesoc_rx_source_payload_data,
    rom_dat0,
    builder_array_muxed0,
    .main_basesoc_rx_count({main_basesoc_rx_count[1], main_basesoc_rx_count[2], main_basesoc_rx_count[3]}),
    .main_basesoc_tx_count({main_basesoc_tx_count[1], main_basesoc_tx_count[2], main_basesoc_tx_count[3]}),
    main_dataout0,
    ....

The ports of interest are:

    .builder_csr_bankarray_dat_r({builder_csr_bankarray_dat_r[1], builder_csr_bankarray_dat_r[2], builder_csr_bankarray_dat_r[3], builder_csr_bankarray_dat_r[4], builder_csr_bankarray_dat_r[5], builder_csr_bankarray_dat_r[6], builder_csr_bankarray_dat_r[7]}),
    .main_basesoc_timer_value_7({main_basesoc_timer_value_7[1], main_basesoc_timer_value_7[2], main_basesoc_timer_value_7[3], main_basesoc_timer_value_7[4], main_basesoc_timer_value_7[5], main_basesoc_timer_value_7[6], main_basesoc_timer_value_7[7], main_basesoc_timer_value_7[8], main_basesoc_timer_value_7[9], main_basesoc_timer_value_7[10], main_basesoc_timer_value_7[11], main_basesoc_timer_value_7[12], main_basesoc_timer_value_7[13], main_basesoc_timer_value_7[14], main_basesoc_timer_value_7[15], main_basesoc_timer_value_7[16], main_basesoc_timer_value_7[17], main_basesoc_timer_value_7[18], main_basesoc_timer_value_7[19], main_basesoc_timer_value_7[20], main_basesoc_timer_value_7[21], main_basesoc_timer_value_7[22], main_basesoc_timer_value_7[23], main_basesoc_timer_value_7[24], main_basesoc_timer_value_7[25], main_basesoc_timer_value_7[26], main_basesoc_timer_value_7[27], main_basesoc_timer_value_7[28], main_basesoc_timer_value_7[29], main_basesoc_timer_value_7[30], main_basesoc_timer_value_7[31]}),
    .main_basesoc_rx_count({main_basesoc_rx_count[1], main_basesoc_rx_count[2], main_basesoc_rx_count[3]}),
    .main_basesoc_tx_count({main_basesoc_tx_count[1], main_basesoc_tx_count[2], main_basesoc_tx_count[3]}),

I don't know why this is. The Verilog composer will also do this in instantiations which makes senses because mapped ports may be more than one bit wide and may come from different wire names. But doing it in a module definition's top level ports doesn't make sense to me. Does this seem like a bug? Or is it valid Verilog synthax that Lattice Radiant is just not liking?

I removed this part of the composer and made it only write out the port name in the module header ports and the issue was resolved. See here for the original function that I edited. I removed the part that decided if it was aliased or not and just wrote out the port name.

After doing this, Lattice Radiant was able to generate a bitstream from the netlist run through spydrnet without errors and the design worked great on the board.

ganeshgore commented 1 year ago

Hey @jacobdbrown4, If I remember correctly, the concatination syntax appears in the module definition if you are missing the corresponding (same name) cable in the definition or down to property on the cable, and the port has a mismatch.

jacobdbrown4 commented 1 year ago

Thanks for your response, Ganesh. As I've worked on this and learned more, it seems SpyDrNet is handling this correctly.