Description
I have ported the Xilinx XPM libary to VHDL in order to use it with GHDL. It works in Questasim, at least up to the point where I have tested it. In GHDL TYPES.INTERNAL_ERROR is raised. I don't know how to pinpoint the GHDL issue in this particular code.
Expected behaviour
The simulation should finish with the following output (questasim)
# ** Note: Test Completed Successfully
# Time: 93860 ns Iteration: 4 Instance: /xpm_fifo_tb
How to reproduce?
git clone https://gitlab.nikhef.nl/franss/xpm_vhdl.git
cd xpm_vhdl/script
compile_ghdl.sh
Context
versiontarball_urlcommit SHAIf a GHDL Bug occurred block is shown in the log, please paste it here:
******************** GHDL Bug occurred ***************************
Please report this bug on https://github.com/ghdl/ghdl/issues
GHDL release: 1.0-dev (v0.37.0-1070-ge18e9569) [Dunoon edition]
Compiled with GNAT Version: 10.2.0
Target: x86_64-linux-gnu
/home/frans/XPM_VHDL/script/
Command line:
ghdl -r --std=08 xpm_fifo_tb
Exception TYPES.INTERNAL_ERROR raised
Exception information:
raised TYPES.INTERNAL_ERROR : trans.adb:555
Call stack traceback locations:
0x555e32a1e1f1 0x555e32a1e5cf 0x555e32a1e84b 0x555e32baec36 0x555e32bbf802 0x555e32c1b8e0 0x555e32bf2fd8 0x555e32bf32b7 0x555e32bf30c4 0x555e32c3b320 0x555e32c3b447 0x555e32c3baa0 0x555e32c3c232 0x555e32c3de3e 0x555e32c3df93 0x555e32c413b7 0x555e32c45ca0 0x555e32c45ae3 0x555e32c45dda 0x555e32c09da2 0x555e32bfd754 0x555e32c0e634 0x555e32c08948 0x555e32c4e3fd 0x555e32b8db42 0x555e32ac7a4d 0x555e32c51d6f 0x555e32883af1 0x7f27944200b1 0x555e3288224c 0xfffffffffffffffe
******************************************************************
Additional context
Add any other context about the problem here. If applicable, add screenshots to help explain your problem.
I can reproduce the crash.
Thanks for looking into this. Is there any way I can help debugging this? I do write c/cpp and am usually good at firmware and software, but the hex numbers in the debug output don't mean anything to me yet.
@fransschreuder, there are some hints for debugging in https://ghdl.github.io/ghdl/development/Debugging.html
One way to find analysis bugs is to divide and conquer by manipulating the source file.
Here commenting out all the concurrent statements then re-exposing them in a sliding fashion from the top until all the bugs have shown up or all the concurrent statements analyze.
On line 668 of xpm_fifo_base.vhd:
wea => (others => ram_wr_en_i),
we'd find a dependence on an implicit signal for an actual expression that isn't currently supported by ghdl:
IEEE std 1076-2008 6.5.6.3 Port clauses:
If the actual part of a given association element for a formal port of a block is the reserved word inertial followed by an expression, or is an expression that is not globally static, then the given association element is equivalent to association of the port with an anonymous signal implicitly declared in the declarative region that immediately encloses the block. The signal has the same subtype as the formal port and is the target of an implicit concurrent signal assignment statement of the form
anonymous <= E;
where E is the expression in the actual part of the given association element. The concurrent signal assignment statement occurs in the same statement part as the block.
A work around would entail declaring a signal with a compatible subtype to the port actual in the immediately preceding block declarative region whose scope encloses the instantiation, assigning the expression to it in a concurrent assignment statement and using that signal as the actual. For troubleshooting purposes any valid and supported construct is fine. An actual of `(others => '0') will do for identifying more bugs without changing line numbering.
The next bug is another implicit signal on line 816:
port map(wrst_busy, wr_clk, (rd_pntr_ext-extra_words_fwft), rd_pntr_wr_dc);
(I stopped and searched the port maps and simply missed this one.)
These are the only two constructs causing INTERNAL_ERROR in xpm_fifo_base.vhd which analyzes successfully otherwise.
The immediate ghdl corrective action would be to add a message saying the creation of implicit signals for actual expressions isn't yet supported instead of reporting an INTERNAL_ERROR.
This is the first synthesis vendor's library code I recall seeing with this -2008 feature. It may alter the implementation priority from Tristan's perspective. It's been supported by at least two simulator vendors for several years and several third party library vendors have attempted to use it in the past.
I pigeonholed the original source and after finding the stack size too high disabled checking for running the fifo testbench in compile_ghdl.sh:
ghdl -r --std=08 xpm_fifo_tb --max-stack-alloc=0
And:
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../src/ieee2008/numeric_std-body.vhdl:1776:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../src/xpm/xpm_fifo/simulation/xpm_fifo_tb.vhd:91:10:@93860ns:(report note): Test Completed Successfully
simulation finished @93860ns
I used RCS (force of habit) to store the original. The changes are:
rcsdiff -b xpm_fifo_base.vhd
===================================================================
RCS file: RCS/xpm_fifo_base.vhd,v
retrieving revision 1.1
diff -b -r1.1 xpm_fifo_base.vhd
608a609,610
> signal wea: std_logic_vector (0 downto 0); -- ADDED
>
620a623,624
> wea <= (others => ram_wr_en_i); -- ADDED
>
668c672
< wea => (others => ram_wr_en_i),
---
> wea => wea, -- WAS (others => ram_wr_en_i),
801a806,807
> signal fifo_reg_in: std_logic_vector (RD_PNTR_WIDTH downto 0); --ADDED
> begin
813a820,821
> fifo_reg_in <= rd_pntr_ext-extra_words_fwft; -- ADDED
>
816c824
< port map(wrst_busy, wr_clk, (rd_pntr_ext-extra_words_fwft), rd_pntr_wr_dc);
---
> port map(wrst_busy, wr_clk, fifo_reg_in, rd_pntr_wr_dc); -- CHANGED
The testbench doesn't seem to be too thorough from the ability to tolerate mistakes. These changes should be carefully checked.
Dear @diogratia,
Thank you for the changes in the code, these 2 changes indeed fix the bug and the simulation now runs and completes successfully.
This is the first synthesis vendor's library code I recall seeing with this -2008 feature. It may alter the implementation priority from Tristan's perspective. It's been supported by at least two simulator vendors for several years and several third party library vendors have attempted to use it in the past.
This is not really a synthesis vendor's library, the xilinx xpm vendor's library is Verilog only. Since I plan to start using ghdl a verilog library wouldn't work, so I decided to translate the library into VHDL. The intention is to use it only for simulation, for synthesis the internal library in Vivado can still be used. I will add a proper readme file and then drop it somewhere on github.
Most helpful comment
One way to find analysis bugs is to divide and conquer by manipulating the source file.
Here commenting out all the concurrent statements then re-exposing them in a sliding fashion from the top until all the bugs have shown up or all the concurrent statements analyze.
On line 668 of xpm_fifo_base.vhd:
we'd find a dependence on an implicit signal for an actual expression that isn't currently supported by ghdl:
IEEE std 1076-2008 6.5.6.3 Port clauses:
A work around would entail declaring a signal with a compatible subtype to the port actual in the immediately preceding block declarative region whose scope encloses the instantiation, assigning the expression to it in a concurrent assignment statement and using that signal as the actual. For troubleshooting purposes any valid and supported construct is fine. An actual of `(others => '0') will do for identifying more bugs without changing line numbering.
The next bug is another implicit signal on line 816:
(I stopped and searched the port maps and simply missed this one.)
These are the only two constructs causing INTERNAL_ERROR in xpm_fifo_base.vhd which analyzes successfully otherwise.
The immediate ghdl corrective action would be to add a message saying the creation of implicit signals for actual expressions isn't yet supported instead of reporting an INTERNAL_ERROR.
This is the first synthesis vendor's library code I recall seeing with this -2008 feature. It may alter the implementation priority from Tristan's perspective. It's been supported by at least two simulator vendors for several years and several third party library vendors have attempted to use it in the past.
I pigeonholed the original source and after finding the stack size too high disabled checking for running the fifo testbench in compile_ghdl.sh:
And:
I used RCS (force of habit) to store the original. The changes are:
The testbench doesn't seem to be too thorough from the ability to tolerate mistakes. These changes should be carefully checked.