String parameters cannot propagate over design

[mtdudek]

Hi, I'm using sv2v version v0.0.7-32-geae46b7. I was trying to build ibex for arty with flow sv2v->yosys->vivado and I've encountered this issue.

SRAMInitFile is string like parameter that is passed further to memory module as its MemInitFile parameter. By default MemInitFile parameter is size 1 and it cannot contain value of SRAMInitFile parameter resulting in error when $readmemh is run by yosys to initialize memory.

To fix this issue I changed: parameter SRAMInitFile = "" to parameter [8*100:1] SRAMInitFile = ""

To run this flow you can use this edalize branch. It has everything you need to launch sv2v as part of toolchain. Also add this to your top_artya7.core

synth_surelog:
    default_tool: vivado
    filesets:
      - files_rtl_artya7
      - files_constraints
    toplevel: top_artya7
    parameters:
      - SRAMInitFile
      - PRIM_DEFAULT_IMPL=prim_pkg::ImplXilinx
    tools:
      vivado:
        part: "xc7a100tcsg324-1"  # Default to Arty A7-100
        synth: "yosys"
        yosys_synth_options: ['-iopad', '-family xc7', '-run :check', "frontend=sv2v"]
        yosys_read_options: ['-DPRIM_DEFAULT_IMPL=prim_pkg::ImplXilinx','-noassert','-defer']
        frontend_options: ['--write=adjacent', '-DSYNTHESIS']

Now you can build with commandfusesoc --cores-root=. run --build --tool vivado --target=synth_surelog lowrisc:ibex:top_artya7 --part xc7a35ticsg324-1L --SRAMInitFile="../../../examples/sw/led/led.vmem"

[zachjs]

Thank you for filing this issue! sv2v does include a conversion intended to support variable length string parameters, but with everything going on here it can be difficult to say what exactly isn't working as expected. Do you have a method of reproducing this that doesn't depend on tools besides sv2v and Yosys? A minimal reproduction example will greatly help me to identify the issue and produce a fix.

[mtdudek]

I'll try to create small test case that will reproduce this issue

[nmoroze]

I just ran into this issue as well, here's a small minimal example!

Here are three files -

mem.sv

module mem (
    input [1:0] addr,
    output[3:0] data
);
    reg [3:0] mem[4];
    parameter InitFile = "";

    initial begin
        if (InitFile != "") begin : gen_meminit
            $display("Initializing memory %m from file '%s'.", InitFile);
            $readmemh(InitFile, mem);
        end
    end
    assign data = mem[addr];
endmodule

top.v

module top (
    input [1:0] addr,
    output [3:0] data
);
    mem #(
        .InitFile("test.mem")
    ) mem (
        .addr(addr),
        .data(data)
    );
endmodule

test.mem

0 1 2 3

If you put these in the same directory, convert mem.sv to mem.v with sv2v mem.sv > mem.v, and then run yosys -p "read_verilog -sv top.v mem.v; hierarchy", you get an error like:

Initializing memory $paramod$7edc3a56ba94a07b186c98b138761a1c98a1379c\mem from file ''.
mem.v:0: ERROR: Can not open file `` for \$readmemh.

However, this code seems to elaborate fine using the original SV in Vivado.

[zachjs]

@nmoroze Thanks for sharing this example! In this case, sv2v needs the opportunity to convert top so that the instantiation of mem may be elaborated to augment the string parameter binding. sv2v's string parameter conversion adds an additional parameter providing the length of the string. I'm open to suggestions regarding the handling of string parameters.

[nmoroze]

Ah, that makes sense. The actual scenario where I ran into this problem was something similar (unconverted Verilog top-level instantiating an sv2v-converted submodule), but I can just convert the top-level as well no problem, so I should be all set for my use case! Thanks for the prompt reply, and sorry for potentially adding noise to the thread - not sure anymore if OP is actually facing the same issue.

[mtdudek]

I've read on xilinx forums that construction parameter NAME = "" is valid verilog construct Here is link to it https://forums.xilinx.com/t5/Synthesis/Is-there-support-for-string-parameters-in-SystemVerilog/td-p/696420

[zachjs]

The string parameter conversion was targeted at things like https://github.com/bespoke-silicon-group/basejump_stl/blob/5f085ae/bsg_misc/bsg_concentrate_static.v, where the size of the variable-length string parameter must be known. Because $bits is not part of Verilog-2005, and because usages may (quite reasonably) depend on the default value of that width parameter, the instantiations of the module must explicitly provide the width of the parameter.

As of 7ffea36ddd96568c06f71c6eb1a26c8331f3f10d, I've updated the variable-length string parameter conversion to only apply to modules which actually depend on the type/size of the parameter. For @nmoroze's case, this should allow you to avoid converting the top-level module in certain cases. As for @mtdudek's original issue, I'm curious if you see better results with this change.

[mtdudek]

@zachjs I'm grateful for your last commit. It fixed my issue