[Bug] Riviera-PRO component binding not working between libraries #1073
Comments
|
Generally speaking we recommend that all third-party IPs are compiled according to the vendor instructions. They are typically encrypted so VUnit can't analyze them for compile order anyway. Even if you can, there is no point for VUnit to spend time analyzing a possible large set of file that rarely change. Once you have the library compiled you can add it to the project with add_external_library. Is that what you do already? |
|
The problem happens due to |
|
On further digging it seems that if something is instantiated through |
|
Since VUnit + TSFPGA is a common approach, I'm sure there is a solution. I've not been working with Xilinx for quite some time but maybe @LukasVik knows what's going on |
|
I did some printing and I see that the order here is like And in the actual architecture rtl of abcd_cdc is
component abcd_cdc_fifo
port (
rst : in std_logic;
wr_clk : in std_logic;
rd_clk : in std_logic;
din : in std_logic_vector(31 downto 0);
wr_en : in std_logic;
rd_en : in std_logic;
dout : out std_logic_vector(31 downto 0);
full : out std_logic;
empty : out std_logic
);
end component;
signal empty, full : std_logic;
begin
cdc_fifo_i : abcd_cdc_fifo
port map (
rst => not source_reset_n,
-- Ingress
wr_clk => source_clk,
din => source_data,
wr_en => source_valid and source_ready,
full => full,
-- Egress
rd_clk => target_clk,
dout => target_data,
rd_en => target_valid and target_ready,
empty => empty
);
source_ready <= source_reset_n and not full;
target_valid <= target_reset_n and not empty;
end architecture rtl; |
|
Since binding happens at elaboration the important thing is that the library has been compiled before the simulation starts. Unfortunately, I haven't used TSFPGA enough to guide you. The tool adds an abstraction layer on-top of VUnit so I think you should validate your setup with that project first. |
|
Well I use Xilinx daily but I haven't used IP cores for the last five years. So the knowledge of how to do these things is not really top of mind. But let's see. @SzymonHitachi Could you share the content of the file |
|
Also, if you could add trace printouts of the |
|
Since this is a bigger project, I have filtered out all of the unrelated files and kept
|
|
|
I also have the final compile order from $ git diff -- vunit/sim_if/__init__.py
diff --git a/vunit/sim_if/__init__.py b/vunit/sim_if/__init__.py
index a1d84d8e..2a336935 100644
--- a/vunit/sim_if/__init__.py
+++ b/vunit/sim_if/__init__.py
@@ -272,6 +272,12 @@ class SimulatorInterface(object): # pylint: disable=too-many-public-methods
source_files_to_skip = set()
+ print("<<< source_files to compile")
+ for sf in source_files:
+ print(sf.name)
+ print("<<< end")
+ exit(-5)
+
max_library_name = 0
max_source_file_name = 0
if source_files:and the filtered output is |
|
Somehow the dependencies between |
|
To add: |
|
@LarsAsplund I have traced it back to if implementation_dependencies:
add_dependencies(self._find_component_design_unit_dependencies, vhdl_files)I see the same later on in When i set it to |
|
@SzymonHitachi Thank you for all the info and your thorough investigation. To begin with, a quick fix that will most likely make it work is to set the |
|
This part of the code is a little under-documented. I am partially the author of it so I guess I am to blame. But, I'll try to explain. Line 16 in 95f5959 A typical (small) compile order might look like this: This is Line 64 in 95f5959 If This was the default behavior for quite a while. the If we are running a So given the example above, if we simulate some code that instantiates the component/entity of the FIFO IP, VUnit parser would detect that the FIFO entity is in the file This can save some time in e.g. CI in large projects with a lot of IP cores. HOWEVER: If the scenario is the reverse, the code we're simulating uses So if the IP that we are |
|
@SzymonHitachi If you confirm that |
|
But the actual root issue is still somewhere in VUnit. Disabling It seems that VUnit does not realize that this module in module abcd_cdc_fifo (
rst,
wr_clk,
rd_clk,
din,
wr_en,
rd_en,
dout,
full,
empty
);
input wire rst;
(* X_INTERFACE_PARAMETER = "XIL_INTERFACENAME write_clk, FREQ_HZ 100000000, FREQ_TOLERANCE_HZ 0, PHASE 0.0, INSERT_VIP 0" *)
(* X_INTERFACE_INFO = "xilinx.com:signal:clock:1.0 write_clk CLK" *)
input wire wr_clk;
(* X_INTERFACE_PARAMETER = "XIL_INTERFACENAME read_clk, FREQ_HZ 100000000, FREQ_TOLERANCE_HZ 0, PHASE 0.0, INSERT_VIP 0" *)
(* X_INTERFACE_INFO = "xilinx.com:signal:clock:1.0 read_clk CLK" *)
input wire rd_clk;
(* X_INTERFACE_INFO = "xilinx.com:interface:fifo_write:1.0 FIFO_WRITE WR_DATA" *)
input wire [31 : 0] din;
(* X_INTERFACE_INFO = "xilinx.com:interface:fifo_write:1.0 FIFO_WRITE WR_EN" *)
input wire wr_en;
(* X_INTERFACE_INFO = "xilinx.com:interface:fifo_read:1.0 FIFO_READ RD_EN" *)
input wire rd_en;
(* X_INTERFACE_INFO = "xilinx.com:interface:fifo_read:1.0 FIFO_READ RD_DATA" *)
output wire [31 : 0] dout;
(* X_INTERFACE_INFO = "xilinx.com:interface:fifo_write:1.0 FIFO_WRITE FULL" *)
output wire full;
(* X_INTERFACE_INFO = "xilinx.com:interface:fifo_read:1.0 FIFO_READ EMPTY" *)
output wire empty;is the same as this component in architecture rtl of abcd_cdc is
component abcd_cdc_fifo
port (
rst : in std_logic;
wr_clk : in std_logic;
rd_clk : in std_logic;
din : in std_logic_vector(31 downto 0);
wr_en : in std_logic;
rd_en : in std_logic;
dout : out std_logic_vector(31 downto 0);
full : out std_logic;
empty : out std_logic
);
end component;
beginI guess this is a bug/limitation in the Verilog parser. I don't write much Verilog personally, but the code looks a little weird. The |
|
@SzymonHitachi PS; I thougth your company is using hdl-modules also? A FIFO in pure VHDL is a lot nicer than an IP core. Should use this: https://hdl-modules.com/modules/fifo/fifo.html#asynchronous-fifo-vhd |
|
Happy new year! I finally am back and did dive into the topic. After more debugging I can confirm that for my file so it might be around the lines of #1073 (comment) |
|
The verilog IP file seems to be parsed properly. I guess the issue might come from the fact that My guess here is that experimenting with set_property default_lib myproject [current_project]Could yield some improvements. |
|
I might be wrong but I really don't think this is the issue. If you look into the generated |
|
I don't think the concept of an IP core belonging to a library exists. And it should work either way since it is a component instantiation. If you use entity instantiation (which I would personally), the library is |
|
Yeah they end up in Is there any way to force the IP project VUnit compilation before rest of VUnit? Any way to really control the order other that adding manual dependency on the specific generated files? |
Again, I really don't think this is the issue. If I still think the issue is this: #1073 (comment)
Yes this is what I suggested here: #1073 (comment) |
|
Yes I did try it. It did not work. While it changed the order of operations, the IPs (yes instantiated as components) are still not placed before the files instantiating them. Currently I'm trying to force a dependency of my whole main project library on |
|
Few additional notes:
So it seems we have 2 different overlapping issues here:
|
Using
tsfpgaassume that I have 1 VHDL filemyfile.vhdand 1 IP TCL file withmycomponent.xil_defaultliblibraryDuring the simulation now, In Riviera-PRO, simulator does not bind the generated IP properly to the component:
Solution discussion:
This seems to be easily fixed by updating
vunit/sim_if/rivierapro.py->_create_load_functionby adding:which is added in modelsim also.
This however results in significant increase of simulation time (increasing with the amount of tcl IPs) on start of elaboration with warnings for each of the IPs:
Note: Same warning does not show up for modelsim.
What would be the actual good way of solving this issue?
The text was updated successfully, but these errors were encountered: