Add feature to add custom constants that are put in registers (similar to GIT registers)

[elialm]

It's very nice to get external information such as git status in a register, but I'd like an extension of this feature where one can add any arbitrary externally sourced information in registers.

An example use-case: I have a project where I keep version information in a .yml file since other scripts require this. In the .tcl script, one can extract this version number and put it in this config record. No need for us to copy this value in the RTL. Then one project might have this version in a .json, one in an .ini, one in a .toml, etc, etc. I saw that you guys had put in a bunch of useful stuff in the register interface (e.g. build time, vivado version), but it would be nice if this config file would be more easily extendible and reached. Also since everyone has their wants and needs, some need something more light to include in legacy project.

As to how to implement, one could do something similar to preSynth_GitVersionRegister_Vivado.tcl to put data in a sort of config file, then have code similar to mem_GitVersionRegister.pkg.vhdl to read this out. Personally, I'd make it into a record type of some sorts. One can then put this record in a package and access this accross the entire project. No need to wire stuff. And would then easily integrate with the contant registers you guys already have.

[stefanunrein]

Hi @elialm. Thanks for submitting this feature-request. I see it as useful as you and will try to implement it in the next days.

[stefanunrein]

@elialm please check out #29

The output port contains all information of the register in a sorted way. Is this enough for your legacy project?

We are thinking of a way to create the mem-file in yml format. This will be probable inside the next major release.

[elialm]

Hi @stefanunrein, I see you got to work. This is not exactly like how I envisioned no. Yes, the version information is accessible to the project, but this requires that the project instantiate AXI4Lite_GitVersionRegister, so it's not as small as I'd like. In our case, the project already has a AXI Lite slave, I only need the version information. I could then also opt to just instantiate the entity and leave the AXI Lite bus unconnected, but that's not ideal. Not sure how much this would then also contribute to elaboration time that will eventually be tossed away.

I think, to make it very small and easily integrated, I would enjoy having all the stuff defined in the mem (or yaml file in the future), accessible to RTL. So something like this in a package:

constant c_config_git_info : t_config_dict := config_read_file("/path/to/file/above");

Then anywhere I need a specific config that is defined in that config file, I can just do something like so:

constant c_info_git_hash         : std_logic_vector(c_git_hash_length - 1 downto 0) := config_get_hslv(c_config_git_info, "git_hash", c_git_hash_length);
constant c_info_dirty_modified   : std_logic                                        := config_get_logic(c_config_git_info, "dirty_modified");
constant c_info_dirty_untracked  : std_logic                                        := config_get_logic(c_config_git_info, "dirty_untracked");

This would then be easy to use, and would also decouple the logic between the implementation of the slave registers and populating those registers (which is quite big, as it has to parse a file and a lot can go wrong there).

Additionally, this PR does not address the need for a generic config file which is accessible to the RTL at compile time. Every project has different needs and build system, so a way for any pre-script (be it tcl, python, bash, you name it) to export stuff into a config file that the RTL can read is not there. What's your opinion on that?

Also, I would agree on going to the yaml route. It bring a bit more structure to the config file, would be especially awesome if the aforementioned feature would be a thing. Otherwise, I don't really see the need since it would add to the already long elaboration times. I wouldn't go for anything too complex, something simple like .ini might already be good enough.

[stefanunrein]

Ah, now I got you.
The idea was, that with the entity you have all features available (like UID). But it is no problem to create a wrapper function to create directly a constant.
I have done that with the latest commit. You can use it with:

constant My_Version : T_Version_Register := to_Version_Register(MEM_PATH);

You can then navigate through the records. Do you think it is really necessary to have search strings like config_get_logic(c_config_git_info, "dirty_modified");?

Your code would look like this now:

	constant My_Version              : T_Version_Register    := to_Version_Register(PROJECT_DIR & VERSION_FILE_NAME);
	constant c_info_git_hash         : std_logic_vector      := My_Version.Top.GitHash;
	constant c_info_dirty_modified   : std_logic             := My_Version.Top.Version_DirtyModified;
	constant c_info_dirty_untracked  : std_logic             := My_Version.Top.Version_DirtyUntracked;
	constant c_info_ToolVersion_Year : unsigned              := unsigned(My_Version.Common.ToolVersion_Year);

[elialm]

HI @stefanunrein. This is already much leaner of a solution, thank you. Something like this looks very easy to integrate in something like a legacy project.

The magic of the search path comes in with generic requirements. Git and version information might be something that we all would benefit from. But who knows, maybe something wants to put some address in that config file, or a reference number to some other product, or an encryption key, or some random environment variable, or the git information of a submodule, you name it. Instead of predicting what a developer might need, I think itś better to put the screwdriver in the hands of that developer, if you catch my drift. That's the original intent of my feature request.

The way I would then see it, is to be able to extend the preSynth_GitVersionRegister_Vivado.tcl script to export your own constants to put in the record. Something like:

set product_ref $::env("PRODUCT_REF")
export_constant "product_ref" $product_ref

or getting information from submodules:

set current_dir $pwd
cd $submodule_dir
set submodule_commit_hash [exec git rev-parse HEAD]
cd $current_dir

export_constant "submodule_commit_hash" $submodule_commit_hash

[stefanunrein]

In theory this idea is really good :), in practice, this makes tools really slow. There is something like this available: https://github.com/Paebbels/JSON-for-VHDL

Reading in a simple JSON file in synthesis in Vivado takes already multiple minutes, each query as well.

For now, to keep the usage simple and to be faster, we will stay with the record-based approach. If the tools support string operations better/faster, we can change it in the future.

[elialm]

Yeah, I don't see the JSON based approach working well if it takes minutes to parse it. But I'm not arguing that, just as simple way to extend the underlying .mem file. I actually have an implementation of this which I don't think runs all that slow. It parses a key/value paired file like so:

git_hash 1d010b1
dirty_modified 1
dirty_untracked 1

Extending it like so allows the code to search for the key dynamically without having to change anything in the parsing code. The parser creates a dictionary (t_config_dict) that is literally a list of records containing the keys and values as string. The functions that then access these strings, will convert it to the correct datatype (e.g. config_get_logic or config_get_hslv).

Timewise, the addition to the .tcl script is negligible. For elaboration, I don't think this adds too much. I can see if I can run a benchmark on this if you're interested?

[Paebbels]

The problem is string handling with Vivado. It looks there is a huge memory leak and performance penalty. We saw this effect before, but it improved a bit in past years so we can handle register descriptions and some simple files (line by line reading).

The problem with VHDL before 2019, you can't go back in file reading, so everything must be processed in a single pass reading operation, which means when comparing/searching for strings, it involves loops and string operations.

I'll update my very very old project (JSON-for-VHDL) and file it as a bug report and to AMD soon. We're just updating it to latest version. I talked to some other compiler developers and it might be a bigger problem then just a memory leak ...

[elialm]

Ai, that sounds like very deep rooted issues. Do you know if it's specifically the file IO or the string operations? Cuz if it's just file IO, one can read the entire file in one go, and then take the time to read/parse the file as you go (like what I am doing). If not, then we'll have to wait for AMD to sort this out.

What do you then want to do with this PR? Cuz I think we both like the proposal, but it's something that can't be implemented then.

I'm also very interested in the communications with AMD, could you keep me updated? It's that allowing for this kind of file IO and string stuff can be very powerful for these kinds of automations.