Skip to content

Use of SystemVerilog Interfaces #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MikeOpenHWGroup opened this issue May 4, 2021 · 12 comments
Open

Use of SystemVerilog Interfaces #52

MikeOpenHWGroup opened this issue May 4, 2021 · 12 comments

Comments

@MikeOpenHWGroup
Copy link

This guide lists SV interfaces as problematic and that their use is discouraged. Do you have a specific rationale for this?

My own experience is that SV Interfaces are invaluable for verification code, and can be used for RTL as long as care is taken with synthesis. I do not know if SV Interfaces are supported by all FPGA synthesis tools (its been a while since I've tried that).

@imphil
Copy link
Contributor

imphil commented May 4, 2021

The style guide at https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md is primarily focused on synthesizeable SV, which passes through a much larger set of tools than DV code, leading to a rather conservative choice of allowed features. We (me personally and others on OpenTitan) had various cases in the past where interfaces were mis-synthesized, partially supported, etc.

Unfortunately I don't have specific test cases or specific bugs at hand to be able to check if that is still a valid concern, making it effectively a risk trade-off: do interfaces provide enough value to try again? For OpenTitan, we didn't find a good reason to use them again, but we'd be very interested in the experience others have!

@sjgitty @tjaychen @eunchan do you remember more details of when you last tried SV interfaces? (I think Scott had a story there about a misinterpretation of some code by a tool very late in the design flow, leading to a hard-to-diagnose bug shortly before tapeout.)

For DV code, SV interfaces are usable and widely used within OpenTitan and Ibex DV, even though we don't explicitly allow them in the DV style guide at https://github.com/lowRISC/style-guides/blob/master/DVCodingStyle.md#interfaces-clocking-blocks-modports.

@sriyerg that's something we should probably spell out more strongly?

@sriyerg
Copy link

sriyerg commented May 5, 2021

We already maximize code-reuse and reduce potential connectivity mistakes by bundling common port signals into structs. I would assume the historic decision to prefer structs over interfaces was to ensure support for a wide range of tools, including some of the open source ones. Perhaps someone can comment on whether there were tools that did not play nice with interfaces.

In general I'd imagine the RTL designer-maintained interfaces will come with limitations (confined to synthesizable subset of SV, for starters), so that weakens the argument of being able to reuse them in DV code. For DV, implementing interfaces is crucial - its the most flexible way to get the dynamic side of SV (OOP / classes-based verification) to talk to the static side (RTL, tb). I'd rather prefer to let the DV have the freedom to implement interfaces the way they see fit (add tasks, functions, assertions, other verification logic etc.).

I don't see a reason to explicitly state that in the DV style guide because its a given (it does not explicitly disallow them either). Though, I am happy to update it if you think it is necessary.

@MikeOpenHWGroup
Copy link
Author

Thanks for the comments everyone. To @sriyerg's comment:

I don't see a reason to explicitly state that in the DV style guide...

I certainty agree with that (and everything else you said for that matter). My comment/question was about the RTL style guide (VerilogCodingStyle), not the DV style guide. Sorry that I was not clear on that point.

You can close this issue if you wish.

@eunchan
Copy link

eunchan commented May 5, 2021

As far as I know, DC, Primetime did not have issues on the interface usage. But the interface should be really basic, mostly bundling the signal only (no clocking). So, at the time, we decided to go for a safer route.

@sriyerg
Copy link

sriyerg commented May 5, 2021

We should update the RTL style guide then to clearly explain why prefer structs over interfaces in that case.

@tjaychen
Copy link

tjaychen commented Jun 14, 2021 via email

@josuah
Copy link

josuah commented Jul 6, 2022

My gratitude to @tjaychen for offering an explanation.

If the name of that tool can be disclosed, we could ask publicly (open-source toolchain) or in private (part of a proprietary toolchain) and report the result here.

@josuah
Copy link

josuah commented Jul 6, 2022

Eventually several aspects of interfaces could be checked to see if an useful subset is still supported, or none at all.

  • inout
  • arrays of interfaces
  • parameters
  • clocking blocks
  • modports in modules ports (module mod (wishbone_bus_if.slave_mp wb); vs module mod (wishbone_bus_if wb);)
  • nested interfaces
  • ...

I am happy to try that list with tools you had doubt with.

@tjaychen
Copy link

tjaychen commented Jul 6, 2022

My gratitude to @tjaychen for offering an explanation.

If the name of that tool can be disclosed, we could ask publicly (open-source toolchain) or in private (part of a proprietary toolchain) and report the result here.

sorry i'm really not sure if i can say it in the open. The project that experienced these issues is itself not an open source project.
However I think we could reach out to the major tool vendors and just ask in general how well interfaces are supported now. Being able to use multi-directional interfaces is IMO cleaner than singular direction structs.

@josuah
Copy link

josuah commented Jul 6, 2022

It is good that this Style Guide also gets tested with the proprietary tools, which could help compatibility across vendors, but also between vendors and open source toolchains.

I did just try today with what I had access to (verilator and yosys), and both support enough of SystemVerilog interfaces for bundling multiple input/output signals together.

As someone who is learning, projects like this, as well as all the code to read from, offer much insight to me.
Thank you all.

@josuah
Copy link

josuah commented Jul 8, 2022

While simulation with Verilator went without trouble, I could not yet find a way to use interfaces that did play well with yosys.

It might be worth having a look at progress of issues such as YosysHQ/yosys#1053 or YosysHQ/yosys#1592 when considering interfaces in a project.

@josuah
Copy link

josuah commented Jul 8, 2022

Someone else could test the below on Lattice and Synplify synthesis engines which are part of Radiant and it worked well:

interface iBus (
	input logic clk,
	input logic rst
);
	logic req;

	modport mpController (
		input	clk,
		input   rst,
		output	req
	);

	modport mpPeripheral (
		input	clk,
		input   rst,
		input	req
	);
endinterface

module mLighter (
	iBus.mpPeripheral  bus,
	output logic [7:0] led
);
	always_ff @(posedge bus.clk, posedge bus.rst)
		if (bus.rst)
			led <= 0;
		else if (bus.req) begin
			led <= led + 'd1;
		end
endmodule

module mTopLevel (
	input  logic clk,
	input  logic rst,
	input  logic req,
	output logic [7:0] led
);
	// instantiate the bus interface
	iBus bus (.clk(clk), .rst(rst));

	// issue a request over the bus
	always_ff @(posedge clk, posedge rst)
		if (rst) bus.req <= 0;
		else     bus.req <= req;

	// instantiate the module
	mLighter peripheral0 (
		.bus(bus.mpPeripheral),
		.led(led             )
	);
endmodule

unknown

This lets us yosys, icarus verilog, and possibly other vendor tools that lack a complete enough support for them in synthesis.
And Verilator, Radiant, and possibly other vendor tools that have enough coverage for a fully working synthesis.

P.S.: sorry for the non-conformant indentation, I am in the process of converting my sources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants