B-Lang-org / bsc

Bluespec Compiler (BSC)
Other
952 stars 146 forks source link

Verilog primitives should probably be renamed #139

Open thoughtpolice opened 4 years ago

thoughtpolice commented 4 years ago

Some of the Verilog primitives that are used by bsc-emitted code have rather unfortunate names. For example, taking the name TriState or Counter seems rather rude in the scope of a global project. Unfortunately, Verilog has no mechanism for controlling visibility of symbols, or doing namespaced/hygenic imports, so we're left with few options other than just renaming them to be as unique as possible. This is basically the same problem encountered in C++ headers which is why __they_use_lots__of_underscored_names_you_wont_choose.

While I don't think we need to go so far, perhaps just renaming all the modules to have the prefix __BLUESPEC_ would be enough? So FIFO0 becomes __BLUESPEC_FIFO0.

I also think that to be "future compatible", we should encourage tools interfacing with bsc to:

1) Scan the Verilog directory, and track that every .v filename will map to a single Verilog module name (e.g. __BLUESPEC_FIFO1.v contains module __BLUESPEC_FIFO1(...).) This could be made part of the bsc UI as well or something -- perhaps bsc -prim-to-file __BLUESPEC_FIFO1 could print out the filename for use in automation, or you could just use Tcl. 2) Assume any __BLUESPEC_ entities that appear as "blackboxes" in a design can have the blackbox "filled" with one of these modules. This is what tools like yosys-bluespec do: they assume any missing entities in the module hierarchy that match the files in the Verilog/ directory are to be "filled" by those modules. 3) Users can also just add all the files to their project and let the synthesizer figure it out, like they currently do.

This should mean that users, provided they automate their tooling correctly, should be mostly oblivious to adding/changing/renaming Verilog primitives.


On a side note, lib/Libraries/main.v, which provides a tool-neutral harness for "one-click" simulation compiles, should probably be renamed and moved out of this directory, since it's not meant to be used in simulation, and confuses the above logic.

thoughtpolice commented 4 years ago

I should note that as an alternative to option 3 above, I am using a feature of Yosys called the "RPC frontend" to solve this problem. Briefly, you can register a set of module names with Yosys at startup, and tell it that if it encounters instantiations of those modules in a design, to generate those module source on-demand using a specific command. So the idea is that if I synthesize a Bluespec module top, and the emitted code uses FIFO0 (in FIFO0.v), then I register FIFO0 with the RPC callback, and the synthesis tool will "resolve" that module by executing cat /usr/local/bluespec/lib/Verilog/FIFO0.v to get the Verilog source code, and stuff that into the design/compilation and proceed. This makes the use of Bluespec with Yosys far more "transparent" because you do not have to worry about managing primitives.

So in my case, "stealing" names like Counter seems a bit unfortunate. I don't necessarily want a Counter entity in the design to be mapped to the bluespec one implicitly, just because the compiler decided to!


(yosys-bluespec solves this in a different way by manually inspecting the design using the Yosys C++ API, and was originally written long before the RPC frontend existed, but it broadly does the same thing.)

quark17 commented 3 years ago

I am in favor of renaming the primitives with a prefix, to avoid name clash. I suggest __BSC_, since that is the tool name.

Your original post is a bit confusing to me, though. I think your bullet points 1 and 2 describe how people already use BSC, and I imagine that people only do 3 when they are providing their generated files to third parties who don't have BSC. (For example, with Vivado, I provide a search path: first it looks in my design directories, then it looks in BSC's Verilog.Vivado, then in BSC's Verilog.) (Bluetcl can also be used to walk the .ba files for a design, to identify the imported modules that it uses; there's a script in util that does this. Bluespec had an automated FPGA flow that used a script like this to identify which files specifically to include from the Verilog* directories, but now we use search paths rather than listing files individually.) So I agree that it's important that automation allows users to be oblivious to changes in the primitives, but don't see that we're doing anything that contradicts that? In any case, there is still a flat namespace, so I also agree that using a unique-ish prefix is a good idea.

On the topic of main: Note that the location of the file is not lib/Libraries/main.v as you typed; it's at lib/Verilog/. So there's nothing in the directory name that implies that the files there have to be only for implementation of libraries. However, I get your point that there are different kinds of Verilog (top-level drivers vs library implementations) and if a user is providing a directory to their tool as a search path, they may want to precisely specify just one of those subsets -- so having separate directories for the different types of Verilog can be useful. (It would also mean that we wouldn't have to rename main to be __BSC_main.) Soon we will also want to support Verilator as a simulator and, in that case, a main.cpp will be needed instead of a Verilog driver. So maybe a new directory created for drivers, under which we can have different directories according to tools or languages; and maybe the Verilog directory could be placed under a parent directory (or renamed) to indicate that it is for libraries.