zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
498 stars 50 forks source link

inline attributes in instance port list fail to parse #192

Closed emfredrickson closed 1 year ago

emfredrickson commented 2 years ago

sv2v fails to parse inline attributes inside an instance's port list. An example file that that occurs in is cdc_fifo_gray.sv. In the linked file, this occurs in the instance quoted below.

  cdc_fifo_gray_src #(
    .T         ( T         ),
    .LOG_DEPTH ( LOG_DEPTH )
  ) i_src (
    .src_rst_ni,
    .src_clk_i,
    .src_data_i,
    .src_valid_i,
    .src_ready_o,

    (* async *) .async_data_o ( async_data ),
    (* async *) .async_wptr_o ( async_wptr ),
    (* async *) .async_rptr_i ( async_rptr )
  );
zachjs commented 2 years ago

sv2v doesn't yet support attributes on port connections, but this could be added. It wouldn't necessarily be difficult, but it would be a larger change because I did not anticipate this feature. Would it be acceptable for your use case for sv2v to simply ignore those attributes? I'm not aware of Yosys in particular having any special logic for an async attribute.

zachjs commented 2 years ago

@emfredrickson Do you have any update here? It's definitely possible to add support here, but I'd like to be clear on the use case before I get started. Thanks!

KatCe commented 1 year ago

Hello. We wanted to run sv2v on the CVA6 core and came across the same issue. Commands to reproduce are:

git clone https://github.com/openhwgroup/cva6.git
cd cva6
git submodule update --init --recursive
bender sources -f -t synthesis > bender.sources
sed -i "s/synopsys_sram/util\/sram/g" bender.sources
morty -f bender.sources -DVERILATOR -DSYNTHESIS > pickled.sv
sv2v pickled.sv

Our goal is to ignore the attributes. Would appreciate if sv2v removes them instead of failing with Parse error: unexpected token '(*' (Sym_paren_l_aster). Thank you!

zachjs commented 1 year ago

Would appreciate if sv2v removes them instead of failing with Parse error: unexpected token '(*' (Sym_paren_l_aster).

I've just pushed this change. Can you please try it out?

KatCe commented 1 year ago

Thank you @zachjs for the quick fix! The attribute parsing works fine.

There are other errors now when parsing cva6 with the above mentioned commands. The assertions at the end of module axi_id_remap lead to error pickled.sv:26842:3: Parse error: missing expected 'endmodule'. . I've tried to use the following switches: sv2v -E=Always -E=Assert -E=Logic pickled.sv Results in the same error. Is sv2v supposed to filter the assertions here?

zachjs commented 1 year ago

I see a few different issues:

  1. The source uses comments like // pragma translate_off, which are not standards-compliant, robust, or portable. sv2v doesn't do anything with these "magic comments". As a workaround, you could do something like: sed -E 's,// (pragma|synthesis) translate_off,\`ifdef SYNTHESIS,' | sed -E 's,// (pragma|synthesis) translate_on,`endif,'.
  2. There is a stray port connection simd_mask_i on instance i_fpnew_bulk of fpnew_top in fpu_wrap.
  3. Some packages are missing from the combined file, e.g., core/include/*.sv common/local/rvfi/rvfi_pkg.sv vendor/pulp-platform/axi/src/axi_pkg.sv.
  4. The default instantiation of, e.g., axi_lite_to_apb is illegal because uses logic as the default type for what must be a struct. Ideally the default instantiation of a module would be legal, or no default parameter value would be specified. Are you using that module? I can look into building some workaround into sv2v.
  5. You probably don't need to exclude those conversion phases in sv2v.
zachjs commented 1 year ago

@KatCe Are you still facing issues with the above? I'd love to hear your feedback!

zachjs commented 1 year ago

@emfredrickson @KatCe I am closing this issue given the inactivity and because I think I have resolved the original request. Please reopen or file a new issue if you have any questions! In the future, I could work on passing through port connection attributes rather than dropping them if there is any interest.