Re: [myhdl-list] Incorrect conclusions in TSConIT paper

[Christopher F. <chr...@gm...>]

On 6/29/12 12:43 PM, Norbo wrote:
> Am 26.06.2012, 13:51 Uhr, schrieb Christopher Felton
> <chr...@gm...>:
>
>> On 6/22/2012 9:57 AM, Jan Decaluwe wrote:
>>> 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
>>
>> The author did a poor job picking their examples.  I say
>> this because I don't think their VHDL example is correct,
>> either.  They don't provide enough information but for the
>> VHDL example to be correct the instantiator would need to
>> take care of the output word size to handle the bit growth
>> as a result of the multiplies and additions (or limit the
>> input range).  I think if they would have verified the VHDL
>> they would have found the VHDL implementation did not meet
>> their requirements (or any requirements for a FIR filter).
>>
>> Also, I don't think the author provides the description of
>> the RAM/ROM for the coefficients in the VHDL example.  The
>> VHDL example is not a fair comparison because it would need
>> additional description, somewhere, to define the coefficient
>> array.  Where as the MyHDL version the coefficient array
>> description is embedded!
>>
>> Here is my version of the FIR filter in MyHDL that matches
>> the simplified templated:
>>
>>       from myhdl import *
>>       def firfilt(sig_in, sig_out, coef, clk):
>>           buffer = [Signal(intbv(0, min=sig_in.min, max=sig_in.max)) \
>>                     for ii in range(len(coef))]
>>           coef = tuple(coef)
>>           mshift = len(sig_in)-1
>>           @always(clk.posedge)
>>           def hdl_sop():
>>               sop = 0
>>               # Note this adds an extra delay! (Group delay N/2+2)
>>               for ii in range(len(coef)):
>>                   buffer[ii].next = sig_in if ii == 0 else buffer[ii-1]
>>                   c = coef[ii]
>>                   sop = sop + (buffer[ii] * c)
>>               sig_out.next = (sop >> mshift)
>>
>>           return hdl_sop
>>
>
> i just tried to run the converted vhdl through quartus and it came up with
> the following error:
>
> 	Error (10500): VHDL syntax error at firfilt.vhd(39) near text "when";
> expecting ";"
>
> hm.. maybe its not supported in a clocked process but i am not so sure i
> have to look at that closer.
> the next error after i changed to a normal if i got:
>
> 	Error (10405): VHDL error at firfilt.vhd(78): can't determine type of
> object at or near identifier "shift_right" -- found 0 possible types
>
>
>
>
> Note: I changed buffer to bufferVal because it is a reserved keyword in
> vhdl
>
>
>
> My version with the initial values would look like this:
>
>     def firfilt(sig_in, sig_out, coef, clk):
>         bufferVal = [Signal(intbv(0, min=sig_in.min, max=sig_in.max)) for ii
> in range(len(coef))]
>         coefs = [Signal(intbv(ii, min=min(coef), max=max(coef)+1)) for ii in
> coef]
>         mshift = len(sig_in)-1
>         @always(clk.posedge)
>         def hdl_sop():
> 	  sop = 0
> 	  # Note this adds an extra delay! (Group delay N/2+2)
> 	  bufferVal[0].next=sig_in
> 	  for ii in range(len(coef)-1):
> 	      bufferVal[ii+1].next = bufferVal[ii]
> 	      sop = sop + (bufferVal[ii] * coefs[ii])
> 	  sig_out.next = (sop >> mshift)
>
>         return hdl_sop
>
> but there is still the shift error.
>
>
> greetings
> Norbo
>
>
>

I didn't follow the reason for adding the LoS with initial values?  Why 
did you add the

     coefs = [Signal(intbv(ii, min=min(coef), max=max(coef)+1)) \
              for ii in coef]
?

Yes, in GHDL I get the following error for the converted VHDL.

     ghdl -c pck_myhdl_08dev.vhd firfilt.vhd
     firfilt.vhd:46:36: ';' is expected instead of 'when'

I am running the latest 0.8dev and didn't check with 0.7.

I am not sure what the the error is, when I have a little more time I 
can investigate (or maybe someone else has some insight).

I did add cosimulation with Verilog to the testbench and the 
cosimulation runs fine (passes tests).  The updated code is available in 
the same location, bitbucket repo.  I renamed the "buffer" as well.  I 
also added a reset so I didn't need a special case for checking the 
output.  The circuit would run ok without a reset it just would take N 
cycles (N being the number of taps) before a valid output is available.

Regards,
Chris