feat: add design version info at runtime

[cebarobot]

Recently, we have added version info of the design code (the commit id) in the XiangShan RTL code. This patch adds the version info in the emu/simv program. Now the emu/simv program will print the commit id of design when initializing.

This patch may also resolve the issue where the emu build time would not update when re-compiling. Now, with each build, version.h is re-generated, causing common.cpp, the source code file which contains printing logic, to be recompiled, which subsequently triggers a relink of emu. This process takes a few seconds, which is longer than before, but still acceptable.

[poemonsense]

Version strings are valuable. Some suggestions to improve the code.

[poemonsense]

1. Do we need to check whether git exists?

2. I think we can simply pass them as a C macro.

SIM_CXXFLAGS += -DDUT_VERSION_STRING=\\\"${VERSION_COMMIT_ID}${VERSION_IS_DIRTY}\\\"

3. Maybe we can also add the DIFFTEST_VERSION_STRING.

[cebarobot]

2. I think we can simply pass them as a C macro.

SIM_CXXFLAGS += -DDUT_VERSION_STRING=\\\"${VERSION_COMMIT_ID}${VERSION_IS_DIRTY}\\\"

This won't work, for two reasons.

• Verilator writes all FLAGS into VSimTop.mk, which won't be update as long as verilator is not triggered again.
• The original source code file common.cpp (and its timestamp) is never changed. In that way, the compiling system make will think the compiled result common.o is always up-to-date and no need to be re-compiled. Hence the commit id will always keep the old one when the whole project is compiled for the first time.

[cebarobot]

Check git is a good proposal. I'm considering to genenrate the commit info string in the design repo and pass it to difftest.

[cebarobot]

Difftest commit info is also valuable, but I think it's better to:

• add all submodules commit info

difftest: asdfqwe
yunsuan: qwerasd
...

• or, record detail dirty info by git status --porcelain, since the main repo commit has included the submodule commit info.

 M difftest

Which one do you think is better?

[poemonsense]

• Verilator writes all FLAGS into VSimTop.mk, which won't be update as long as verilator is not triggered again.

This is what we want as this dut version should be the same as the generated verilog, not the git repo.

Consider this case: make emu is generated with version A. We checkout it to version B. If we enforce a make emu again, the timestamp will be changed to version B. This is not what we want.

The DUT version should be exactly matching the verilog code, which is passed at verilator build time instead of the current git commit.

[poemonsense]

• add all submodules commit info

difftest is a general co-sim framework for RISC-V processors, not only for XiangShan.

The reason we use git here is for the general-purpose usage. If XiangShan, or other DUTs, want a more complicated version string, they should pass the string by command line DUT_VERSION_STRING=XXX.

[poemonsense]

There is no need to generate this header. See below comments for common.cpp

[poemonsense]

4. There's no need to add one more line for the version string. We can simply add them to the elf_name.

