Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > VHDL > VHDL expert puzzle

Reply
Thread Tools

VHDL expert puzzle

 
 
Thomas Stanka
Guest
Posts: n/a
 
      11-27-2012
On 25 Nov., 20:11, Jan Decaluwe <(E-Mail Removed)> 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

 
Reply With Quote
 
 
 
 
Jan Decaluwe
Guest
Posts: n/a
 
      11-28-2012
On 11/28/2012 02:15 PM, Mike Perkins wrote:
> On 27/11/2012 14:44, Thomas Stanka wrote:
>> On 26 Nov., 19:14, rickman <(E-Mail Removed)> 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
>>

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

--
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
 
Reply With Quote
 
 
 
 
hansove.frimodig@gmail.com
Guest
Posts: n/a
 
      11-28-2012
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.
 
Reply With Quote
 
Michael S
Guest
Posts: n/a
 
      11-28-2012
On Nov 25, 9:11*pm, Jan Decaluwe <(E-Mail Removed)> 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/au...=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.


 
Reply With Quote
 
Michael S
Guest
Posts: n/a
 
      11-28-2012
On Nov 27, 5:10*pm, Thomas Stanka <(E-Mail Removed)>
wrote:
> On 25 Nov., 20:11, Jan Decaluwe <(E-Mail Removed)> 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.


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.
 
Reply With Quote
 
Michael S
Guest
Posts: n/a
 
      11-28-2012
On Nov 28, 5:50*pm, Michael S <(E-Mail Removed)> wrote:
> On Nov 27, 5:10*pm, Thomas Stanka <(E-Mail Removed)>
> wrote:
>
>
>
>
>
>
>
>
>
> > On 25 Nov., 20:11, Jan Decaluwe <(E-Mail Removed)> 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.

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


Oh, another style flow is xnor.
The only valid use of xnor is when you want to obfuscate your
intentions.
 
Reply With Quote
 
rickman
Guest
Posts: n/a
 
      11-29-2012
On 11/28/2012 10:50 AM, Michael S wrote:
> On Nov 27, 5:10 pm, Thomas Stanka<(E-Mail Removed)>
> wrote:
>> 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.


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.


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


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.


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


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
 
Reply With Quote
 
rickman
Guest
Posts: n/a
 
      11-29-2012
On 11/28/2012 10:55 AM, Michael S wrote:
>
> 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
 
Reply With Quote
 
Kerry Imming
Guest
Posts: n/a
 
      11-29-2012

"rickman" <(E-Mail Removed)> wrote in message
news:k97ral$oin$(E-Mail Removed)...
> On 11/28/2012 10:55 AM, Michael S wrote:
>>
>> 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?


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


 
Reply With Quote
 
Michael S
Guest
Posts: n/a
 
      11-29-2012
On Nov 29, 4:50*pm, "Kerry Imming" <(E-Mail Removed)> wrote:
>
> 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.

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
VHDL-2002 vs VHDL-93 vs VHDL-87? afd VHDL 1 03-23-2007 09:33 AM
VHDL 2002 vs VHDL 1993 dude VHDL 1 03-23-2006 01:18 PM
multiD-vhdl: Multi Dimensional Arrays (allowing generics on each dimension) for VHDL (including ports) albert.neu@gmail.com VHDL 2 03-21-2006 04:05 PM
A puzzle to puzzle you sk A+ Certification 1 07-17-2004 05:19 PM
what's the difference between VHDL 93 CONCATENATION and VHDL 87 CONCATENATION? walala VHDL 3 09-18-2003 04:17 AM



Advertisments