stnolting / neorv32

:desktop_computer: A small, customizable and extensible MCU-class 32-bit RISC-V soft-core CPU and microcontroller-like SoC written in platform-independent VHDL.
https://neorv32.org
BSD 3-Clause "New" or "Revised" License
1.59k stars 225 forks source link

Version management, maybe JSON-for-VHDL and/or pyVersioning #36

Closed umarcor closed 1 year ago

umarcor commented 3 years ago

While reviewing the latest changes, I found that the version is manually bumped in several places.

On the one hand, there is project JSON-for-VHDL, which allows writing (meta)data in a JSON file and then accessing it from VHDL. In fact, the JSON content can be passed as an string top-level generic, or as a file. It is synthesisable, so it can be used for setting parameters/generics in the design. However, it does not take any area. That is, it is not a JSON core. It's a library that is executed at compilation/synthesis time.

On the other hand, @Paebbels has pyVersioning for handling version management across multiple languages (VHDL, C, Python...). It is meant, precisely, for bumping the version in the HDL source, the C drivers, the docs, etc. all consistently.

In the mid term, it might be interesting to consider these or other similar alternatives.

stnolting commented 3 years ago

I found that the version is manually bumped in several places.

Right. I know it is bad practice. This whole "versioning concept" emerged in a very early stage of the project. Basically, it is just a counter that is incrementing with each new modification...

On the one hand, there is project JSON-for-VHDL, which allows writing (meta)data in a JSON file and then accessing it from VHDL. In fact, the JSON content can be passed as an string top-level generic, or as a file. It is synthesisable, so it can be used for setting parameters/generics in the design. However, it does not take any area. That is, it is not a JSON core. It's a library that is executed at compilation/synthesis time. ...

Look interesting - especially the JSON-for-VHDL concept. But I am not quite sure there. I have experimented with a simple bash script that extracts the version number from the VHDL package file and inserts it into the documentation. A very hacky and unpretty workaround - indeed. We should discuss this in #37 as the project lacks a real concept of straight versioning (shame on me!)... 😉

umarcor commented 3 years ago

Honestly, I don't think there is any direct solution. Even in the Python ecosystem, multiple different approaches are used. Hence, I'm good with any solution you pick. I believe the important issue is having the version defined in a single place, and/or generating it consistenly. #41 is an small step towards that. I agree that we should discuss #37 before implementing a solution across the repo.

JimLewis commented 3 years ago

VHDL-2019 gives a mechanism to put a date code that you can use as a revision indication. Long term - and perhaps longer for synthesis tools - the language will have a solution.

stnolting commented 3 years ago

While reviewing the latest changes, I found that the version is manually bumped in several places.

SemVer seems to be the way to go! -> #37

On the one hand, there is project JSON-for-VHDL, which allows writing (meta)data in a JSON file and then accessing it from VHDL. In fact, the JSON content can be passed as an string top-level generic, or as a file. It is synthesisable, so it can be used for setting parameters/generics in the design. However, it does not take any area. That is, it is not a JSON core. It's a library that is executed at compilation/synthesis time.

