Issue with hierarchical cast not accepted by Yosys

[flaviens]

Hi Zach!

I'm trying to bring the following sv file into yosys: ariane_pickled.zip.

sv2v does not complain (the -E flag does not seem to influence here) but Yosys says the following:

> sv2v -E=UnbasedUnsized -DSYNTHESIS -DVERILATOR -DARIANE_SMALLER_CACHES generated/ariane_pickled.sv -w sv2v_out.v

> yosys read_verilog -defer -sv sv2v_out.v

1. Executing Verilog-2005 frontend: sv2v_out.v
Parsing SystemVerilog input from `sv2v_out.v' to AST representation.
generated/sv2v_out.v:25484: ERROR: syntax error, unexpected OP_CAST

The problem seems related to a cast with casts inside:

  assign axi_lite_resp_o = '{
    aw_ready: axi_req_valid[WR] & axi_req_ready[WR],        // if AXI AW & W valid & tree gnt_o[WR]
    w_ready:  axi_req_valid[WR] & axi_req_ready[WR],        // if AXI AW & W valid & tree gnt_o[WR]
    b:       '{resp: axi_bresp},                            // from spill reg
    b_valid:  axi_bresp_valid,                              // from spill reg
    ar_ready: axi_req_valid[RD] & axi_req_ready[RD],        // if AXI AR valid and tree gnt[RD]
    r:       '{data: axi_rresp.data, resp: axi_rresp.resp}, // from spill reg
    r_valid:  axi_rresp_valid                               // from spill reg
  };

I'm using the newest versions of both sv2v and yosys.

EDIT: I resolved the issue by refactoring code but it may still be an interesting point.

How would you recommend solving this problem? Thanks!

[zachjs]

Thank you for sharing this issue! I'm glad to hear you've been able to work around it.

I believe sv2v's behavior can be improved here. The illegal output is generated because:

• Modules such as axi_demux have type parameters that default to logic. However, these modules are invalid when instantiated with their default type parameters.
• sv2v keeps around modules that aren't instantiated (even if they have type parameters) because they might be top-level modules, or the user may be converting a design in pieces, even though this isn't recommended.

I see a few different ways to address this:

1. Add a --top parameter to sv2v. In this case, removing uninstantiated modules except ariane_mem_top fixes the issue, shrinks the output, and speeds up conversion a bit. This flag could lend itself to a nice workflow: pass everything into sv2v, specify your top module (like you would downstream), and get exactly the output you need. This may not be clear to users, however.
2. Make sv2v detect and omit invalid uninstantiated modules with type parameters. I hacked something together which works in this case, but it could be quite complex to address this issue in general. Users may also be surprised to find modules automatically "missing" from the output.
3. Remove the invalid default type parameters in the source. I'm not sure why the authors have opted to provide illegal default values here. The standard allows them to be omitted to better communicate the intent to tools and users. That said, I don't think it's reasonable for me to request others change their "style", and this doesn't solve the general problem in sv2v.

I've greatly appreciated your input on past issues. Can you share your thoughts on the above?

[flaviens]

Hi Zach! Thank you for looking into it! Indeed there seems to be no perfect solution that would fit all cases. imo the --top flag (whose potential confusion could be alleviated by another flag name, but I have no great suggestion on top of my head ^^') seems to be the most reasonable alternative, even though arguably this could be considered not to be the job of sv2v. One could argue that it's not great if some other source code uses the big Verilog file as a kind of module library, but kn general this is already problematic as soon as there are module parameters. I agree with your last point as well. We cannot hope to enforce (even very reasonable) "sv2v best practices" everywhere and forever and should probably minimally rely on them.

Also, this issue seems to be a rare issue that is fairly easy to solve without sv2v modifications. While it would be nice, maybe resolving it is not worth your time at this point.

[zachjs]

Thank you for your feedback! In 911243dac428c3145ac68e12a3f56bc5b8db58f7, I added a --top flag which can filter modules in the intuitive way (e.g., -s in iverilog). When using --top ariane_mem_top on your original test case, the invalid modules are filtered away. This may be a nice flag to use in general: on your test case, the conversion is about 30% faster and the output is nearly 50% shorter.

[flaviens]

Thanks! That looks great! I think it resolves the issue, I'll close it :+1: