OpenXiangShan / difftest

Modern co-simulation framework for RISC-V CPUs
Mulan Permissive Software License, Version 2
120 stars 68 forks source link

[BUG] `make difftest-so` is broken. #519

Open Tang-Haojin opened 2 days ago

Tang-Haojin commented 2 days ago

Related component: build

Describe the bug At XiangShan repository. When I run make sim-verilog && cd difftest && make difftest-so, it reports an error:

/nfs-nvme/home/tanghaojin/kmhv2_release/_work/XiangShan/difftest/src/test/csrc/vcs/vcs_main.cpp:29:10: fatal error: svdpi.h: No such file or directory
   29 | #include "svdpi.h"
      |          ^~~~~~~~~
compilation terminated.
make: *** [libso.mk:17: difftest-so] Error 1

Seems that this may be related to https://github.com/OpenXiangShan/difftest/pull/506.

To Reproduce Steps to reproduce the behavior:

git clone https://github.com/OpenXiangShan/XiangShan.git XS-difftest-dir
cd XS-difftest-dir
export NOOP_HOME=`pwd`
make init
make sim-verilog
cd difftest
make difftest-so

Expected behavior difftest-so are generated successfully.

Screenshots

image

What you expect us (DiffTest developers) to do for you

Additional context Add any other context about the problem here.

poemonsense commented 1 day ago

1) @xiaokamikami How can we link the svdpi library? Where is it located?

1) There's a missing CI test suggested in https://github.com/OpenXiangShan/difftest/pull/353

@forever043 Could you please help describe the usage of difftest-so and add a test for it? I'm not sure about how we are going to use it. Also, we probably need a README description for this.

poemonsense commented 1 day ago

Given that the current CI only tests NutShell (with a lot of macros disabled), I think we need a) generating profile for XiangShan with some make command, and b) using the profile to test difftest, including building the so.

@Tang-Haojin For a), is there any available command? We only need Chisel elaboration to generate the profile, no need for invoking CIRCT. For b), is there any required test in addition to the difftest-so?

I think the ultimate goal for us is to automatically create official releases for XiangShan and DiffTest in CI. This completes our workflow and ensures its functional correctness.

forever043 commented 1 day ago
  1. @xiaokamikami How can we link the svdpi library? Where is it located?
  2. There's a missing CI test suggested in Make: add support for compiling difftest as a lib.so #353

@forever043 Could you please help describe the usage of difftest-so and add a test for it? I'm not sure about how we are going to use it. Also, we probably need a README description for this.

we use the libdifftest.so generated by difftest-so target in our in-house VCS/PLDM envrionments, it act as a standalone difftest DPI library. A basic compile CI job should be enough for this target, but it needs ~20min chisel elab job to generate the required headers.

Tang-Haojin commented 1 day ago

For a), is there any available command? We only need Chisel elaboration to generate the profile, no need for invoking CIRCT.

I will add a make argument to do this now.

For b), is there any required test in addition to the difftest-so?

I cannot come up with any additional test right now. If we need something else more than just make difftest-so, I will file a new PR later.

poemonsense commented 1 day ago

we use the libdifftest.so generated by difftest-so target in our in-house VCS/PLDM envrionments, it act as a standalone difftest DPI library. A basic compile CI job should be enough for this target, but it needs ~20min chisel elab job to generate the required headers.

I'll add the difftest-so compile job after this is fixed.

Chisel elaboration, which only elaborates the design without FIRRTL transforms and CIRCT runs, should take less time. I'll work with Haojin on this. Hopefully we can find a way to testing it.

Tang-Haojin commented 1 day ago

A relatively simple way is to eliminate CIRCT pass: https://github.com/OpenXiangShan/XiangShan/pull/3974. It takes less than 3 minutes (Scala compile time is not included) to run make sim-verilog CHISEL_TARGET=chirrtl on XiangShan CI server, but I do not know if it is acceptable on GitHub-hosted free servers.

poemonsense commented 1 day ago

I'm drafting a PR for testing it. Let's see what will happen.

forever043 commented 1 day ago

A relatively simple way is to eliminate CIRCT pass: OpenXiangShan/XiangShan#3974. It takes less than 3 minutes (Scala compile time is not included) to run make sim-verilog CHISEL_TARGET=chirrtl on XiangShan CI server, but I do not know if it is acceptable on GitHub-hosted free servers.

