Issue with the PULPissimo SoC

[flaviens]

Hi Zach!

I encountered an issue with working with sv2v on the PULPissimo SoC. My sv2v version is currently sv2v v0.0.7-114-g836536c.

Prerequisites

https://github.com/pulp-platform/bender.git https://github.com/zarubaf/morty.git

Steps to reproduce

git clone https://github.com/pulp-platform/pulpissimo.git
cd pulpissimo
bender sources -f -t rtl -t synthesis > bender.sources
sed -i 's/sv,"/sv"/g' bender.sources
morty -f bender.sources -DVERILATOR -DSYNTHESIS > pickled.sv
sed -iE 's/[\(]\*[^\*]*\*[\)]//g' pickled.sv
sv2v pickled.sv

Outcome

pickled.sv:32070:37: Parse error: unexpected token '{' (Sym_brace_l)

which corresponds to

    assign addr_map[i] = axi_rule_t'{
      idx:        i,
      start_addr: addr_t'( i   * AxiStrbWidth),
      end_addr:   addr_t'((i+1)* AxiStrbWidth)
    };

Also, once I received today with the same pickled file (but cannot reproduce it):

sv2v: src/Convert/Scoper.hs:480:17-59: Non-exhaustive patterns in Variable Local
                                         newType
                                         newName
                                         []
                                         Nil

Please let me know if you need more indications. Your work is much appreciated! :)

[zachjs]

Is there any chance you could upload the input file? This would speed things up on my side.

[flaviens]

Sure! Just give me a minute

[flaviens]

Here it is: https://github.com/flaviens/pickled_pulpissimo/blob/master/pickled.sv

[zachjs]

On both the uploaded file and the original source, I run into a few different issues, but not yet the one you've listed above.

1. sv2v did not yet support what might be called "explicitly typed struct patterns" like Foo'{ x: 1, y : 2}, but this has now been fixed
2. sv2v would erroneously flag certain identifiers within packages as illegally depending on the global scope, but this has been fixed
3. The source file has wildcard imports from both defs_div_sqrt_mvp and riscv_defines in the global scope. These packages both export definitions for C_PC, which causes sv2v to (accurately) flag usages of C_PC as ambiguous. With the conflicting imports residing in the same file, most tools flag the ambiguity. If they are in separate files, it appears most other tools use whichever definition was imported in the earliest file (which is probably not what you want), while at least one vendor appears to implicitly give the wildcard import in the current file higher precedence. However, sv2v does not currently implement any special behavior when dealing with package imports based on the split of the source files.
4. If I hack around the above issue, sv2v correctly complains that the instance of axi_cdc_src in axi_lite_cdc_src_intf refers to ports src_req_o and src_resp_i, which do not exist in axi_cdc_src (the directionality is flipped).

[flaviens]

Hi Zach, thank you warmly for your amazingly fast reaction and the new feature! Also thanks for the interesting explanations.

Now after fixing all the mentioned and applying the fresh sv2v version, I finally (sadly) run again into the aforementioned issue:

sv2v: src/Convert/Scoper.hs:480:17-59: Non-exhaustive patterns in Variable Local
                                         newType
                                         newName
                                         []
                                         Nil

You can reproduce it by applying sv2v to the updated file: https://github.com/flaviens/pickled_pulpissimo/blob/master/pickled.sv

[zachjs]

I've found and fixed a couple more issues, but at least a few still remain. I'll share a more complete progress update sometime tomorrow.

[zachjs]

I've made some solid progress, and have fixed the following issues:

1. The pattern match failure above was due to sv2v not handling functions which used an unpacked typename as a return type. This is now handled by packing the result type, as Verilog-2005 does not permit function result types to have unpacked dimensions.
2. A similar issue existed for parameters/localparams which use unpacked typenames.
3. The interface conversion previously aggressively substituted constants during inlining with the intent of ensuring modport expressions would only refer to top-level constants in the destination module, but this was done in a way that allowed parameters to lose their type information. This aspect of the interface conversion was designed before there was a separate phase for substituting constants across generate scopes. The interface conversion now defers the resolution of these constants to this newer phase.
4. The recently added support for "explicitly typed struct patterns" was not quite as complete as I thought.
5. Type parameters are always resolved iteratively, setting aside "templates", instantiating modules, and adding new parameters as needed. One issue allowed some of these templates to stick around even though they were no longer needed. Another issue allowed certain "partially instantiated templates" to be reduced into a standard modules, despite their second-order parameters remaining unspecified.

While a few other issues may still remain, I have encountered an interesting case in the source. The module axi_to_reg uses logic as the default for certain type parameters which could only reasonably be structs. The module isn't used anywhere in the available source, so sv2v makes the default instantiation of the module available in the output, because such modules could be used externally. However, the default instantiation of this module is naturally garbage, because logic isn't a struct with fields.

I imagine most tools would error on such an input as the following. sv2v doesn't currently catch this as invalid, but I'd argue the source should be fixed regardless.

module bad;
    parameter type T = logic;
    T x;
    assign x.y = 1;
endmodule
module top;
    initial $display("hi");
endmodule

[flaviens]

Hi Zach, Congratulations for all this work! The pickled file indeed passes through sv2v now !

However, there remain some syntax errors in the sv2v output, as Verilator reports. I updated the repository to help you: https://github.com/flaviens/pickled_pulpissimo/ (you may want to re-clone it because it has been rebased).

Just make sure you have Verilator in your path, and make.

[zachjs]

I will look into a pair of issues related to unbased unsized literals. In the meantime, I think you may be interested in setting `default_nettype none. Currently, there are wires like s_int_datasize which are implicitly created as size 1, despite being assigned a wider value.

[zachjs]

I've also noticed the latest pickled file is missing the boot_rom module, which especially needs to be present because it has an interface port.