[myhdl-list] Incorrect conclusions in TSConIT paper

[Jan D. <ja...@ja...>]

I came across a paper that compares HDLs:

http://referaat.cs.utwente.nl/TSConIT/download.php?id=1144

Many conclusions and remarks about MyHDL are incorrect
for the following reasons:

1) the author doesn't fully understand the nature of
MyHDL/Python. This can be excused for a newbie; perhaps
we have to do a better job explaining.

2) Some conclusions are based on non-functional code.
No excuses here; conclusions based on code that doesn't
work are not acceptable.

I will first explain the conceptual errors, and then
go into the details of the incorrect code (for those
interested).

The author makes the common error of equating MyHDL with
conversion. But MyHDL is primarily about modeling in
general. Conversion is really just a workaround to
play well with the backend tools in established
design flows.

For example, he states that MyHDL only supports
intbv and not floating point, which is completely wrong.
For modeling purposes, any type can be used as the underlying
type of a Signal. Conversion is restricted to intbv and a
few other types (because its primary target is synthesis)
but when the purpose is modeling and system verification,
why worry about conversion?

The author has critique on duck typing (which has indeed
disadvantages) but fails to recognize that it is exactly
this that allows MyHDL models to be written in
a type-independent way.

The author complains about verboseness and large amounts
of conditional statements, but fails to take advantage of
the idiomatic Python way to work with booleans:

     empty = Signal(bool(0))
     ...
     empty.next = (dindex == 0)

which would reduce the nr of lines drastically.

Finally, the author complains about cryptic messages when
conversion fails. Taken on its own, he has a point. It is
certainly true that the conversion error reporting can be improved.
However, the reason why this is such a blocking problem in
this case is a flawed methodology. The author seems to consider
the convertor as some kind of compiler which can help to
produce working MyHDL code. But this is completely wrong.
The goal of the convertor is to convert *working* MyHDL code
to equivalent Verilog/VHDL.

Creating working MyHDL code has to be done in the Python way.
We don't have a compiler that finds a lot of errors. In a super
dynamic language like Python, the run-time rules. In MyHDL,
this means simulation:

http://www.myhdl.org/doc/current/manual/conversion.html#simulate-first

My biggest critique on the paper is that the author apparently
hasn't done this. When he got stuck with conversion, his immediate
reaction should have been to simulate. I believe that even a
simple simulation would have revealed basic errors, and
correcting those would have made conversion much more friendly.

This is a severe critique, and I will show in detail
why I came to this conclusion.

The author presents the following example:

--
from myhdl import *

def firfilt (sigin, sigout, coefs, clk):
     buffer = Signal(intbv(0)[len(sigin)*len(coefs)])
     inlen = len(sigin)
     saved_coefs = coefs
     colen = len(saved_coefs)

     @always(clk.posedge)
     def logic():
         buffer.next[inlen:] = sigin
         buffer.next[:inlen] = buffer[inlen * (colen-1):]
         tmp = 0
         for index in range(colen):
             mult = saved_coefs[inlen*(index+1):inlen*index] * \
                    buffer[inlen*(index +1): inlen*index]
             tmp = tmp + sigin
         sigout.next = tmp

     return logic

def convert():
     sin = Signal(intbv(0)[8:])
     sout = Signal(intbv(0)[8:])
     coefs = [2, 8, 6 ,20 ,3]
     clk = Signal(bool(0))
     toVHDL(firfilt , sin , sout , coefs , clk )

convert()
--

It seems clear that this code has not been simulated and
cannot work. First:

     buffer = Signal(intbv(0)[len(sigin)*len(coefs)])

is most likely wrong as only a single bit is selected,
Probably what is meant is a bit vector definition as follows:

     buffer = Signal(intbv(0)[inlen*colen:])

Furthermore, the following lines:

         mult = saved_coefs[inlen*(index+1):inlen*index] * \
                buffer[inlen*(index +1): inlen*index]
         tmp = tmp + sigin

must be wrong: what can be the purpose of multiplying
a sequence of constants with a bit vector?  Furthermore, it
seems nothing happens with variable mult. Looking at the VHDL
version, what was probably intended is something like
the following:

--
from myhdl import *

def firfilt (sigin, sigout, coefs, clk):
     inlen = len(sigin)
     colen = len(coefs)
     buffer = Signal(intbv(0)[inlen*colen:])

     @always(clk.posedge)
     def logic():
         buffer.next[inlen:] = sigin
         buffer.next[:inlen] = buffer[inlen*(colen-1):]
         tmp = 0
         for index in range(colen):
             c = coefs[index]  # select one coefficient
             mult = c * buffer[inlen*(index +1):inlen*index]
             tmp = tmp + mult # mult/accumulate
         sigout.next = tmp

     return logic

def convert():
     sin = Signal(intbv(0)[8:])
     sout = Signal(intbv(0)[8:])
     coefs = (2, 8, 6, 20, 3) # tuple of constants
     clk = Signal(bool(0))
     toVHDL(firfilt , sin , sout , coefs , clk )

convert()
--


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