Trying to differentiate between two simple clock divider circuits - VHDL

Question

I'm very new to VHDL and I'm trying to design a simple clock divider process, but I'm running into a strange disparity between two forms of a process that I can't tell apart logically. I'd really appreciate help with this! Sorry if I don't include enough context, I don't know yet what would be helpful for this sort of thing.

Form 1:

div_main : process(clk_in) is
variable count : integer := 0;
begin
    if (rising_edge(clk_in) and sw_in(3) = '1') then
        count := count + 1;
        if (count > div) then
            count := 0;
            tmpclk <= not tmpclk;
        end if;
    end if;
end process div_main;

Form 2:

div_main : process(clk_in) is
variable count : integer := 0;
begin
    if (rising_edge(clk_in) and sw_in(3) = '1') then
        count := count + 1;
    end if;
    if (count > div) then
        count := 0;
        tmpclk <= not tmpclk;
    end if;
end process div_main;

(The sw input is a form of clk enable) (The clk_in is a 125MHZ pin output on the FPGA)

The first form works perfectly, but I'm having a ton of issues with the second. It's tmpclk output is nearly random. The only difference I can see is whether the conditional reset of count is evaluated on only rising edges or on both rising and falling edges, which I wouldn't think matters. I would really appreciate if someone could explain why they behave so differently!

Answer 1

I'm not surprised that the second form gives you issues. Let's look at the code line-by-line.

First, we have the process declaration:

div_main : process(clk_in) is
    variable count : integer := 0;
begin

This tells the synthesizer/simulator to "trigger" the process any time clk_in changes. It also declares a count variable used internally. Note that this may be problematic in a synthesized design due to the fact that integer is not derived from std_logic like the signed and unsigned types defined in ieee.numeric_std (basically std_logic_vector with extra operator overloads). I would recommend changing the declaration to something like this:

    variable count : unsigned(15 downto 0) := (others => '0'); -- Set the size to whatever you need

Next, we have a synchronous statement triggered on the rising edge of the clock, along with a condition. count is incremented on every rising edge as long as sw_in(3) is asserted.

    if (rising_edge(clk_in) and sw_in(3) = '1') then
        count := count + 1;
    end if;

The problem lies in your next statement, which is a condition that is evaluated on both clock edges (no rising_edge(clk_in) if in if statement). Since the two if statements are not mutually exclusive, it is possible that count will be assigned twice in the same clock cycle.

    if (count > div) then
        count := 0;
        tmpclk <= not tmpclk;
    end if;
end process div_main;

If count were a signal, you might get more predictable behavior, but with a variable, it's difficult to say what will happen here. This article, which provides an excellent bit of insight into the difference between signals and variables.

The most important thing to understand (and the largest source of confusion) is that variables immediately take the value of their assignment, whereas signals depend on if the signal is used in combinational or sequential code.

To put it another way, a simulator may be able to assign the variable twice in the same process, but this would be extremely problematic to implement in hardware. You are essentially shorting two outputs together, which is certainly not what you want.

---

While the first form is better, you will still encounter a similar issue if you try to implement it in hardware. Here is an implementation that should work in hardware, though it may still be better to use a signal instead of a variable for your counter.

div_main : process(clk_in) is
    variable count : unsigned(15 downto 0) := (others => '0');
begin
    if rising_edge(clk_in) and sw_in(3) = '1') then
        -- Note that count is only assigned once per rising edge of clk_in
        if (count > div) then
            count := (other => '0');
            tmpclk <= not tmpclk;
        else
            count := count + 1;
        end if;
    end if;
end process div_main;