VHDL expert puzzle

[Jan Decaluwe]

In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&

--
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Python as a HDL: http://www.myhdl.org
VHDL development, the modern way: http://www.sigasi.com
World-class digital design: http://www.easics.com

 

[Nicolas Matringe]

Le 25/11/2012 20:11, Jan Decaluwe a écrit :

In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&

The feedback term is 0 when the LFSR is all 0s so it won't count because

it resets to all 0s.

Nicolas

 

[Nicolas Matringe]

Le 25/11/2012 20:49, Nicolas Matringe a écrit :

Le 25/11/2012 20:11, Jan Decaluwe a écrit :
In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&


The feedback term is 0 when the LFSR is all 0s so it won't count because
it resets to all 0s.

Or maybe not... What's that 'xnor' operator ? I've never used it before.

Nicolas

 

[Alan Fitch]

On 25/11/12 19:49, Nicolas Matringe wrote:

Le 25/11/2012 20:11, Jan Decaluwe a écrit :
In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&

The feedback term is 0 when the LFSR is all 0s so it won't count because
it resets to all 0s.

Nicolas

I don't think so - with XOR gates you mustn't use all zeros, but he's

used XNOR, so you mustn't use all '1's.

regards
Alan

--
Alan Fitch

 

[Alan Fitch]

On 25/11/12 19:11, Jan Decaluwe wrote:

In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&

I guess it also depends what he means by flaw. The only signal required
in the sensitivity list is clk; and if he'd made LFSR type unsigned, he
wouldn't have needed the type conversions.

But I guess that doesn't affect functionality (which I assume is what is
meant by a flaw). Perhaps I should read the second page of the post...

Of course it may just be that the feedback taps on the LFSR are wrong
for a maximal length sequence, but I can't be bothered to look that up

Alan

--
Alan Fitch

 

[Jan Decaluwe]

On 11/25/2012 09:02 PM, Alan Fitch wrote:

On 25/11/12 19:11, Jan Decaluwe wrote:
In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&


I guess it also depends what he means by flaw. The only signal required
in the sensitivity list is clk; and if he'd made LFSR type unsigned, he
wouldn't have needed the type conversions.

But I guess that doesn't affect functionality (which I assume is what is
meant by a flaw).

Correct. The superfluous signals in the sensitivity list are
messy, but they don't affect behavior.

Perhaps I should read the second page of the post...

I think you may find it interesting.

Of course it may just be that the feedback taps on the LFSR are wrong
for a maximal length sequence, but I can't be bothered to look that up

Of course not. Again, he claims that an experienced designer
would see it immediately.

Jan

--
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Python as a HDL: http://www.myhdl.org
VHDL development, the modern way: http://www.sigasi.com
World-class digital design: http://www.easics.com

 

[Alan Fitch]

On 25/11/12 20:13, Jan Decaluwe wrote:

On 11/25/2012 09:02 PM, Alan Fitch wrote:
On 25/11/12 19:11, Jan Decaluwe wrote:
In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&


I guess it also depends what he means by flaw. The only signal required
in the sensitivity list is clk; and if he'd made LFSR type unsigned, he
wouldn't have needed the type conversions.

But I guess that doesn't affect functionality (which I assume is what is
meant by a flaw).

Correct. The superfluous signals in the sensitivity list are
messy, but they don't affect behavior.

Perhaps I should read the second page of the post...

I think you may find it interesting.

OK I cracked and read it.

