vunit
vunit copied to clipboard
Rework codec
Linked with issue #818
The proposed modifications on the codec package are retro-compatible. The tb_codec.vhd has not been modified to underline this. The tb of the com package tb_com_codec.vhd has not been touched either.
The tests compile and pass for data_type and com package with Modelsim 2020.1. I will try with GHDL soon.
Here is a list of changes (non-exhaustive):
- Add comments into the code which explains the purpose of this package and how each type is encoded.
- Removed hard coded value in the code
- Replace the type 'string' used to encode by an alias named 'code_t' to ease readability and maintainbility.
- Inverse the
functionand aliasnamesto ease readability, maintainbility and easectrl+F:
becomesfunction encode(constant data : std_ulogic_vector) return string; function decode(constant code : string) return std_ulogic_vector; alias encode_std_ulogic_vector is encode[std_ulogic_vector return string]; alias decode_std_ulogic_vector is decode[string return std_ulogic_vector];function encode_std_ulogic_vector(data : std_ulogic_vector) return string; function decode_std_ulogic_vector(code : string) return std_ulogic_vector; alias encode is encode_std_ulogic_vector[std_ulogic_vector return string]; alias decode is decode_std_ulogic_vector[string return std_ulogic_vector]; - The code length for integer automatically adapts to the width of the integer (1993/2002/2008: 32bits, 2019: 64bits). (Not tested with VHDL-2019).
- Added clearly which function should be used by a VUnit user vs a VUnit advanced user vs a VUnit developer.
- Functions
encode_array_headerandget_rangeare replaced (these names are not consistent with the other functions):function encode_range(range_left : integer; range_right : integer; is_ascending : boolean) return code_vec_t;function decode_range(code : code_vec_t) return range_t;encode_array_headerandget_rangeare deprecated but preserved for backward compatibility.encode_array_headerandget_rangetake the encoded string ofrange_left,range_rightandis_ascendingwhileencode_rangeanddecode_rangedirectly take integers and a boolean.- Note that
encode_array_headercan encode two ranges but it was never used for that purpose (anywhere in the VUnit repo). Theencode_rangecan only encode a single range.
- Replace the
ieee.numeric_std.unsignedbyieee.numeric_std.unresolved_unsigned - Replace the
ieee.numeric_std.signedbyieee.numeric_std.unresolved_signed - Simplify the local
log2function used to encode therealtype by using theieee.math_realpackage. The same idea could be applied tostring, but I did not see the use case so it was not implemented. - For each encoded type which is a
scalar, anenumerated typeor arecord type, there is a function namedfunction code_length_**type_name** return naturalwhich indicate how many characters are needed to encode a value of the selected type. - For each encoded type which is an
array type, there is a function namedfunction code_length_**type_name**(length : natural) return natural;which indicate how many character are needed to encode a value of the selected type. - The type
std_ulogic_arrayis similar to thestd_ulogic_vectorbut with an integer range. However, it was not the case for bit_vector. There is now a typebit_arrayfor that purpose. - The function
to_byte_arrayandfrom_byte_arrayare replaced (these names are not consistent with the other functions and the introduction ofbit_arrayrequired a change ofbit_vectortobit_array):function encode_raw_bit_array(data : bit_array) return code_t;function decode_raw_bit_array(code : code_t) return bit_array;to_byte_arrayandfrom_byte_arrayare deprecated but preserved for backward compatibility.
- Create a new package
common_pkgwhich contains constants/fonction used by thecodecpackage but not specific to it like:-- Retrieve the integer width of the simulator function get_simulator_integer_width return positive; - Added a page in the online documentation which shows the declaration package of the
codec.vhdandcodec-2008p.vhdfiles. - Updated the python code to generate the encode/decode functions for user-specified type.
@std-max pointed that unresolved_unsigned does not exists in VHDL-93. I lauched test only with VHDL 2008. I will run some more tests in VHDL 93 and correct this (and eventually other mistakes of the same nature)
I have lauched all tests inside the vunit/vhdl sub-directories with Modelsim 2020.1 and GHDL 3.0.0-dev (v2.0.0-101-g791ff0c1). The tests have been perfomed with VHDL-93 (when possible) and VHDL-2008.
All tests pass in all configurations (GHDL vs Modelsim and 93 vs 2008).
Note: I made slight modifications because GHDL raises an error when casting a null range of type bit_array(-1 downto 0) to bit_vector. Error: 'bound check failure' Example:
procedure decode_bit_vector(constant code : in code_t; variable index : inout code_index_t; variable result : out bit_vector) is
variable ret_val : bit_array(result'range);
begin
decode_bit_array(code, index, ret_val);
result := bit_vector(ret_val); -- ERROR 'bound check failure' with GDHL but not with modelsim
end procedure;
This code has been transformed into:
procedure decode_bit_vector(constant code : in code_t; variable index : inout code_index_t; variable result : out bit_vector) is
variable ret_val : bit_array(result'range);
begin
if ret_val'length /= 0 then -- GHDL work-around
decode_bit_array(code, index, ret_val);
result := bit_vector(ret_val);
else
result := "";
end if;
end procedure;
LRM: VHDL-2008: '5.2.1 General'
A constraint is compatible with a subtype if each bound of the range belongs to the subtype or if the range constraint defines a null range. Otherwise, the range constraint is not compatible with the subtype.
So I guess this is a GHDL mistake. I will link the corresponding issue from the GHDL repo when created.
@LarsAsplund ? @umarcor ?
After 8b8c2db I can run all tests in ModelSim but GHDL fails compilation of codec_builder without error message. Riviera-PRO and Active-HDL compile (after changing the non-positive code_t range and adding the in mode) but they fail simulation with an internal error which I can't decrypt. I will have to do some bisecting of the changes to find the root cause. I guess you can't get hold of Riviera-PRO and Active-HDL but I recommend testing with free GHDL
The commits 4e7fd60bd76cf3255aec8417c5649ffb61298f9a and 7515a1f52f1ed4577a385b2bc07cf720d7dd7ee5 update the sources so that it compiles with GHDL (GHDL 3.0.0-dev (v2.0.0-101-g791ff0c1)). There is a comment about that earlier: https://github.com/VUnit/vunit/pull/822#issuecomment-1114030144.
I tested with Modelsim the commit 8b8c2db0ad43a33f4113063a1ab0278543f88787 but no other tool.
And I tested with Modelsim and GHDL starting commit e2de513df289ab8c79a7271a48b6a55bae893525 (see comment https://github.com/VUnit/vunit/pull/822#issuecomment-1114030144).