Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

Fix FL_PIPE REM signal width when DATA_WIDTH is 0 or 8 #5

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

pavelekm
Copy link
Contributor

@pavelekm pavelekm commented Feb 9, 2024

FL_PIPE has some unexpected behavior with low DATA_WIDTH values:

 - for DATA_WIDTH = 32, REM is abs(log2(32 / 8) - 1) = 1 downto 0
 - for DATA_WIDTH = 16, REM is abs(log2(16 / 8) - 1) = 0 downto 0
 - for DATA_WIDTH =  8, REM is abs(log2( 8 / 8) - 1) = 1 downto 0
 - for DATA_WIDTH =  0, REM is abs(log2( 0 / 8) - 1) = ? downto 0

For 32 and 16 bits this works correctly. For 8 bits, (1 downto 0) makes no sense, since FL has just one byte that is always valid – it does not need a 2-bit REM. It does not need REM at all. When DATA_WIDTH = 0, the result depends on how systhesis evaluates log2(0); in Vivado it evaluates to 0, so the result is again (1 downto 0).

This proposed fix replaces the bogus ranges for 8 and 0 data widths with null range (-1 downto 0), so that, when not needed, the REM signal becomes zero width and is simply left unused. The REM_WIDTH function previously used to compensate for the weird REM port behavior is replaced by a constant. Unnecessary includes of non-standard numeric packages are also removed.

While Vivado happily ignores null-range signals, I'm not 100% sure if it is acceptable in other toolchains like Modelsim or Quartus. So you may want to test it and consider a different solution if it causes any problems. I suppose that only applies to the DATA_WIDTH = 8 case, since when data width is zero, there are already null ranges in the data ports.

@jakubcabal jakubcabal merged commit 26b1c54 into CESNET:main Jul 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants