32-bit counter and test bench How could this VHDL counter and its test bench be improved?  I am interested in anything you see that could be done better, but especially in the test bench:


Is wait for 10 ns better or worse than any other time delay?
The test is very minimal.  Should it do more?


counter32.vhdl:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity counter32 is
  port (
    clk: in std_ulogic;
    ena: in std_ulogic;
    rst: in std_ulogic;
    q  : out std_ulogic_vector(31 downto 0));
end entity;

architecture rtl of counter32 is

  signal count : unsigned(31 downto 0);

begin

  process(clk, rst)
  begin
    if rst = '1' then
      count &lt;= X"00000000";
    elsif rising_edge(clk) then
      if ena = '1' then
        count &lt;= count + 1;
      end if;
    end if;
  end process;

  q &lt;= std_ulogic_vector(count);

end architecture;


counter32_tb.vhdl:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity counter32_tb is
end counter32_tb;

architecture tb of counter32_tb is

  component counter32
    port (
      clk: in std_ulogic;
      ena: in std_ulogic;
      rst: in std_ulogic;
      q  : out std_ulogic_vector(31 downto 0));
  end component;

  signal clk: std_ulogic;
  signal ena: std_ulogic;
  signal rst: std_ulogic;
  signal q  : std_ulogic_vector(31 downto 0);

begin

  dut : counter32 port map (
    clk =&gt; clk,
    ena =&gt; ena,
    rst =&gt; rst,
    q =&gt; q);

  process
  begin

    -- Reset
    clk &lt;= '0';
    ena &lt;= '1';
    rst &lt;= '1';
    wait for 10 ns;
    rst &lt;= '0';
    wait for 10 ns;

    -- Counts on clock leading edge if enabled
    clk &lt;= '1';
    wait for 10 ns;
    assert q = x"00000001";

    -- Does not count on clock trailing edge
    clk &lt;= '0';
    wait for 10 ns;
    assert q = x"00000001";

    -- Does not count when not enabled
    ena &lt;= '0';
    clk &lt;= '1';
    wait for 10 ns;
    assert q = x"00000001";

    -- Clears on reset
    rst &lt;= '1';
    wait for 10 ns;
    assert q = x"00000000";

    wait;

  end process;

end tb;


Shell script to run the test bench in GHDL:

OPTS=--workdir=build
ghdl -a ${OPTS} *.vhdl
ghdl -e ${OPTS} counter32_tb
ghdl -r ${OPTS} counter32_tb --vcd=counter32.vcd
Nice with some HDL on Code Review. I'll contribute with my opinions. All-in-all, I find the code all well-written.
Opinions not already covered in previous answers

A more standard way of setting count at reset would be (others =&gt; '0').

That is, as opposed to hard-coding a bit/hex literal - even if you for some reason hard-code the width as you have done, (others =&gt; '0') is more standard in this case.


You might have a good reason for asynchronous reset, but if not, synchronous reset is a better habit.

You often see these asynchronous resets, it is unfortunately a more common habit, since that's what is (or was) most often taught.
See my motivations here: http://www.fpga-dev.com/resets-make-them-synchronous-and-local/



Opinions already covered/partly covered
Somewhat in order of importance (in my subjective opinion).

Define the roll-over behaviour.

The behaviour is actually defined for an unsigned in numeric_std: any carry bits will be ignored so the unsigned will roll-over. However, neither simulators nor synthesis tools should be trusted on following this. (And even less, humans reading the code.) Also; it is good to really think about what behaviour you desire, or is the most stable: roll-over or saturate? I'll take the chance to give two comments on this choice:

For synthesis, roll-over rather than saturation will yield less logic and improved timing. The tool will be able to use a counter macro right-off.
If reaching max count is expected never to happen, add an assertion so a simulation will flag in that case. Then still code the RTL behaviour explicitly to roll-over (less logic, better timing) if there aren't stability reasons to code it to saturate.




Parameterize the width. Put it in a constant or generic.

Generate the clock separately (e.g. clk &lt;= not(clk) after 10 ns;, and make sure to init clk to '1'.

This is more standard and more readable.
Then, when coding your stimuli, do not dead-reckon the time for stimuli assignments, instead, use wait until rising_edge(clk); etc. This makes code more stable, and gives deterministic and proper times for the stimuli changes (not only in regards to time, but also to simulation deltas, which is equally important).


Use entity instantiation

More details and motivation: http://www.fpga-dev.com/leaner-vhdl-with-entity-instantiation/



Simulation performance

There's nothing wrong in coding the RTL of the counter like this, but it could be noted that there is a simulation performance gain possible: Make count a variable, local to the process, do tests and arithmetics on this variable, and in the end of the process drive the variable to a counter signal (defined in the architecture).

Others

Thumbs up for self-checking test benches.