Info("%s(" DUT_VERSION_STRING ", " DIFFTEST_VERSION_STRING ") compiled at " __DATE__ ", __TIME__ " \n", elf_name);

[poemonsense]

Since we are passing versions with C macros, there's no need to creating the headers here.

[poemonsense]

This patch may also resolve the issue where the emu build time would not update when re-compiling.

I don't fully understand this issue. What is the case that the build time is not updated? I think when C++ files (or other dependencies) change, it is rebuilt with build time updated. Otherwise, make emu will avoid rebuild the project.

[cebarobot]

This patch may also resolve the issue where the emu build time would not update when re-compiling.

I don't fully understand this issue. What is the case that the build time is not updated? I think when C++ files (or other dependencies) change, it is rebuilt with build time updated. Otherwise, make emu will avoid rebuild the project.

TL;NR: the timestamp-based tool (make) and the hash-based tool (ccache) both thinks the souce code common.cpp containing __DATE__ and __TIME__ didn't change and refused to re-compile it.

Although the value of the marco __DATE__ and __TIME__ changes when building, but the common.cpp file and its timestamp is never changed after cloning, causing the building system make to always think the compiled result common.o is always up-to-date and there is no need to recompile common.cpp. So the common.o always keeps the old time when the whole project is built for the first time. The common.cpp does not depend on any other source files but the headers included.

Acctually I'm not confident with this fix. There is also some hash-based cache of souce file (maybe from ccache), which will still influence the accurancy of __DATE__ and __TIME__. It seems that the ccache is not in preprocessor mode. For example,

1. try make emu and ./build/emu gets time 111 and commit asdf.
2. commit.
3. try make emu and ./build/emu gets time 222 and commit qwer.
4. revert the commit.
5. try make emu and ./build/emu gets time 111 and commit asdf again.

Now, I acctually want to write the date and time into the version.h file.

[poemonsense]

TL;NR: the timestamp-based tool (make) and the hash-based tool (ccache) both thinks the souce code common.cpp containing __DATE__ and __TIME__ didn't change and refused to re-compile it.

When common.cpp is changed, the dependency will cause a rebuild.

Acctually I'm not confident with this fix. There is also some hash-based cache of souce file (maybe from ccache), which will still influence the accurancy of __DATE__ and __TIME__. It seems that the ccache is not in preprocessor mode. For example,

1. try make emu and ./build/emu gets time 111 and commit asdf.
2. commit.
3. try make emu and ./build/emu gets time 222 and commit qwer.
4. revert the commit.
5. try make emu and ./build/emu gets time 111 and commit asdf again.

Now, I acctually want to write the date and time into the version.h file.

This is unacceptable because every time of make emu will cause a rebuild for C++ files and the emu. This behavior does not address any serious issue but brings significant overhead. 1) This rebuild does not bring any useful information. Why do we need a new timestamp? What's the timestamp for? 2) This violates the dependencies and causes a rebuild at any time.

Besides, we have said a lot of times to avoid using ccache. It has been removed from dependencies since 2022 (OpenXiangShan/xs-env@a28ca6c). The reason is also that, using ccache does not address any serious issue, but it will bring significant overhead.

[poemonsense]

For difftest, as a cosim framework, we are only considering the general-purpose usage.

A version tag is welcomed, and we can add it to the emu build message. We can also add the git commit info as the default message.

But that's all we will do in this difftest PR.

1. difftest will not complicate the problem by introducing a specialized version tag scheme for XiangShan. If XiangShan, or the DUT projects want a complex version, please pass it by the command line. For example, you may pass an env variable, which can be updated at build time, similar as how PGO works.

This is the core reason I'm rejecting the verison.h. It does not address any serious issue, but it will bring significant overhead. There're many other ways to address your concern on maintaining a correct version tag, but I think you are choosing a bad one.

I want to note that, what difftest accepts as the DUT is the Verilog files, not any git repo or Chisel code. The reason we are supporting the git commit info as the default option is to support the general-purpose usage. That's also why we need a git checker. This fundamental design choice to separate the DUT and the cosim framework will reject some of your proposals, including __DATE__, messages for submodules, version headers, ...

2. discussing the __DATE__ and __TIME__ macros is out of the context of this PR. If you are changing their build behaviors, please raise a new PR or issue. We can discuss about them then.

[cebarobot]

Acctually, I have tried the following methods:

1. Passing version info by C marco through SIM_CXXFLAGS & -D.

   • It won't work.
   • Verilator stores the flags into VSimTop.mk. According to make dependency, VSimTop.mk is regenerated by verilator only when Verilog files change. $(EMU_MK): $(SIM_TOP_V) | $(EMU_DEPS)
   • If we add the commit id of difftest, when the C++ files from difftest change and the difftest repo gets a new commit, VSimTop.mk will not be regenerated and keeps the old flags. I don't think that is what we want.
   • (This is also the reason why PGO has to using an unusual way to pass its parameter.)

