Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  BugVise
  CovVise
  Force-Gate-Sim
  Gspice
  IPC::Locker
  Rsvn
  SVN::S4
  Voneline
  WFH
General Info
  Papers

Issue #1363

CMake and Python support

Added by Patrick Stewart 5 months ago. Updated 5 months ago.

Status:
AskedReporter
Priority:
Low
Category:
Unsupported
% Done:

0%


Description

I've added support for building with CMake (the verilated output, not verilator itself), and for generating a python module as output using pybind11. The code is here: https://github.com/patstew/verilator/tree/python I've added documentation and examples for each, hopefully they're clear enough.

History

#1 Updated by Wilson Snyder 5 months ago

  • Category set to Unsupported
  • Status changed from New to AskedReporter
  • Assignee set to Patrick Stewart

Nice patch set, you seem to have mastered the key stuff.

Though not explicitly stated I assume your goal is to eventually get this merged upstream so here's some comments.

First, please separate out the python and cmake patches (it seems python needs cmake though so would build on that patch), then when ready (below) file a new bug for python.

As to the cmake patch my main concern is I haven't used CMake and when there are bugs a few years from now related to CMake are you willing to signup to support fixing them?

The same applies to the python code, but here I need to soul search a bit as to if I want to accept it. This feels more of something for your personal use case and unlikely to be widely useful. Perhaps use it for at least a few months and see how it settles out.

--- a/bin/verilator
+++ b/bin/verilator
+    --cmake                     Generate CMake file list instead of MakeFile

Maybe it should be --make cmake (versus the default "--make gmake" similar to the "--compiler gcc" flag), so when yet another build system we don't need to keep adding unique flags?

+    verilate(target TRACE SYSTEMC NAME name TOP top SOURCES ... ARGS ...
+             INCLUDEDIRS ... SLOW_FLAGS ... FAST_FLAGS ...)
+
+Lowercase and ... should be replaced with arguments, the uppercase parts delimit
+the arguments and can be passed in any order, or left out entirely if optional.

I think we should make the flag names close or identical to existing makefile arguments out of clarity.

+=item NAME

PREFIX?

+=item TOP

TOP_MODULE?

+Optional. Sets the name of the top module. Defaults to the name of the first
+file in the SOURCES array.
+=item ARGS

VERILATOR_FLAGS?

+=item INCLUDEDIRS

INCLUDE_DIRS?

but is this critical? Expect -f lists are the commoner case.

+=item SLOW_FLAGS

OPT_SLOW

+=item FAST_FLAGS

OPT_FAST

+++ b/examples/cmake

Example names should perhaps be cmake_c and python_c, since they assume C code generation.

Please add comment headers similar to the existing example files.

Call the written vcd file logs/vlt_dump.vcd to match tracing_c example.

"make examples" on the top directory fails as no Makefile is found - think you need one just to run your steps documented.

Following your instructions I get this:

CMake Error at CMakeLists.txt:11 (cmake_policy):
  Policy "CMP0074" is not known to this version of CMake.
+++ b/examples/cmake/.gitignore

Please also in gitignore include the files ignored in tracing_c/.gitignores.

Fix some missing trailing newline.

+++ b/examples/python/pyexample.cpp
+    //declare_globals declares the common parts of verilator (Verilated class and tracing classes)

Everywhere add one space after // or # comment chars.

+++ b/include/verilated_py.h
+#include "verilated.h" 
+#include <pybind11/pybind11.h>
+#ifdef VM_TRACE
+#ifdef VM_TRACE_FST
+#include "verilated_fst_c.h" 
+#else
+#include "verilated_vcd_c.h" 
+#endif
+#endif

Indent #if blocks appropriately "# ifdef" etc.

+namespace vl_py {

VerilatedPy

+inline void declare_globals(pybind11::module& m) {
+    m.def("VL_THREAD_ID", &VL_THREAD_ID);

Verilator doesn't currently require C++11, and this seems to. Perhaps it's ok to require C++ for this mode, but the test needs to be skipped when not configured for C++11.

+++ b/src/V3EmitCBase.h
-    AstCFile* newCFile(const string& filename, bool slow, bool source) {
+    static AstCFile* newCFile(const string& filename, bool slow, bool source) {

Indeed. Pushed this.

+++ b/src/V3EmitCMake.cpp
+static void cmake_set(std::ofstream& of, const string& name, const string& value,
+                        const string& cache_type = "", const string& docstring = "") {

Rather than pollute the global namespace, put these into your class.

+static string quote(const string& s) {

Quote is unused, remove.

+static string normalise(string s) {

Sorry, I'm American so "normalize". ;) Is non-windows happy with this?

+++ b/verilator-config.cmake
+#Check flag support. Skip on MSVC, these are all GCC flags.
+if (NOT (DEFINED VERILATOR_CFLAGS OR CMAKE_CXX_COMPILER_ID MATCHES MSVC))
+    include(CheckCXXCompilerFlag)
+    foreach (FLAG faligned-new fbracket-depth=4096 Qunused-arguments Wno-bool-operation Wno-parentheses-equality
+                  Wno-sign-compare Wno-uninitialized Wno-unused-but-set-variable Wno-unused-parameter
+                  Wno-unused-variable Wno-shadow)

I'm scared about having to maintain this code in two places (configure.ac and here). Not sure how to deal with that.

Put the -'s in front of these as much better when grepping and some compilers use +'s.

foreach (FLAG -faligned-new -fbracket-depth=4096 -Qunused-arguments ...

You don't seem to check for the appropriate multithreaded flags.

Need a test_regress/t/t_flag_cmake.pl test outside the examples. If your scenario line is right this will also test multithreaded.

#2 Updated by Patrick Stewart 5 months ago

That was quick. I'm actually off on holiday imminently, so won't get round to an updated patchset for a few weeks.

The cmake and python bits are already in separate commits, though the python one depends on the cmake one, the cmake one works alone. I can focus on the cmake one in this issue if you like.

Primarily I'm just punting this code out in the spirit of sharing. I would like to see it upstream, because it would make updating a lot easier for me, though it's completely understandable if you don't want to merge stuff you'll never use. As far as supporting it goes, I intend to keep it working for myself for the foreseeable future, and I might check on here occasionally or when I'm alerted, but I'm unlikely to add improvements that I don't need or respond to issues as quickly/comprehensively as you seem to.

I might well be in a minority of one with the python stuff, though I think it's pretty good for directly comparing a reference implementation of some DSP algorithm written with numpy etc to a verilog implementation. I've been doing it manually for a while, and got sick of writing the boilerplate, so decided to make these patches.

Also available in: Atom