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&

 

[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 :
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

 

[Jan Decaluwe]

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

 

[rickman]

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]

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]

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

 

[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]

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]

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]

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]

I would agree to this further. For this puzzle, some vendor
synthesisers have treated sensitivity lists differently and made
different assumptions to complicate things still further.

There a issues at multiple levels with this. However
a major one has only to do with modeling and simulation,
namely whether adding the delays should make a difference
to the behavior of the clock output (as he claims),
or not (which is what we see and expect).

Any standard VHDL simulator should give the same answer here.

Jan

 

[hansove.frimodig]

The flaw in the design is that a constant integer "countmax" is compared with an LFSR that counts in a random way. This means that the output period will not be as expected.

 

[Michael S]

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=2....

--
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

An original code is o.k. except for bad style, as mentioned by just
about everybody above, and for kludgy idea to use MLSR to divide
clock.
At least what's implemented is real 14-bit MLSR with period = 2^14-1.
It appears to divide the input clock by 24998 and that's supposedly
was an original intention. Or did he tried to divide by 25000? Or, may
be he really divides by 25000 and I miscalculated by one somewhere?
Anyway, it's pretty close

A "fixed" code is much worse.
It, supposedly unintentionally, implements 15-bit LSR with
period=7905, i.e. *not* MLSR. Input clock is divided by 11684. It's
sounds damn unlikely that that was an intention.

 

[Michael S]

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.

On this one, and only this one, I disagree.

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.

I also don't like naming synchronous reset 'rst'. I'd rather prefere
'srst" or 'sreset'.

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

Just about everything he wrote on the second page makes no sense.

 

[Michael S]

On this one, and only this one, I disagree.


I also don't like naming synchronous reset 'rst'. I'd rather prefere
'srst" or 'sreset'.



Just about everything he wrote on the second page makes no sense.

Oh, another style flow is xnor.
The only valid use of xnor is when you want to obfuscate your
intentions.

 

[rickman]

On this one, and only this one, I disagree.

I had a bit of confusion on this one myself. Not that he was using an
integer per-se, but that he didn't explain in a comment why this value
was important or how it was derived. I had to consider that this value
was ill-conceived and didn't want to bother with looking up the LFSR and
calculating where this value would appear in the sequence, etc. It
would have been useful if he had added a comment saying the length of
the loop is xxx clocks or yyy time.

I also don't like naming synchronous reset 'rst'. I'd rather prefere
'srst" or 'sreset'.

I think for FPGAs it is very common to specify an async reset to assign
the configuration value of each FF, so I have come to expect async
resets. But if they use a sync reset, it doesn't bother me. I don't
expect that aspect of the reset to be part of the name. I think resets
are complex enough that they should be designed and documented at the
system level.

Just about everything he wrote on the second page makes no sense.

Yes, I'm not sure what he was thinking. It is a bit funny how he
responds to this in the blog. He starts out discussing it a little
defensively and after three or four rounds of increasing defensiveness
on his side he says something like, "Just forget about it". I expect it
was a bit embarrassing to make a mistake so publicly. I'm sure we have
all made similar mistakes, the kind where we slap the side of our head
and say, "what was I thinking?" But to do it publicly is a different
matter.

I think the "Just forget about it" comment was on the second page of
comments and there were six when I read it. So I guess he is getting
beat up pretty badly. I feel for him.

Rick

 

[rickman]

Oh, another style flow is xnor.
The only valid use of xnor is when you want to obfuscate your
intentions.

I don't understand. What would you use in place of xnor? In LFSRs
there are two ways of coding them. Using the XOR function creates two
loops, one being the all zeros state. Using the XNOR function creates
two loops, one being the all ones state. What is wrong with using the
XNOR function to describe a LFSR with the all ones state as the excluded
state?

Rick

 

[Kerry Imming]

I don't understand. What would you use in place of xnor? In LFSRs there
are two ways of coding them. Using the XOR function creates two loops,
one being the all zeros state. Using the XNOR function creates two loops,
one being the all ones state. What is wrong with using the XNOR function
to describe a LFSR with the all ones state as the excluded state?

I think the problem is not the LFSR, but that the XNOR operation can be
non-intuitive.

For example: a XNOR b XNOR c is equivalent to a XOR b XOR c
Since there is an even number of inversions, the inversions cancel out.
In the given example, there is an odd number of XNORs, so it works as
written.

My preferred style would be. d <= NOT (a XOR b XOR c);

- Kerry

 

[Michael S]

I think the problem is not the LFSR, but that the XNOR operation can be
non-intuitive.

For example:   a XNOR b XNOR c  is equivalent to a XOR b XOR c
Since there is an even number of inversions, the inversions cancel out.
In the given example, there is an odd number of XNORs, so it works as
written.

My preferred style would be.    d <= NOT (a XOR b XOR c);

- Kerry

Yes, that's exactly what I meant to say.