2. Passing version info dynamically just like PGO.

   • It won't work either, and it is very similar to the __DATE__ and __TIME__ issue.
   • The common.cpp is only compiled once during the first compilation of the emu. After the first compilation, the common.cpp file itself and its timestamp is never updated, even if the marco value has changed. Therefore make thinks common.o is always up-to-date and never re-compile it. As a result, the common.o as well as emu always keeps the marco value of the first compilation.
   • I will open another issue to discuss this problem detailly.

3. Base on 2, just touch common.cpp to enforce the recompilation of common.cpp

   • Ugly. And it won't work as there is ccache.
   • Regardless of whether ccache is beneficial or not, it's already installed on our machines, and Verilator is configured to use it by default. Also, other users may follow the verilator document to install ccache. I didn't find any performance data of ccache and I will design an experiment to test it.
   • Although the timestamp of common.cpp file changes but its hash does not change. So ccache just uses the previous compilation result and the emu keeps the marco value of the first compilation.

4. Writing version info into an header file

   • It works.
   • This solve the tree issue before: makefile store the flag, file timestamp dependency, file hash not change.
   • This does introduce some overhead, but since it's under 10 seconds, I consider it acceptable. The additional steps are limited to the following: generating version.h, recompiling common.cpp, and linking emu.

5. Put an magic string into common.cpp and use awk to replace it in the final ELF file.

   • Tooooooooo ugly.

Do you have any suggestion? I will keep trying to find the best way to solve this problem.

I understand that difftest is an general-purpose cosim framework. The complex version info would be genetated by the DUT project, but I suggest that the difftest may give space for such requirement, as it can't be achieved with just the DUT project alone.

[poemonsense]

I understand that difftest is an general-purpose cosim framework. The complex version info would be genetated by the DUT project, but I suggest that the difftest may give space for such requirement, as it can't be achieved with just the DUT project alone.

I think simply passing the version tag (along with the verilog source files) when make emu works perfectly.

As you mentioned, this version tag macro will be written in the VSimTop.mk when Verilator verilates the design. I think this behavior is exactly what we want for the version tag -- it should be with the verilog files instead of the current git commit of any repo.

For simplicity, we can use the current git commit when make emu is called as the default macro value. However, we still reserve the possibility for DUTs to pass their own tag strings.

How do you think of this? Is there any issue where this implementation won't work?

• It won't work either, and it is very similar to the __DATE__ and __TIME__ issue.
• The common.cpp is only compiled once during the first compilation of the emu. After the first compilation, the common.cpp file itself and its timestamp is never updated, even if the marco value has changed. Therefore make thinks common.o is always up-to-date and never re-compile it. As a result, the common.o as well as emu always keeps the marco value of the first compilation.
• I will open another issue to discuss this problem detailly.

Yes. Much appreciate we can discuss this issue separately -- whether the timestamps should be updated (with common.cpp rebuilt) even if there aren't any change to the difftest C++ files.

[cebarobot]

I think simply passing the version tag (along with the verilog source files) when make emu works perfectly.

I think there is still two problems:

1. The version tag includes a dirty tag, which may be missed if the difftest cpp source code is changed. In this case, cpp souce code modification just not triggers the verilator to regenerate the VSimTop.v.
2. There is still the old issue: common.cpp won't be recompiled by make, because it is 'up-to-date'.

By the way, I don't get the gap between the version of verilog files and the version of the repo. The verilog files are just from the repo or generated from the repo.

[poemonsense]

I think simply passing the version tag (along with the verilog source files) when make emu works perfectly.

I think there is still two problems:

1. The version tag includes a dirty tag, which may be missed if the difftest cpp source code is changed. In this case, cpp souce code modification just not triggers the verilator to regenerate the VSimTop.v.

Yes. This is what we want: Verilator only re-verilates the design only when SimTop.v changes. Otherwise, only changed dependencies (C++ files) are re-built. What do you mean by "missed"?

2. There is still the old issue: common.cpp won't be recompiled by make, because it is 'up-to-date'.

Yes. We don't want to recompile it (because the RTL is not changed).

