On SystemVerilog Coding Conventions - Challenging the Status Quo

When I first started out I remember reading all of these nice naming conventions for SystemVerilog. For example, when developing a new verification component, we're supposed to choose a descriptive name for it, preferably as short as possible. To distinguish it from other VCs doing the same thing, we should add a prefix that's unique to our organization to the name we chose. If, for example, we would be developing an AHB VC, a recommended name for it would be vgm_ahb. For the name of the package that contains the code we should add the pkg suffix, hence it should be called vgm_ahb_pkg. Any classes we write for our VC should also contain vgm_ahb in their names. For example, our driver class would be called vgm_ahb_driver, our monitor would be called vgm_ahb_monitor, etc.

I didn't pay too much attention to this at first. As a beginner I thought that there were surely good reasons for these conventions and I just happily followed them. Besides, everyone else was doing it. If we look at UVM, the package we import is called uvm_pkg. The classes we use are called uvm_object, uvm_component, etc.

Lately I got to thinking more about this and now I'm asking myself the question "Why?". A reason we might get for why things are like this is so that we can use the ever popular "wildcard import" operator, import some_pkg::*, and still avoid naming collisions. If we try to wildcard import two constructs that have the same name, the compiler will get confused. Making sure that class names are unique means that we'll never have any problems. The jury's still out on whether wildcard imports are good or bad. I haven't yet read any justification that managed to sway me one way or the other, so this is a discussion for another time.

What we can notice, though, is there is a lot of redundancy in the names. For UVM, for example, there are uvm prefixes flying around everywhere. This includes types and enumeration literals. The C language doesn't have any scoping constructs, so everything is visible. This gives people headaches when linking, as name collisions are possible. To avoid this, C developers typically try make their names unique. In SystemVerilog, however, we have the package concept, which is very similar to Java's packages or C++'s namespaces.

If we were creating a UVC, following the current guidelines it might look like this:

// file: vgm_ahb_driver.svh

class vgm_ahb_driver extends uvm_driver;
// ...
endclass
// file: vgm_ahb_monitor.svh

class vgm_ahb_monitor extends uvm_monitor;
// ...
endclass
// file: vgm_ahb_agent.svh

class vgm_ahb_agent extends uvm_agent;
vgm_ahb_driver drv;
vgm_ahb_monitor mon;

// ...
endclass
// file: vgm_ahb_pkg.sv

package vgm_ahb_pkg;
import uvm_pkg::*;

`include "vgm_ahb_driver.svh"
`include "vgm_ahb_monitor.svh"
`include "vgm_ahb_agent.svh"
endpackage

That's a a bit too much vgm_ahb for my taste. What if we tried to shorten some of these names? Let's just cut the vgm_ahb prefix:

// file: driver.svh

class driver extends uvm_driver;
// ...
endclass
// file: monitor.svh

class monitor extends uvm_monitor;
// ...
endclass
// file: agent.svh

class agent extends uvm_agent;
driver drv;
monitor mon;

// ...
endclass
// file: vgm_ahb.sv

package vgm_ahb;
import uvm_pkg::*;

`include "driver.svh"
`include "monitor.svh"
`include "agent.svh"
endpackage

If we were to have a testbench that instantiates an AHB agent, the code would look like this:

class env extends uvm_env;
agent ahb_agent;

// ...
endclass

If we were to have a second AXI UVC built in the same way, then we'd need another way to be able to differentiate between the two agents, since the class names are now identical. We can just use the scoping operator:

class env extends uvm_env;
vgm_ahb::agent ahb_agent;
vgm_axi::agent axi_agent;

// ...
endclass

Why this is basically the same as using prefixes for our class names, isn't it? (There is one extra character in there because a "::" is longer than a "_", but in the grand scope of things I'd say it's negligible.) We could even go one step further and (in about 20 years) have UVM cleaned up. This would make our driver (and other component) definition(s) look like this:

// file: driver.svh

class driver extends uvm::driver;
// ...
endclass

This frees up UVC developers to use short and descriptive names for their classes. The discussion whether to use wildcard import or not becomes orthogonal. For classes where there aren't any name collisions it's just going to work. For classes where there is one, we just use the explicitly scoped name, which is anyway (almost) as long as if we would have used the prefix.

This also applies to type definitions and enumerated literals; basically to anything that has package scope. What can't be scoped, though are macros. Those annoying `uvm_object_utils still have to contain a prefix, since they can't be defined in packages. That's ok, though, since we all agree that macros are evil and we won't use them, right? (Yeah, right!)

Going the way of no prefixes has a tiny inconvenience with the current generation of tools, though. When we compile a testbench, we very often want to do the whole compile with one tool call:

compiler +incdir+path/to/vgm/ahb vgm_ahb.sv +incdir+path/to/vgm/axi vgm_axi.sv ... 

If both packages include files called driver.svh, then for the case above we'll have the nice surprise that the AXI package includes AHB source files instead of its own. This is because the AHB directory is going to be searched before the AXI directory. We just have to separate the compile into multiple tool invocations:

compiler +incdir+path/to/vgm/ahb vgm_ahb.sv
compiler +incdir+path/to/vgm/axi vgm_axi.sv ...

How often do we need to compile our UVCs anyway? I think this is something we could live with for now, but it would have been really great if the compiler would have automatically searched the directory where the package is located. Then we wouldn't need any funky +incdir+ arguments and this whole issue could have been averted.

Another topic I want to touch upon is those include guards we all see everywhere. I guess this is C++ legacy, where we need to include library headers to use them. The use model in SystemVerilog (similar to Java) is to import packages, though. Coming back to the example above, it doesn't make any sense for a verification environment to include the AHB driver.svh file, since that file file comes bundled with the vgm_ahb package. Trying to protect ourselves from including that file again is analogous to trying to protect ourselves from others including our cpp files in C++. Anybody who does this will be immediately informed by the compiler that something is wrong.

Coming back to macros again, that's the only place where such include guards have any place, since a macro header could get included in multiple files in the current compilation unit. I guess a lot of the confusion stems from the fact that the same svh file extension is used for both code that is intended to be included by the user (i.e. macro headers) and for code that package developers are supposed to include in their packages (as was the case above for driver.svh, monitor.svh, etc.). Maybe it would make more sense to create a new file extension for the latter case. Off the top of my head, maybe svi (SV implementation) would be a good name. Then we could publish the guideline that svh files can be included, but svi files cannot.

SystemVerulog is mixed up enough, with its design/hierarchy constructs on one side and OOP constructs on the other. The OOP constructs themselves are a confused amalgamation of C++ and Java. We can't just blindly take conventions from the latter languages and bolt them on here. We're going to need to develop our own coding conventions that make sense in the context of this language.

Comments