The chirrtl stage need ~30GB memory, which may exceed the quota of free servers :(

poemonsense commented 1 day ago

A relatively simple way is to eliminate CIRCT pass: OpenXiangShan/XiangShan#3974. It takes less than 3 minutes (Scala compile time is not included) to run make sim-verilog CHISEL_TARGET=chirrtl on XiangShan CI server, but I do not know if it is acceptable on GitHub-hosted free servers.

The chirrtl stage need ~30GB memory, which may exceed the quota of free servers :(

It does fail now.

If we are testing it using the predefined profile, is it mostly the same as building from XiangShan?

forever043 commented 1 day ago

If we are testing it using the predefined profile, is it mostly the same as building from XiangShan?

Is there any place other than Release where we can stage some CI generated files?

poemonsense commented 1 day ago

If we are testing it using the predefined profile, is it mostly the same as building from XiangShan?

Is there any place other than Release where we can stage some CI generated files?

CI supports uploading some artifacts. But I'm not sure it is appropriate for us to depend on CI builds instead of official releases. Depending on CI may cause circular dependence issues. For example, XiangShan depends on DiffTest but DiffTest depends on XiangShan

Tang-Haojin commented 1 day ago

A relatively simple way is to eliminate CIRCT pass: OpenXiangShan/XiangShan#3974. It takes less than 3 minutes (Scala compile time is not included) to run make sim-verilog CHISEL_TARGET=chirrtl on XiangShan CI server, but I do not know if it is acceptable on GitHub-hosted free servers.

The chirrtl stage need ~30GB memory, which may exceed the quota of free servers :(

10GB Xmx is enough, and Github-hosted free machine has 16GB RAM. I will try to add another argument to modify -Xmx in build.sc. Maybe we can retry later.

Tang-Haojin commented 1 day ago

After merging https://github.com/OpenXiangShan/XiangShan/pull/3974 and https://github.com/OpenXiangShan/XiangShan/pull/3975, we can try make sim-verilog CHISEL_TARGET=chirrtl JVM_XMX=10G.

Tang-Haojin commented 1 day ago

Time and memory consumption:

image
xiaokamikami commented 1 day ago
  1. @xiaokamikami How can we link the svdpi library? Where is it located?
  2. There's a missing CI test suggested in Make: add support for compiling difftest as a lib.so #353

@forever043 Could you please help describe the usage of difftest-so and add a test for it? I'm not sure about how we are going to use it. Also, we probably need a README description for this.

we use the libdifftest.so generated by difftest-so target in our in-house VCS/PLDM envrionments, it act as a standalone difftest DPI library. A basic compile CI job should be enough for this target, but it needs ~20min chisel elab job to generate the required headers.

If you want to use sv_dpiv.h, you must first initialize the VCS or palldium environment variables

xiaokamikami commented 1 day ago

Another option is to emulation the vcs using emu, as is done in CI, which will generate code that does not depend on sv_dpi

poemonsense commented 1 day ago
  1. @xiaokamikami How can we link the svdpi library? Where is it located?
  2. There's a missing CI test suggested in Make: add support for compiling difftest as a lib.so #353

@forever043 Could you please help describe the usage of difftest-so and add a test for it? I'm not sure about how we are going to use it. Also, we probably need a README description for this.

we use the libdifftest.so generated by difftest-so target in our in-house VCS/PLDM envrionments, it act as a standalone difftest DPI library. A basic compile CI job should be enough for this target, but it needs ~20min chisel elab job to generate the required headers.

If you want to use sv_dpiv.h, you must first initialize the VCS or palldium environment variables

Then what's the use case to compile it? @forever043 @Tang-Haojin Should we compile them in VCS env and use them as well in VCS env? If so, CI can hardly test it.

Tang-Haojin commented 1 day ago
  1. @xiaokamikami How can we link the svdpi library? Where is it located?
  2. There's a missing CI test suggested in Make: add support for compiling difftest as a lib.so #353

@forever043 Could you please help describe the usage of difftest-so and add a test for it? I'm not sure about how we are going to use it. Also, we probably need a README description for this.

we use the libdifftest.so generated by difftest-so target in our in-house VCS/PLDM envrionments, it act as a standalone difftest DPI library. A basic compile CI job should be enough for this target, but it needs ~20min chisel elab job to generate the required headers.

If you want to use sv_dpiv.h, you must first initialize the VCS or palldium environment variables

Then what's the use case to compile it? @forever043 @Tang-Haojin Should we compile them in VCS env and use them as well in VCS env? If so, CI can hardly test it.

Previously, I can compile it on verilator environment. @cailuoshan Is this expected or I just happened to successfully compile it with verilator?

xiaokamikami commented 1 day ago
  1. @xiaokamikami How can we link the svdpi library? Where is it located?
  2. There's a missing CI test suggested in Make: add support for compiling difftest as a lib.so #353

@forever043 Could you please help describe the usage of difftest-so and add a test for it? I'm not sure about how we are going to use it. Also, we probably need a README description for this.

we use the libdifftest.so generated by difftest-so target in our in-house VCS/PLDM envrionments, it act as a standalone difftest DPI library. A basic compile CI job should be enough for this target, but it needs ~20min chisel elab job to generate the required headers.

If you want to use sv_dpiv.h, you must first initialize the VCS or palldium environment variables

Then what's the use case to compile it? @forever043 @Tang-Haojin Should we compile them in VCS env and use them as well in VCS env? If so, CI can hardly test it.

Previously, I can compile it on verilator environment. @cailuoshan Is this expected or I just happened to successfully compile it with verilator?

If use make emu, difftest won't generate code that requires sv_dpi.h