By the way, I don't get the gap between the version of verilog files and the version of the repo. The verilog files are just from the repo or generated from the repo.

I see. Maybe we are not clear for this.

Basically, the build process for emu is as follows:

1. DUT generates the verilog SimTop.v (difftest does not do anything at this step)

2. Verilation: DiffTest Makefile accepts the command make emu and calls Verilator to create the VSimTop.mk (with all macros fixed at this step)

3. C++ compilation: DiffTest then compiles all C++ files, including the Verilated C++ files and DiffTest C++ files

• Here's (as I understand) how the version.h works:

Every time we are at step 3, verison.h is generated with the version tag equal to HEAD. This ensures that tags are always up-to-date (the version of the repo).

Specifically, this HEAD will be the up-to-date commit of the DUT directory. DiffTest does not know whether this HEAD matches the Verilated RTL design at step 2.

• Here's how I prefer to

Every time we are at step 2, DiffTest accepts a version tag of the DUT (the version of verilog files) and writes it to the VSimTop.mk. This ensures that, this tag remains unchanged even if the DUT directory has any updates.

Does this make sense to you?

[cebarobot]

Ok, I understand the gap between you and me.

There is two version tag, for DUT and for difftest.

• For DUT version tag, now I agree with you: Writing it to VSimTop.mk makes sense.
• For difftest version tag, writing it to VSimTop.mk make no sense, because difftest source code (those c++ files) modification does not trigger verilator to rebuild VSimTop.mk.

2. There is still the old issue: common.cpp won't be recompiled by make, because it is 'up-to-date'.

Yes. We don't want to recompile it (because the RTL is not changed).

For this problem, I'm writing an new issue. I will explain it there.

[poemonsense]

Ok, I understand the gap between you and me.

There is two version tag, for DUT and for difftest.

• For DUT version tag, now I agree with you: Writing it to VSimTop.mk makes sense.

Yes.

• For difftest version tag, writing it to VSimTop.mk make no sense, because difftest source code (those c++ files) modification does not trigger verilator to rebuild VSimTop.mk.

Agree. I'm not sure how we can force re-compiling common.cpp every time when any other C++ file changes. This seems difficult.

If we succeed doing this, the __DATE__ issue will be resolved as well, because it will be updated once recompiled.

Considering that this case (change difftest versions after make emu) seldom happens, maybe we can simply pass the static version tag for now, before we find an appropriate way to do it?

[cebarobot]

Agree. I'm not sure how we can force re-compiling common.cpp every time when any other C++ file changes. This seems difficult.

If we succeed doing this, the __DATE__ issue will be resolved as well, because it will be updated once recompiled.

Considering that this case (change difftest versions after make emu) seldom happens, maybe we can simply pass the static version tag for now, before we find an appropriate way to do it?

That's why I introduced the version.h. version.h is included by common.cpp so verilator will add it to the dependency of common.cpp automatically, therefore common.cpp will be re-compiled.

[poemonsense]

Agree. I'm not sure how we can force re-compiling common.cpp every time when any other C++ file changes. This seems difficult.
If we succeed doing this, the __DATE__ issue will be resolved as well, because it will be updated once recompiled.
Considering that this case (change difftest versions after make emu) seldom happens, maybe we can simply pass the static version tag for now, before we find an appropriate way to do it?

That's why I introduced the version.h. version.h is included by common.cpp so verilator will add it to the dependency of common.cpp automatically, therefore common.cpp will be re-compiled.

This seems an approach with high overhead bringing minor improvements. I think DUT version tags are much more valuable than DiffTest tags. Maybe we can only implement the DUT version tag now in this PR.

Frankly speaking, for advanced users, they should know that they are updating the DiffTest repo. For beginner users, they should never checking out the difftest repo on their own.

Technically, I propose we find better methods before implementing the real DiffTest tags. version.h looks like a hacky workaround that breaks the original intention of dependencies. Like winning 5 points here but losing other 10 points elsewhere?