1
\$\begingroup\$

I have a counter called lastelem_reg. At the rising edge of each clock, it should check whether another signal enqueue is HIGH. If it is, then lastelem_reg will be incremented by 1 in the next clock, otherwise it continues to hold its old value. This seems like a very simple problem, but I'm not getting the intended behavior. Here's the VHDL code:

    process(clk,reset)
    begin
        if (reset='1') then
            lastelem_reg    <= (others=>'0');
        elsif (rising_edge(clk)) then
            lastelem_reg    <= lastelem_next;
        end if;
    end process;
    
    
    process(lastelem_reg)
    begin
        if (enqueue = '1') then
            lastelem_next <= lastelem_reg + 1;
        else
            lastelem_next <= lastelem_reg;
        end if;
    
    end process;

Simulating the above code, results in the waveform shown below. We see that lastelem_reg never gets updated. enter image description here

So, I thought let's add enqueue to the sensitivity list of the second process and see what we get. The waveform is shown below. We see two things happening here that not supposed to happen. First, the lastelem_next signal changes twice every clock cycle, which shouldn't be happening. Second, the lastelem_reg is updating 1 clock before it should. For example, at 10ns, when the first enqueue signal comes, lastelem_reg should be 0, but here it is 1.

Any idea how to overcome this issue ? enter image description here

\$\endgroup\$
  • \$\begingroup\$ There's nothing to overcome. Putting enqueue in the second sensitivity list is correct. There's nothing wrong with lastelem_next changing like that, and lastelem_reg is updating on every clock, because enqueue is asserted on every clock. \$\endgroup\$ – Dave Tweed Jul 4 '20 at 23:50
  • \$\begingroup\$ Thanks for the reply. I could probably live with fact that lastelem_next is changing twice every clock cycle, but this leads to incorrect behavior as lastelem_reg is expected to start counting from 0. As you see, it starts counting from 1 here. This is because it is reacting to changes in enqueue before the clock comes, which is not correct behavior. \$\endgroup\$ – Chad Winters Jul 4 '20 at 23:55
  • \$\begingroup\$ It IS starting at zero -- that's the value it has right up to the first clock edge. And enqueue is high at that clock edge, so it increments. If you want something different, you need to drive the inputs differently. Or if you want it to increment TO zero, then you need to reset it to all-ones (effectively -1). \$\endgroup\$ – Dave Tweed Jul 4 '20 at 23:57
  • \$\begingroup\$ By starting at 0, I meant that lastelem_reg should be 0 during the first clock pulse (with enqueue HIGH). It is only in the second clock that it should update to 1. But as you can see, lastelem_reg starts at 1 from the get go. This is problematic for me because I have a bunch of other modules that work based on the fact that lastelem_reg is 0 during the first clock (with enqueue HIGH). If I can overcome the issue here, then I wouldn't have to modify the other codes to adapt to this new behavior. \$\endgroup\$ – Chad Winters Jul 5 '20 at 0:24
  • \$\begingroup\$ Then like I said before, reset lastelem_reg to all ones (... (others => '1')). Then the first enqueue operation will increment it to zero. On the other hand, if you don't want it to increment at all on that first clock edge, you must not assert enqueue there. \$\endgroup\$ – Dave Tweed Jul 5 '20 at 0:33
2
\$\begingroup\$

Forget using combinational processes with complicated and error-prone sensitivity lists, until the once-in-a-blue-moon time you actually need them.

Especially when you move on to state machines; but even here, a single synchronous process is cleaner and shorter, as well as much easier to get right. This reproduces the code you have (with the correction to the sensitivity list)

process(clk,reset)
begin
    if reset='1' then
        lastelement     <= (others=>'0');
    elsif rising_edge(clk) then
        if enqueue = '1' then
            lastelement <= lastelement + 1;
        end if;
    end if;
end process;

There is one additional wrinkle in your problem specification however:

At the rising edge of each clock, it should check whether another signal enqueue is HIGH. If it is, then lastelem_reg will be incremented by 1 in the next clock, otherwise it continues to hold its old value.

This needs an additional single cycle delay:

process(clk,reset)
begin
    if reset='1' then
        lastelement     <= (others=>'0');
        lastelement_d1  <= (others=>'0');
    elsif rising_edge(clk) then
        if enqueue = '1' then
            lastelement <= lastelement + 1;
        end if;
        lastelement_d1  <= lastelement;
    end if;
end process;

and I think lastelement_d1 (lastelement delayed 1 cycle) is what you are looking for.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.