Interface port array not expanded correctly

[tpoikela]

Hi. I tried this input:

interface test_if();

parameter int DW = 32;

logic[DW-1:0] data;

modport consumer (
    input data
);

endinterface

module test_mod(
    input clk,
    input rst,
    test_if.consumer two_ifs_in[2],
    output[31:0] dout
);

// test_if two_ifs[2](); // PARSE ERROR

reg[31:0] reg_ff;
assign dout = reg_ff & two_ifs_in[0].data;

always_ff@(posedge clk or negedge rst)
if (rst == 1'b0) begin
    reg_ff <= 0;
end
else begin
    reg_ff <= c_if.data;
end

endmodule

Expected result: Interface port array would be expanded like two_ifs_in_0_data and two_ifs_in_1_data (or similarly, even using 2d arrays but that is poorly supported by some open source tools). Also, expected that line commented with // PARSE ERROR would be parsed correctly, but it causes an error, if left uncommented.

Actual result: Only one instance of interface is added (as if I had port declaration test_if.consumer two_ifs_in).

This is the output I got (wrong lines are also commented):

module test_mod (
        clk,
        rst,
        two_ifs_in_data,
        dout
);
        input clk;
        input rst;
        parameter signed [31:0] DW = 32;
        input wire [DW - 1:0] two_ifs_in_data;  // WRONG, should be two instances or 2d array
        output [31:0] dout;
        reg [31:0] reg_ff;
        assign dout = reg_ff & two_ifs_in[0].data;  // OK, if interface port array expanded to 2d array, WRONG otherwise
        always @(posedge clk or negedge rst)
                if (rst == 1'b0)
                        reg_ff <= 0;
                else
                        reg_ff <= c_if.data;
endmodule

Let me know if more details are needed.

[zachjs]

sv2v does not yet support interface arrays. This will likely be a significant task. Before I get started, I'm curious what the aspects of this feature are used commonly.

• I haven't stumbled across designs using this feature. How common is it?
• I imagine it's not uncommon for the dimension of the interface to be based on a parameter. Is that true? If so, this rules out unrolling the arrays directly.
• I'm considering inserting the array dimensions into each of the underlying data declarations. This might yield slightly confusing output, as two_ifs_in[0].data would become something like two_ifs_in_data[0+:32].
• The specification allows for multi-dimensional interface arrays. Does anyone use them?
• I believe it's legal to to create an interface array and then bind individual elements or slices to ports. Do people do that?

[tpoikela]

I haven't stumbled across designs using this feature. How common is it?

I've used this in 2 ASIC tapeouts, and currently using it one project. Maybe lack of support in open-source tools is one reason it's not so common. But at least Verilator is supporting it. So would be nice to get it working for sv2v/Yosys combo.

I imagine it's not uncommon for the dimension of the interface to be based on a parameter. Is that true? If so, this rules out
unrolling the arrays directly.

In designs I've done, this is the case. Parameter has been used.

I'm considering inserting the array dimensions into each of the underlying data declarations. This might yield slightly confusing output, as two_ifs_in[0].data would become something like two_ifs_in_data[0+:32]

If it's the easiest to implement, I would not worry about confusing output too much, as long as it works correctly. During the last year, I've seen lots of handcoded VHDL using this kind of scheme.

The specification allows for multi-dimensional interface arrays. Does anyone use them?

I have not used them due to bad support from tools. Is it lots of extra work on top of 1-d interface arrays?

I believe it's legal to to create an interface array and then bind individual elements or slices to ports. Do people do that?

I think it's very valid usecase. Imagine you have like 8x8 crossbar, and you connect interface port array [8] to the crossbar, then each individual element to different IP blocks connected to the crossbar.

[zachjs]

I've added flattening for instances, so the multi-dimensional case will be taken care of. I need to put some work into the interface conversion before I can really start adding support for this.

[zachjs]

I've expanded the capabilities of the interface conversion, most notably to allow the use of complex modport expressions. I think I'm getting closer to a point where support for interface arrays could be added. If you have other examples using interfaces (but not interfaces arrays), I'd be interested in seeing if there are any edge cases I've missed before proceeding.

[tpoikela]

Thanks a lot for the effort on this one! I tried sv2v for 70k lines of some autogenerated SV register code, which uses lots of interfaces, but the conversion was OK, and iverilog compiled the generated code. Tried sv2v to a bigger, hand-written design but I could not get packed unions to work correctly, so I gave up.

Two "corner cases" I came up with are using generic interface port like this:

module test_mod (interface gen_if);
endmodule

This seems to produce syntax error.

Another one is modports within generate:

interface intf_t #(parameter int NUM_CLIENTS = 0);
    bit [NUM_CLIENTS-1:0] req;
    for (genvar i=0; i< NUM_CLIENTS; i++) begin: mps
        modport client_mp (output .client_req( req[i] ));
    end
endinterface

For this, the final expansion in the top-level module is not exactly correct:

Input:

// Bus interface with parameterized number of client modports
interface intf_t #(num_clients = 0); 
    bit [num_clients-1:0] req;
    for (genvar i=0; i< num_clients; i++) begin: mps 
        modport client_mp (output .client_req( req[i] )); 
    end 
endinterface

// A generic client that attaches to the bus
module client_m (intf_t client_ifc);
// ... code will drive client_ifc.client_req
endmodule

module top;
    
    parameter N = 8; 
    intf_t #(.num_clients(N)) intf();
    for (genvar j=0; j < N; j++) begin: clients
        client_m client (.client_ifc (intf.mps[j].client_mp));
    end

endmodule

Output:

module client_m (client_ifc_req);
        parameter client_ifc_num_clients = 0;
        inout wire [client_ifc_num_clients - 1:0] client_ifc_req;
endmodule
module top;
        parameter N = 8;
        genvar intf_i;
        parameter intf_num_clients = N;
        wire [intf_num_clients - 1:0] intf_req;
        generate
                genvar j;
                for (j = 0; j < N; j = j + 1) begin : clients
                        client_m client(.client_ifc(intf_mps[j].client_mp)); //ERROR
                       // Correct conversion might be: client_m client(.client_ifc( intf_req[j]));
                       // But I'm not sure this is generic enough and works in all cases
                end
        endgenerate
endmodule

[zachjs]

Regarding packed unions: While sv2v has support for packed unions, I don't think I have any test coverage for them. If you have found a bug, please file an issue.

sv2v does not currently have the ability to resolve the types of expressions referring to declarations within generate loops. This is definitely something I'd like to get around to. It doesn't look like support for modports within generate loops is very widespread, so this probably isn't very high on my priorities for this project.

Supporting generic interface bundles can also be added. sv2v also doesn't handle modules instantiated with different modports in different instantiations, which would likely be handled by the same conversion procedure as generic interface bundles. Initially, I considered handling these similar to how I handle modules with type parameters: consider each instantiation of the module, and create new versions of the module for each different set of type parameters, or in this case, for each interface/modport binding combination used to instantiate it. However, in drafting up a test case, I realized my current approach to converting interfaces might be fundamentally flawed.

interface InterfaceM;
    logic [4:0] x = 0;
    task hello;
        input integer inp;
        x = x + 1;
        $display("Hello from InterfaceM %0d %b", inp, x);
    endtask
    modport A(input .x(x[0]));
    modport B(input .x(x[2:1]));
endinterface

// could get bound to the entire interface bundle, or either modport
module ModuleM(InterfaceM i);
    initial i.hello(-1);
    initial $display("ModuleM got %b", i.x);
endmodule

module top;
    InterfaceM im();
    ModuleM x(im.A);
    ModuleM y(im.B);
    ModuleM z(im);
endmodule

When task hello is called from the module, x refers to the x of the interface, not the modport. And in fact, all of the instances of ModuleM have the ability to write to x! While these tasks are obviously not synthesizable, one can easily imagine having multiple modules using the same interface writing to the same register, which would yield a similar issue.

Perhaps I am missing something, but I don't think this can be represented in Verilog with module instantiations, even if each instance referred to a different version of ModuleM, as the underlying data would have conflicting drivers. Because of this, I think the only way this could be converted would be to inline the instances of ModuleM. However, if someone did something like having a recursive interface instantiation that depended on a parameter value, inlining in this way can't be done at conversion time. I'm curious as to your thoughts on these issues.

[zachjs]

Update: I am working on supporting generate block scoping and changing the interface conversion to inline modules. It may be a couple weeks before you see significant progress on the key interface conversion features identified in this issue.

[tpoikela]

Thanks for the update, and working on this! And couple of weeks does not sound bad at all.

Regarding the question about your example, it is not something I've used before. I'm not sure if the task should be even visible through modports unless it is imported by the modports. That's the case for logic/wires inside the interface. But I'm sure different modules cannot access different slices of x this way using the task. The "full" x is visible in the task always, due to the scoping rules of SV. Modports cannot change this scoping or affects how tasks are referring to variables inside interface.

[zachjs]

Sorry for delay in pushing progress on this! I just pushed major update to the interface conversion. It now inlines modules with modport bindings. Modules and interfaces inlined preserve their hierarchy using generate blocks. Support has been added for modports defined within generate loops and for generic interfaces. The conversion should be significantly more robust.

@tpoikela @b1f6c1c4: Could you give the new conversion a spin? Please note that Yosys does not currently support some of the Verilog-2005 generate scoping features now used by this conversion. I have Yosys PRs either published or incoming addressing these limitations.

Without too much additional effort, I believe I should be able to handle interface arrays where modport bindings are 0-dimensional, and I believe I have a path to add support for slices.

[zachjs]

As of 2b377ce, I have added support for interface arrays, modport array bindings, and interface-using module instance arrays. Please let me know if you find any issues!

[tpoikela]

I tried with v0.0.6, but it produces no output with the original test file in my 1st comment on this issue. Also, a simpler test file with an interface seems to be failing. v0.0.5 is able to convert a simple file correctly. Shall I file another issue for another test file?

[tpoikela]

I also tried compiling 2b377ce, and it seems to have the same issue as v0.0.6. No output produced for a simple interface conversion test.

[zachjs]

Interfaces and modules using modports are now inlined during conversion, removing the originals. Running with -v/--verbose retains some comments which indicate what has happened, though I recognize the behavior is surprising. Adding a module which instantiates test_if or test_mod should yield better results.

[tpoikela]

Ok, I see.

So after adding this:

module top();                                                                                    
    test_mod i_test_mod();
endmodule: top

I see the following output:

module top;           
    generate          
        begin : i_test_mod                     
            wire clk;         
            wire rst;          
            test_if.consumer two_ifs_in [0:1];         
            wire [31:0] dout;                    
            reg [31:0] reg_ff;   
            assign dout = reg_ff & two_ifs_in[0].data;
            always @(posedge clk or negedge rst)
                if (rst == 1'b0)         
                    reg_ff <= 0;
                else
                    reg_ff <= c_if.data;
        end                                                                                                           
    endgenerate                                                                                                       
endmodule

But the output is not Verilog and neither it's in SystemVerilog anymore. Also, now top-level ports are removed, so I cannot really access the design through ports anymore. This module cannot be easily used in synthesis either as I cannot write any constraints for input/output ports. I'm not sure if synthesizer preserved anything without top-level ports, or would just remove everything as unconnected. Not sure if I'm still missing something.

It seems that some files can be converted without additional instantiation and produce correct output. So far, these have been files without interfaces. But when I use interface port in a module, it simply prints // removed interface-using module. It would seem like I cannot use any interface ports in the top-level module. I would say this is pretty bad limitation, and limits the use drastically.

I would expect the test_mod to be converted like this:

module test_mod#(
parameter DW=32
)
(
    input clk,
    input rst,
    input[DW-1:0] test_if_consumer_two_ifs_in_data_0,
    input[DW-1:0] test_if_consumer_two_ifs_in_data_1,
    output[31:0] dout
);

Even if the param DW is dropped, and hardcoded to default value, that would not be too bad.

[zachjs]

I would argue leaving two_ifs_in unconnected is not legal. sv2v does not currently notice and error in this case the way other tools do (I can fix that). However, using a top level module like the following produces more reasonable results:

module top;
    test_if two_ifs[2]();
    wire clk;
    wire rst;
    wire [31:0] dout;
    test_mod i_test_mod(clk, rst, two_ifs, dout);
endmodule

module top;
	generate
		genvar _sv2v_arr_B1DE2;
		for (_sv2v_arr_B1DE2 = 0; _sv2v_arr_B1DE2 <= 1; _sv2v_arr_B1DE2 = _sv2v_arr_B1DE2 + 1) begin : two_ifs
			localparam signed [31:0] DW = 32;
			wire [31:0] data;
		end
	endgenerate
	wire clk;
	wire rst;
	wire [31:0] dout;
	generate
		localparam _sv2v_bind_base_2679F_two_ifs_in = 0;
		begin : i_test_mod
			wire clk;
			wire rst;
			localparam _sv2v_base_two_ifs_in = _sv2v_bind_base_2679F_two_ifs_in;
			wire [31:0] dout;
			reg [31:0] reg_ff;
			assign dout = reg_ff & top.two_ifs[_sv2v_base_two_ifs_in].data;
			always @(posedge clk or negedge rst)
				if (rst == 1'b0)
					reg_ff <= 0;
				else
					reg_ff <= c_if.data;
		end
		assign i_test_mod.clk = clk;
		assign i_test_mod.rst = rst;
		assign dout = i_test_mod.dout;
	endgenerate
endmodule

Back in June I noticed that there are synthesizable sources using interfaces which cannot be converted without inlining/instantiating modules. I opted to make the interface conversion always use this strategy for simplicity. Note that interface ports cannot always be trivially expanded/flattened into bundles of wires. For example, the dimensions of a modport array could depend on a parameter, or a single module could be instantiated with different interface or modport types. Instantiating modules in this way essentially preserves the hierarchy of the design using generate blocks, but I can imagine this may interfere with downstream analysis. Is there a way I can address this?

There are several conversions in sv2v which require that the entire design is made available to sv2v. If a top level module uses an interface port, presumably there is a corresponding testbench which also needs to be converted. In the case of synthesis, its not clear to me how one would expect such a top level module to be converted for consumption by downstream tools. Can you help me understand what I'm missing here?

[tpoikela]

Synthesis and simulation are slightly different usecases. I agree that top-level module without ports is OK for simulation. But it does not work for synthesis, as far as I know.

If non-interface port top-level wrapper module is required, I guess it's something people can live with, although somewhat limiting. Verilator also does not support top-level interface ports, and I've seen various EDA tools having problems with them. I don't think the synthesis behaviour is defined in the SystemVerilog LRM, and it's tool-specific.

If the support could be added for top-level interface ports, I would do it like this:
If the top-level module contains an interface port (also using a modport), only the ports exposed by the modport should be converted into input/output ports of the top-level Verilog. In this case, all always-blocks and assign-statements in the interface would not be converted, and would be left out. For tasks imported by module, I'm not sure yet how to handle them. I haven't used them before in synthesizable code. So in the case of top-level interface port, it would have to be treated as bundle of wires, as there is no place to inline the always-blocks and such. You could issue a warning/error from always-blocks and other constructs that cannot be inlined for top-level module though.

For synthesis tools, they use the default parameter values of top-level module, unless overridden from within the tool. I think the only way to handle parametrized modport array in top-level module would be to inline the modport array based on default parameter value in the interface. I can try to come up with an example how I would do the conversion exactly.

a single module could be instantiated with different interface or modport types

Can you clarify this a bit more? A module can connect to several modports in single interface, but that should not be a problem if you expand each modport into bundle of wires. Note that probably in this case the testbench would have 2 or more interface instances (or corresponding bundles of wires), the other modports connecting to testbench components.

[zachjs]

Verilator also does not support top-level interface ports, and I've seen various EDA tools having problems with them. I don't think the synthesis behaviour is defined in the SystemVerilog LRM, and it's tool-specific.

Given the apparent confusion among existing tools, I am inclined to avoid trying to handle this case for the time being. That said, if the spec is updated, or major tools arrive at some consensus behavior, then I’ll happily reconsider.

However, the possibility for sv2v to ingest non-trivial code and output nothing can certainly be surprising. I intend to add warnings for sources involving interfaces or packages which result in no output. I would also like to add an error for instances with unconnected modports.

a single module could be instantiated with different interface or modport types

Can you clarify this a bit more?

I was thinking of a source like:

interface Intf;
    wire x, y, z;
    modport A(
        output x,
        input y
    );
    modport B(
        output x,
        input y,
        input z
    );
endinterface

module top(i);
    Intf i;
    ...
endmodule

wherein the expanded set of parts should arguably depend on which modport top is instantiated with.

[zachjs]

I wanted to follow up and see what your milage has been with the current state of the interface conversion. One major concern had to do with ease of debugging when modules or interfaces are inlined, and I'd be interested in exploring ways of alleviating any current pain. Please let me know what you think!

I've also added the additional errors and warnings I mentioned earlier, which should reduce surprises.

[zachjs]

Thank you very much for all of your suggestions and feedback! Working through these features and the adjacent requirements they created was eye-opening, and I believe sv2v has benefitted greatly.

I'm closing this issue as the stated features have been implemented and it's been inactive for some time. I'm still interested in exploring potential ways of making the output easier to use with downstream tools, and potentially developing some standard for expanding top-level modules with interface ports. If anyone has requests or suggestions regarding the interface conversion, please feel free to open another issue!