I don't really understand what's going on, I would have to simulate it
(though of course you've already done that Jan!).

I agree there's something fishy about adding output delays. What
difference would that make in this case?

Of course it may just be that the feedback taps on the LFSR are wrong
for a maximal length sequence, but I can't be bothered to look that up

By moving the assignment to d0 into the process, he has implied another
flip-flop, which makes me think that my guess that the feedback taps are
wrong is correct - though there's no way an experienced designer could
spot that (unless they'd memorized that table in XAPP 210 - I've
memorized the app note number, but not the table ).

Of course not. Again, he claims that an experienced designer
would see it immediately.

Mysteriouser and mysteriouser...

Alan

Jan

--
Alan Fitch

 

[rickman]

On 11/25/2012 2:11 PM, Jan Decaluwe wrote:

In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&

I see two things I would flag. One is the fact that they include
extraneous signal names in the sensitivity list. This can cause a
simulation to run slowly, but I've never seen a synthesizer to create
different logic because of it.

The other is the omission of a reset assignment for the signal clk_out
which is registered. This means the reset will simply disable the
clk_out FF from changing state when rst is asserted, but otherwise the
circuit will function normally.

I don't see an error that will stop the design from working.

The use of XNOR gates in the feedback means the all 1s state will latch
up while the use of XOR gates in the feedback means the all 0s state
will latch up. The XNOR is just an XOR with the output inverted which
is the same as either one of the inputs inverted or all three inverted.
Thinking of it as all three inverted clearly shows the symmetry of the
latch up states.

Rick

 

[rickman]

On 11/25/2012 3:27 PM, rickman wrote:

On 11/25/2012 2:11 PM, Jan Decaluwe wrote:
In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&



I see two things I would flag. One is the fact that they include
extraneous signal names in the sensitivity list. This can cause a
simulation to run slowly, but I've never seen a synthesizer to create
different logic because of it.

The other is the omission of a reset assignment for the signal clk_out
which is registered. This means the reset will simply disable the
clk_out FF from changing state when rst is asserted, but otherwise the
circuit will function normally.

I don't see an error that will stop the design from working.

The use of XNOR gates in the feedback means the all 1s state will latch
up while the use of XOR gates in the feedback means the all 0s state
will latch up. The XNOR is just an XOR with the output inverted which is
the same as either one of the inputs inverted or all three inverted.
Thinking of it as all three inverted clearly shows the symmetry of the
latch up states.

Rick

Looking at Page 2 of the article I don't see what he is going on about.
I don't think he knows either... The first response in his blog is
trying to grapple with what seems to be an error in his analysis and his
response is...

"Oh i dunno, its far too early in the morning ;-)

anyway the point is it didn't work and now it does Hurrah!!, and hitting
things with hammers when they don't work, although satisfying, does get
expensive."

I'm not going to read 6 pages of reverse sequence blog to try to
understand the gist of this conversation, but clearly the original
article is just a big goof on the author's part.

I hope no newbies read it and get very confused by all the nonsense.

One blog comment is rather pertinent. Hamster said, "I'm quite sure
that a good test bench will be far more complex than the unit under
test, and far harder to write." This has been my experience and I think
this point should be emphasized with engineers time and time again!
Writing code is easy. Writing verified code, not so much.

Rick

 

[Jan Decaluwe]

On 11/25/2012 08:11 PM, Jan Decaluwe wrote:

In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&

I thank each one of you very much for the comments.

It's good to talk to people who care about these matters
and know what they are talking about. On APP, there was
almost no progress during 5 (five!) days.

Even better, one of the comments inspired me to construct
an hypothesis of what went wrong here. Alan Fitch wondered
why lfsr was not an unsigned to make the comparison nicer.

Suppose that in his original code (not the one he posted)
the comparison would be with a bitstring of the wrong
size. That would always fail and explain the behavior
that he describes.

He may have misunderstood the fix when making changes
to the code. Where he then gets his strange explanations
is anybody's guess.

Thanks,

Jan

--
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Python as a HDL: http://www.myhdl.org
VHDL development, the modern way: http://www.sigasi.com
World-class digital design: http://www.easics.com

 

[Nicolas Matringe]

Le 25/11/2012 21:53, rickman a écrit :

One blog comment is rather pertinent. Hamster said, "I'm quite sure
that a good test bench will be far more complex than the unit under
test, and far harder to write." This has been my experience and I think
this point should be emphasized with engineers time and time again!
Writing code is easy. Writing verified code, not so much.

Rule of thumb I once heard is to put twice as much engineers in
verification as there are in design.
Funnily, the company I worked for at the time did exactly the reverse.
And surprisingly they were kicked out of the project (and I resigned)

Nicolas

 

[rickman]

On 11/25/2012 6:31 PM, Jan Decaluwe wrote:

On 11/25/2012 08:11 PM, Jan Decaluwe wrote:
In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

(I don't.)

http://www.programmableplanet.com/author.asp?section_id=2551&doc_id=254705&



I thank each one of you very much for the comments.

It's good to talk to people who care about these matters
and know what they are talking about. On APP, there was
almost no progress during 5 (five!) days.

Even better, one of the comments inspired me to construct
an hypothesis of what went wrong here. Alan Fitch wondered
why lfsr was not an unsigned to make the comparison nicer.

Suppose that in his original code (not the one he posted)
the comparison would be with a bitstring of the wrong
size. That would always fail and explain the behavior
that he describes.

He may have misunderstood the fix when making changes
to the code. Where he then gets his strange explanations
is anybody's guess.

This is one of the reasons why I've never created a blog or other
"expert" column on the web. I may be fairly experienced, but by writing
things like this blog I may be showing what things I *don't* know rather
than what I do... lol

Rick

 

[Thomas Stanka]

On 26 Nov., 19:14, rickman <gnu...@gmail.com> wrote:

This is one of the reasons why I've never created a blog or other
"expert" column on the web.  I may be fairly experienced, but by writing
things like this blog I may be showing what things I *don't* know rather
than what I do... lol

I sign this statement

 

[Thomas Stanka]

On 25 Nov., 20:11, Jan Decaluwe <j...@jandecaluwe.com> wrote:

In the following link, a design is presented that alledgedly
has a flaw. The claim is that this is a simple case and
that any experienced designer will see the flaw immediately.

If this code is from an experienced designer I see that much flaws,
that have no effect of synthesis or simulation itself, but on
readability.
The sensitivity list is horrible and will cause unnecessary simulator
load, the code indention is best effort to confuse readers. Some code
beautify would do better. And as simple as this design is, a real
world code without any comments is only good for protecting your
failures from reviewers.

The usage of integer as start value for a lfsr is quite error prone. A
hex value would be easier to read and would allow to design this
module without integer or unsigned values.
That clk_out is missing in reset path leads to one cycle latency of
rst to clk_out, that is most likely not intended here. But you could
only guess if that has impact on system or is even necessary.
If there is an abvious error in cycle lenght (be it start value or
feedback function), you can see this in simulation. I assume if you
use lfsr, you know that you should double check this, if you are not
really sure what you are doing.

bye Thomas

 

[Jan Decaluwe]

On 11/25/2012 09:22 PM, Alan Fitch wrote:

OK I cracked and read it.

I don't really understand what's going on, I would have to simulate it
(though of course you've already done that Jan!).

Yes.

I agree there's something fishy about adding output delays. What
difference would that make in this case?

According to me, none whatsoever. And that is also what I
see in my simulation.

The crazy thing is that he says he does see a difference,
in the sense that the version with delays "reveals the problem."
In particular, he claims that the output clock clk_out
never toggles in the version with delays, which is what
he says he sees on the FPGA also.

He includes waveforms and uses that observation to claim
that "the mystery has been solved".

But I see clk_out toggling correctly (as I would expect).
In particular, in my simulations it toggles at exactly
the same moments as in the version without delays.

The code has been posted on APP in ready-to-run format.
What is driving me crazy is that noone else on APP seems
to be interested in running it and confirming my results,
or not.

I'm not going to ask that from anybody here - you have helped
me enough already. Still, I include the code as it was posted
below, perhaps someone is interested anyway. (The test bench
as posted contains the design with the delays, and is ready
to run.)

Thanks,

Jan

----

First pass at code


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

entity lfsr_counter is
port (
rst, clk : in std_logic;
clk_out : out std_logic);
end lfsr_counter;

architecture imp_lfsr_counter of lfsr_counter is
signal lfsr: std_logic_vector (13 downto 0);
signal d0, clk_i: std_logic;
constant countmax: integer:=2685;
begin

d0 <= lfsr(13) xnor lfsr(4) xnor lfsr(2) xnor lfsr(0) ;

process(clk,clk_i,lfsr,rst) begin

if rising_edge(clk) then
if rst='1' then
clk_i<='0';
lfsr <= (others=>'0');
else lfsr <= lfsr(12 downto 0) & d0;
if lfsr = std_logic_vector((TO_UNSIGNED(countmax,14))) then
clk_i<=not(clk_i);
lfsr<=(others=>'0');
end if;
end if;
clk_out<=clk_i;
end if;
end process;


end architecture imp_lfsr_counter;


---------------------------------------------------


So here is a very simple testbench :-

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

entity test_bench is
---there are no ports to declare so the entity is empty
end test_bench;

architecture behavioural of test_bench is
--call the device under test from the standard 'work' library
component lfsr_counter_pd
port (
rst, clk : in std_logic;
clk_out : out std_logic);
end component;
--declare inputs and initialise them
signal clock_in : std_logic :='0';
signal reset : std_logic :='0';
--declare outputs and initialise them
signal clock_out : std_logic:='0';
--clock period definition
constant clk_period : time :=10 ns;

begin

--instantiate the unit under test
uut: lfsr_counter_pd port map (
rst => reset,
clk => clock_in,
clk_out => clock_out);

--generate the clock with 50% duty cycle
-- this runs for ever it has no sensitivity list
clk_process : process
begin
clock_in <='0';
wait for clk_period/2;
clock_in <='1';
wait for clk_period/2;
end process;
--generate the stimulus for the unit under test

stim_process rocess
begin
wait for 100 ns;
reset<='1';
wait for clk_period * 2;
reset <='0';
wait;

end process;



end architecture behavioural;


--------------------------------------------------------

Another pass at the code


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

entity lfsr_counter_pd is
port (
rst, clk : in std_logic;
clk_out : out std_logic);
end lfsr_counter_pd;

architecture lfsr_counter of lfsr_counter_pd is
signal lfsr: std_logic_vector (13 downto 0);
signal d0, clk_i: std_logic;
constant countmax: integer:=2685;
constant tpd :time:=2 ns;
begin

d0 <= lfsr(13) xnor lfsr(4) xnor lfsr(2) xnor lfsr(0) after tpd;

process(clk,clk_i,lfsr,rst) begin

if rising_edge(clk) then
if rst='1' then
clk_i<='0';
lfsr <= (others=>'0') after tpd;
else lfsr <= lfsr(12 downto 0) & d0 after tpd;
if lfsr = std_logic_vector((TO_UNSIGNED(countmax,14))) then
clk_i<=not(clk_i)after tpd;
lfsr<=(others=>'0') after tpd;
end if;
end if;
clk_out<=clk_i;
end if;
end process;


end architecture lfsr_counter;


-----------------------------------------------------

Final pass at code

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

entity lfsr_counter is
port (
rst, clk : in std_logic;
clk_out : out std_logic);
end lfsr_counter;

architecture lfsr_counter of lfsr_counteris
signal lfsr: std_logic_vector (13 downto 0);
signal d0, clk_i: std_logic;
constant countmax: integer:=2685;

begin


process(clk,clk_i,lfsr,rst) begin

if rising_edge(clk) then
if rst='1' then
clk_i<='0';
lfsr <= (others=>'0') ;
else lfsr <= lfsr(12 downto 0) & d0;
d0 <= lfsr(13) xnor lfsr(4) xnor lfsr(2) xnor lfsr(0);
if lfsr = std_logic_vector((TO_UNSIGNED(countmax,14))) then
clk_i<=not(clk_i);
lfsr<=(others=>'0') ;
end if;
end if;
clk_out<=clk_i;
end if;
end process;


end architecture lfsr_counter;

----------





--
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Python as a HDL: http://www.myhdl.org
VHDL development, the modern way: http://www.sigasi.com
World-class digital design: http://www.easics.com

 

[Christopher Felton]

<snip>

I'm not going to ask that from anybody here - you have helped
me enough already. Still, I include the code as it was posted
below, perhaps someone is interested anyway. (The test bench
as posted contains the design with the delays, and is ready
to run.)

Thanks,

Jan

For giggles I ran the simulation, both are identical (except clock_out
is delayed by tpd in the second). The two waveforms he has in the post
are not on the same "zoom". The first is zoomed way out where you can
see clock_out (solid for clock_in). The second is zoomed way in where
you can see the individual clock_in.

So, clock_out is probably toggling but he's not centered over an edge to
actually view it. If he used a self checking testbench he might have
caught it (or rather not caught it) instead of relying on manually
viewing the waveform.

Post should have been titled "Foiled by testbenches"?

Regards,
Chris

 

[Jan Decaluwe]

On 11/27/2012 07:22 PM, Christopher Felton wrote:

For giggles I ran the simulation, both are identical (except
clock_out is delayed by tpd in the second).

Many thanks!

Mm, that holds for the intermediate clock signal clk_i, but
clk_out itself has no delay, correct?

The two waveforms he has
in the post are not on the same "zoom". The first is zoomed way out
where you can see clock_out (solid for clock_in). The second is
zoomed way in where you can see the individual clock_in.

So, clock_out is probably toggling but he's not centered over an edge
to actually view it.

Could that be it? He wouldn't have bothered to do a full zoom first?
That would be such silly error that I admit I didn't consider it!

My hypothesis of initially faulty code would be consistent
with what he sees on the fpga.

Heck, perhaps I should just throw the towel. Noone on APP seems
to care.

If he used a self checking testbench he might
have caught it (or rather not caught it) instead of relying on
manually viewing the waveform.

Of course. Actually, sometimes I think that if it's not
self-checking, it shouldn't be called a testbench.

It wouldn't be hard for a case like this (by checking edges,
you can verify whether the generated clock with some
frequence or period spec.)

Jan

--
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Python as a HDL: http://www.myhdl.org
VHDL development, the modern way: http://www.sigasi.com
World-class digital design: http://www.easics.com

 

[Jan Decaluwe]

On 11/28/2012 09:34 AM, Thomas Stanka wrote:

On 27 Nov., 17:20, Jan Decaluwe <j...@jandecaluwe.com> wrote:
The crazy thing is that he says he does see a difference,
in the sense that the version with delays "reveals the problem."
In particular, he claims that the output clock clk_out
never toggles in the version with delays, which is what
he says he sees on the FPGA also.

He includes waveforms and uses that observation to claim
that "the mystery has been solved".

But I see clk_out toggling correctly (as I would expect).
In particular, in my simulations it toggles at exactly
the same moments as in the version without delays.

I advice using delays in general to help debuging code and to detect
inadverted clock-2-data race conditions when clock is going over
several signal assignments, but for me, this means, that I usually
only delay signal assignments from clock edge in clocked process, in
usual code thats enough to see, if data changes before or after clock
edge.

This particular code is no example, of code in which I expect delays
to matter (when using correct simulator).
I can only guess, that the simulator the author is using messes up
with this two lines

process(clk,clk_i,lfsr,rst) begin
if rising_edge(clk) then

and execute the code inside the if rising_edge clause also in delta
cycles that have no rising clock edge.

A VHDL simulator broken in this way? Hard to imagine.


--
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
Python as a HDL: http://www.myhdl.org
VHDL development, the modern way: http://www.sigasi.com
World-class digital design: http://www.easics.com

 

[Thomas Stanka]

On 27 Nov., 17:20, Jan Decaluwe <j...@jandecaluwe.com> wrote:

The crazy thing is that he says he does see a difference,
in the sense that the version with delays "reveals the problem."
In particular, he claims that the output clock clk_out
never toggles in the version with delays, which is what
he says he sees on the FPGA also.

He includes waveforms and uses that observation to claim
that "the mystery has been solved".

But I see clk_out toggling correctly (as I would expect).
In particular, in my simulations it toggles at exactly
the same moments as in the version without delays.

I advice using delays in general to help debuging code and to detect
inadverted clock-2-data race conditions when clock is going over
several signal assignments, but for me, this means, that I usually
only delay signal assignments from clock edge in clocked process, in
usual code thats enough to see, if data changes before or after clock
edge.

This particular code is no example, of code in which I expect delays
to matter (when using correct simulator).
I can only guess, that the simulator the author is using messes up
with this two lines

process(clk,clk_i,lfsr,rst) begin
if rising_edge(clk) then

and execute the code inside the if rising_edge clause also in delta
cycles that have no rising clock edge. In that case the trouble might
result from a mixture of unlucky sensitivity list with broken
simulator and missing delays and any one of the three can be used to
fix this issue.

bye Thomas

 

[Christopher Felton]

On 11/27/2012 3:07 PM, Jan Decaluwe wrote:

On 11/27/2012 07:22 PM, Christopher Felton wrote:

For giggles I ran the simulation, both are identical (except
clock_out is delayed by tpd in the second).

Many thanks!

Mm, that holds for the intermediate clock signal clk_i, but
clk_out itself has no delay, correct?

Correct, I misstated. Only the internal nets will see the added delay.

The two waveforms he has
in the post are not on the same "zoom". The first is zoomed way out
where you can see clock_out (solid for clock_in). The second is
zoomed way in where you can see the individual clock_in.

So, clock_out is probably toggling but he's not centered over an edge
to actually view it.

Could that be it? He wouldn't have bothered to do a full zoom first?
That would be such silly error that I admit I didn't consider it!

My hypothesis of initially faulty code would be consistent
with what he sees on the fpga.

I think your hypothesis is correct; we are missing part of the story.
Either the code is not the original code, he had some other error in the
process, or he ignored the timing reports and the second run he just
happened to meet timing. To your point, if he runs syn+P&R again, it
might fail. Nothing solved or fixed.

Heck, perhaps I should just throw the towel. Noone on APP seems
to care.

Which is odd, they should care. In my mind that is the beauty of a
technical *community*, in the end you get a much better result. This is
a perfect example, his goal was to show simulating/testing can be
beneficial. But his example *failed* to prove the point. To the rescue
the community ... except the author didn't want to participate?

In general this is bad, because the correct feedback is available to the
author and it is not being fixed. Hopefully, the poor quality will will
only reflect on the author and not APP's reputations. And yes, it
doesn't seem like any progress is being made, what's the saying, the
advice is falling on deaf ears.

If he used a self checking testbench he might
have caught it (or rather not caught it) instead of relying on
manually viewing the waveform.

Of course. Actually, sometimes I think that if it's not
self-checking, it shouldn't be called a testbench.

wiggle_bench

It wouldn't be hard for a case like this (by checking edges,
you can verify whether the generated clock with some
frequence or period spec.)

Jan