I have the following simplified example of my code, where the DeltasTest entity can be simulated to show the issue. The clock in the real design is inverted or not based on a generic, and feeds several other entities below this one.
The problem is that the simple edge detector does not work (data_out is just a glitch) in behavioral simulation, due to the delta cycle delay introduced on the clock by the inversion stage. Is there a standard or otherwise elegant way to solve this?
So far my best solution is to assign the data_in signal to another signal, to give it the same delta cycle delay as clk. I thought of using a function to invert the clock as necessary based on the generic as a second parameter to the function, but the clock is used in many places and this did not seem very elegant, and I was note sure it would even solve the problem. Clutching at straws, I also tried making the clock inversion assignment a transport assignment, but, as expected, this made no difference.
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
entity Deltas is
Generic (
CLK_INVERT : boolean := false
);
Port (
clk : in std_logic;
data_in : in std_logic
);
end Deltas;
architecture Behavioral of Deltas is
-- Signals
signal data_reg : std_logic := '0';
signal clk_inverted : std_logic := '0';
signal data_out : std_logic := '0';
begin
ClkInvert : if (CLK_INVERT) generate
clk_inverted <= not clk;
else generate
clk_inverted <= clk;
end generate;
process (clk_inverted)
begin
if (rising_edge(clk_inverted)) then
data_reg <= data_in;
end if;
end process;
process (data_reg, data_in)
begin
if (data_reg /= data_in) then
data_out <= '1';
else
data_out <= '0';
end if;
end process;
-- Other entities use `clk_inverted`. Commented out so that this example compiles
--LowerEntity : entity work.Counter
--port map (
-- clk => clk_inverted
--);
end Behavioral;
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
entity DeltasTest is
end DeltasTest;
architecture Behavioral of DeltasTest is
signal clk : std_logic := '0';
signal data_in : std_logic := '0';
begin
clk <= not clk after 10 ns;
process (clk)
variable count : integer := 0;
begin
if (rising_edge(clk)) then
count := count + 1;
if (count = 4) then
count := 0;
data_in <= not data_in;
end if;
end if;
end process;
uut : entity work.Deltas
Port map (
clk => clk,
data_in => data_in
);
end Behavioral;
I think your "solution" of an additional delta cycle on data_in is probably the cleanest simple solution.
Semantically, the assignment to clk_inverted could translate into hardware as an inverter in the clock signal, thus the delta cycle delay in simulation represents a (possibly(*) real) race condition introduced in the real hardware.
Thus the simulation behaviour is not merely an annoyance but a useful property, warning you of a potential deviation from good synchronous design practice that must be attended to.
(*) Only "possibly real" because, depending on the technology, synthesis may "push" the inversion into the downstream FF by detecting the opposite edge instead - automatically converting your design per Morten's comment.
It becomes more of a problem when the additional delta cycle is buried in somebody else's IP, perhaps a model of an external component such as memory. The IP is then no longer under your control, and a board level simulation may read or write data in the wrong clock cycle.
Here, my answer is to add approximate I/O delays, either as inertial delays in the FPGA's I/O assignments (from the FPGA datasheet or synthesis report), or as transport delays on the PCB. This ensures my simulations approximate teh behaviour of the board - not perfectly, but closely enough to allow me to design the internal logic following normal synchronous practice.
A cleaner solution would be to generate two different clocked processes based on the generic. This eliminates the pesky delta-cycle altogether.
FINALLY! a use for the 2-process form of SM, where keeping the clocked processes as simple as possible has a benefit ...
not so fast...
The behaviour you want to refactor is clock edge detection, which is already refactored into a simple function from the obsolete and complex (level and event) expression.
Why not refactor the clock edge detection further, for example...
function clock_edge(signal clk : in std_logic) return boolean is
begin
if CLK_INVERT then
return falling_edge(clk);
else
return rising_edge(clk);
end if;
end clock_edge;
...
process (my_clock) is
begin
if clock_edge(my_clk) then ...
I have not tested this to see if synthesis tools can implement this as desired. Some probably will, some may not (if they simply special-cased the system-provided functions instead of doing the job properly)
In a hierarchical design, this function would be in a package, used by any entities that needed it. This poses problems of the visibility of the generic : one suggestion would be to use a constant in the package instead, another would be to pass the generic as a second parameter to the function. The right approach probably depends on restrictions imposed by the synthesis tool.