I have some issues with including files from the RTL files.... (related to #35 ) Let's say the version is defined in some global text (or whatever) file. This could be read using plain textio. But the problem here is the configuration of the path where to find the file. A global/static path is not flexible at all. Specifying no path leaves the default path to the mercy of the simulator or synthesis tool...

I don't know if this is primitive workaround or a good platform-independent solution, but how about using an additional VHDL package that contains the version? 🤔 This package file could be created by some simple script, but of course this is not really dynamic... 🤔 🤔

umarcor commented 3 years ago

The language (VHDL) provides features for solving those problems (both initialising mems from files or defining a version in some file/package). The point is whether the tools you want to use do support those language features. Generics and/or generic packages can be used for passing the path. It's just an string. For instance, VUnit defines the top-level generic tb_path, which allows reading/writing files relative to the location of the testbench (see https://github.com/VUnit/vunit/blob/master/examples/vhdl/array_axis_vcs/src/test/tb_axis_loop.vhd#L53). So, passing the revnumber through CLI (as done with asciidoc) might work.

Moreover, generating a VHDL package from another language/script, which contains the version as a constant, is an absolutely feasible solution. I'd say that's exactly what pyVersioning is about.

The JSON-for-VHDL solution is in-between. It allows providing multiple parameters (revnumber, init files, etc.) at once. Note that you can use JSON-for-VHDL in order to initialise some constants in a package, which you then use in the design. That is similar to "generating a VHDL package", but you generate JSON instead.

stnolting commented 3 years ago

The language (VHDL) provides features for solving those problems (both initialising mems from files or defining a version in some file/package). The point is whether the tools you want to use do support those language features. Generics and/or generic packages can be used for passing the path. It's just an string. For instance, VUnit defines the top-level generic tb_path, which allows reading/writing files relative to the location of the testbench (see https://github.com/VUnit/vunit/blob/master/examples/vhdl/array_axis_vcs/src/test/tb_axis_loop.vhd#L53). So, passing the revnumber through CLI (as done with asciidoc) might work.

Sound reasonable. But I see the problem, that a path defined by a generic might be lost when someone creates a setup from scratch (adding all VHDL files by hand / not using a provided script for project setup).

Moreover, generating a VHDL package from another language/script, which contains the version as a constant, is an absolutely feasible solution. I'd say that's exactly what pyVersioning is about.

I think this is more "stable" solution. From my point of view, the question is not really how to generate this package, but when to generate it. It should be auto-generated by some kind of script that is triggered by an abstract action. But what kind of "action"?? And who triggers that? I'm a little bit lost there. :thinking: :wink:

Paebbels commented 3 years ago

Hi. I created pyVersioning to be called e.g. in Vivado (synthesis) and Vivado SDK as pre-build steps.

In Vivado (synthesis), it's called as a pre-tcl hook for the main synthesis step.
In Vivado SDK, it's called as a pre-build step, so SDK writes a rule into the auto generated makefiles.

I suspect Intel, Lattice, ... are also providing such hooks.

So on every build, a file is generated including Git Hash, Git date, Git Time, Branch/Tag, repository URL (to identify forks), build date, build time, tool version, compile options.

In case of Vivado synthesis, we generate a text file, that is read via VHDL file I/O to describe the initial content of an AXI4-Lite register. We could also generate a constant, but if a modified file is part of the list of source files, Vivado would invalidate all generated bitstreams after each run. (It will see a modified source and out-date all outputs :(.

While a AXI4-Lite register at address 0x8000_0000 is good for an SoC (e.g. Xilinx MPSoC) a processor might need special CPU capability / CPU info registers to expose these information.


The idea for pyVersioning is to have one information collector (Git, yaml file, CI service) to write any source file as needed. E.g. on a CI server, you don't have a branch name or tag name, because on CI, the repository is checked out via hash. But, CI services are offering these information via environment variables (name depends on the used service). Anyhow, it would be a bummer to implement the rules for each language and service from scratch to collect all information you want to embed into a version register/string.

stnolting commented 3 years ago

Hi. I created pyVersioning to be called e.g. in Vivado (synthesis) and Vivado SDK as pre-build steps.

I will have a closer look.

In Vivado (synthesis), it's called as a pre-tcl hook for the main synthesis step. In Vivado SDK, it's called as a pre-build step, so SDK writes a rule into the auto generated makefiles.

So it is based on tcl only (toolchain agnostic)?

In case of Vivado synthesis, we generate a text file, that is read via VHDL file I/O to describe the initial content of an AXI4-Lite register.

As far as I understand, you are proposing to generate something like a "memory initialization file" (i.e. not a HDL source file!) that keeps the version information, correct? For the rtl part of the project, the actual version number (32-bit) seems to be sufficient - so an extra memory/hardware(!) module seems to be an overkill. However, it would be interesting to include the "full version" data into the software framework - somehow. 🤔

We could also generate a constant, but if a modified file is part of the list of source files, Vivado would invalidate all generated bitstreams after each run. (It will see a modified source and out-date all outputs :(.

I understand that this is not a good implementation practice, but to honest I would not mind. 😄 Furthermore, Vivado (and maybe other toolchains as well) provides incremental synthesis so only the parts that actually changed will get re-synthesized.

Paebbels commented 3 years ago

So it is based on tcl only (toolchain agnostic)?

The tool is written in Python an meant for any platform (not tested everywhere) and any language being called from any environment. As Vivado understands only TCL, we have a one-liner to call the Python script :).

As far as I understand, you are proposing to generate something like a "memory initialization file" (i.e. not a HDL source file!) that keeps the version information, correct?

In a first approach we generated the content of a VHDL constant in a package. Because the package is observed by Vivado for changes, Every build run, triggered the package generation and latest version information was integrated into the synthesis (e.g. build time). While Vivado used the latest content due to the pre-tcl hook, the GUI rechecked for modified files after finishing the implementation. Now it saw a modified file and it then invalidates the generated *.bit file. This is a tool bug of Vivado ...
Anyhow, it created an endless loop of builds and outdated output files.

By using a mem-init file, Vivado doesn't know it has changed, but the init-file is read on every build. So we tricked Vivado :).

For the rtl part of the project, the actual version number (32-bit) seems to be sufficient - so an extra memory/hardware(!) module seems to be an overkill.

Yes it might be enough to have a single register for a CPU.
But for an SoC I would encourage a complete register.

Furthermore, Vivado (and maybe other toolchains as well) provides incremental synthesis so only the parts that actually changed will get re-synthesized.

Incremental synthesis was and is a mess with Vivado...

umarcor commented 3 years ago

I believe that the register space at address 0x8000_0000 that @Paebbels mentioned is exactly the same as the SYSINFO in NEORV32.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

umarcor commented 3 years ago

Hmmm, @stnolting, I'm not sure it is good to have issues automatically marked as stale/wontfix and then closed. There is no rush for having issues solved, nor a problem of high traffic which needs aggressive filtering. Moreover, since this is an open source project with no funded developers, it is understandable that some features get fixed quick and others take longer. Therefore, I would recommend to avoid enabling an stale bot yet. Issues can be ordered by latest update (https://github.com/stnolting/neorv32/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc) so it's easy to see whether some is "too old".

stnolting commented 3 years ago

:smile: I completely forget that stale is watching this repository...

Hmmm, @stnolting, I'm not sure it is good to have issues automatically marked as stale/wontfix and then closed. There is no rush for having issues solved, nor a problem of high traffic which needs aggressive filtering. Moreover, since this is an open source project with no funded developers, it is understandable that some features get fixed quick and others take longer. Therefore, I would recommend to avoid enabling an stale bot yet. Issues can be ordered by latest update (https://github.com/stnolting/neorv32/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc) so it's easy to see whether some is "too old".

I totally agree :+1: