Re: [myhdl-list] Conversion error to vhdl

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

On 12/6/11 8:12 AM, Norbo wrote:
> On Sat, 03 Dec 2011 19:39:26 +0100, Christopher Felton
> <chr...@gm...>  wrote:
>
>> On 11/30/11 9:52 AM, Norbo wrote:
>>> On Wed, 30 Nov 2011 13:39:25 +0100, Christopher Felton
>>> <chr...@gm...>   wrote:
>>>
>>>> On 11/30/11 5:48 AM, Norbo wrote:
>>>>> On Wed, 23 Nov 2011 17:13:27 +0100, Christopher Felton
>>>>> <chr...@gm...>    wrote:
>>>>>
>>>>>> On 11/22/11 3:15 PM, Norbo wrote:
>>>>>>> On Mon, 21 Nov 2011 17:28:05 +0100, Christopher Felton
>>>>>>> <chr...@gm...>     wrote:
>>>>>>>
>>>>>>>> On 11/21/2011 8:52 AM, Oscar Diaz wrote:
>>>>>>>>> 2011/11/19 Norbo<Nor...@gm...>:
>>>>>>>>>> I have played a bit with myhdl and created the following:
>>>>>>>>>>
>>>>>>>>>> ----------------------------------------------------------
>>>>>>>>>>       from myhdl import *
>>>>>>>>>>
>>>>>>>>>> def bin2gray(A,B, C, width):
>>>>>>>>>>
>>>>>>>>>>          """ Gray encoder.
>>>>>>>>>>
>>>>>>>>>>          B -- input intbv signal, binary encoded
>>>>>>>>>>          G -- output intbv signal, gray encoded
>>>>>>>>>>          width -- bit width
>>>>>>>>>>
>>>>>>>>>>          """
>>>>>>>>>>          @always_comb
>>>>>>>>>>          def logic():
>>>>>>>>>>              C[0].next = A + B
>>>>>>>>>
>>>>>>>>> It seems that you have a bug here. Why you assign the result of a
>>>>>>>>> sum
>>>>>>>>> to a single bit? that's the reason why the converter tried to use
>>>>>>>>> "to_std_logic" for the output. If you want a normal sum, it should
>>>>>>>>> be
>>>>>>>>>
>>>>>>>>> C.next = A + B
>>>>>>>>>
>>>>>>>>> and the VHDL converter will put synthesizable code:
>>>>>>>>>
>>>>>>>>> C<= (A + B);
>>>>>>>>>
>>>>>>>>
>>>>>>>> Good eye Oscar!
>>>>>>>>
>>>>>>>> Yes, there are a couple issues with the bin2gray that was provided
>>>>>>>> (I
>>>>>>>> assumed the bin2gray was an example for conversion means and not an
>>>>>>>> actual bin2gray?).  I didn't notice at first glance but if you want
>>>>>>>> to
>>>>>>>> assign a single bit of a bit-vector you need to do:
>>>>>>>>
>>>>>>>>        C.next[0] = A + B
>>>>>>>>
>>>>>>>>
>>>>>>>> If you assign the bit correctly the conversion works (even though
>>>>>>>> it
>>>>>>>> functional doesn't work).  Simulation will not work!.  Not sure why
>>>>>>>> the
>>>>>>>> converter didn't flag this?  Possible error detection enhancement.
>>>>>>>>
>>>>>>>> Also using the "+" operator instead of the bitwise "^" will cause
>>>>>>>> problems in simulation.  The "+" will overflow the range for a
>>>>>>>> single
>>>>>>>> bit.
>>>>>>>>
>>>>>>>> My previous reply can be ignored and should be ignored.  If any
>>>>>>>> changes
>>>>>>>> need to be made they would be in the error catching/reporting and
>>>>>>>> not
>>>>>>>> modifications to the conversion.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Chris
>>>>>>>>
>>>>>>>>> Or, if you want a binary to gray encoder, just use the example
>>>>>>>>> from
>>>>>>>>> the documentation
>>>>>>>>>
>>>>>>>>> http://www.myhdl.org/doc/current/manual/conversion_examples.html#a-small-combinatorial-design
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Best Regards
>>>>>>>>>
>>>>>>>
>>>>>>> Well the thing is:
>>>>>>>
>>>>>>> If i write:
>>>>>>>
>>>>>>> C[4:0].next = A+B    in vhdl it gets ->      C(4-1 downto 0)<=
>>>>>>> resize(A
>>>>>>> +
>>>>>>> B,4);
>>>>>>>
>>>>>>> or:
>>>>>>>
>>>>>>> C[1:0].next = A+B   in vhdl it gets ->      C(1-1 downto 0)<=
>>>>>>> resize(A +
>>>>>>> B,1);
>>>>>>> which in  in my understanding this makes kind of sense because the
>>>>>>> result
>>>>>>> is cropped to the output size. (maybe a warning would be nice, but
>>>>>>> it
>>>>>>> could be difficult if the calculation gets longer)
>>>>>>>
>>>>>>>
>>>>>>> but when i write :
>>>>>>>
>>>>>>> C[0].next = A+B
>>>>>>> (which is actually the same as the above C[1:0].next = A+B)
>>>>>>>          in vhdl it gets ->      C(0)<= to_std_logic(A + B);
>>>>>>> and is not synthesisable.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> As mentioned, the above is the incorrect syntax.  The converted
>>>>>> results
>>>>>> are unreliable.  To do bit indexing you need to do:
>>>>>>
>>>>>>       C.next[0] = A[0] + B[1]
>>>>>>
>>>>>> If you do C[0].next that is used for a list of signals.  If you run a
>>>>>> simulation (testbench) this error will be identified.
>>>>>>
>>>>>> Review the following links:
>>>>>> http://www.myhdl.org/doc/current/manual/intro.html#bit-indexing
>>>>>> http://www.myhdl.org/doc/current/manual/conversion_examples.html#a-small-combinatorial-design
>>>>>>
>>>>>> Also, run a simple simulation.  That will give you some more insight
>>>>>> into the issue.
>>>>>>
>>>>>> Regards,
>>>>>> Chris
>>>>>
>>>>> Sorry that i keep bagging, i really like myhdl and i really want to
>>>>> get
>>>>> it
>>>>> better
>>>>> so if i write:
>>>>>
>>>>> C.next[0] = A[0] + B[1]
>>>>>
>>>>> the line which is produced is:
>>>>>
>>>>> C(0)<= to_std_logic(to_unsigned(A(0), 1) + to_unsigned(B(0), 1));
>>>>>
>>>>> and it is not synthesisable because of the to_std_logic.
>>>>
>>>> Correct, the conversion generated an equivilant assignment.  Which is
>>>> not synthesizable (it is syntactically correct and will compile with a
>>>> VHDL simulator).  But the two statements are equivalent.  If you run a
>>>> MyHDL simulation you will get more insight.
>>>>
>>>> As previously mentioned the example is not functionally correct.  The
>>>> synthesis tools will catch this, especially VHDL because it is strongly
>>>> typed.
>>>>
>>>>>
>>>>>
>>>>> wheras:
>>>>> C.next[1:0] = A[0] + B[1]
>>>>>
>>>>> gets:
>>>>> C(1-1 downto 0)<= (to_unsigned(A(0), 1) + to_unsigned(B(1), 1));
>>>>> and is synthesisable.
>>>>>
>>>>
>>>> This one works because you provided enough bits for the result.  As
>>>> mentioned, the MyHDL simulation would flag the error.  Verifying your
>>>> module is a simple task.  You should have test code that verifies your
>>>> logic before trying conversion!
>>>>
>>>>
>>>> .chris
>>>
>>>
>>> Case1:
>>>
>>> C.next[1:0] = A[0] + B[1]
>>>
>>> Case2:
>>>
>>> C.next[0] = A[0] + B[1]
>>>
>>> Documentation says:
>>> The Python convention of half-open ranges is followed: the bit with the
>>> highest index is not included
>>> (by the way in the documatation on the page 89 in the table there seems
>>> to
>>> be an error
>>> bv[i:j] =>   slice of bv from i downto j      ----->   should be changed
>>> to
>>> bv[i:j] =>   slice of bv from i-1 downto j  )
>>>
>>>
>>> The result intbv has the same length in both cases. And numberic_std
>>> library says:
>>>
>>>      function "+" (L, R: UNSIGNED) return UNSIGNED =>   "plus";
>>>      -- Result subtype: UNSIGNED(MAX(L'LENGTH, R'LENGTH)-1 downto 0).
>>>
>>> so the result length of an addition of two unsigned has the same length
>>> as
>>> the longest of the both operators and
>>> the result type is a subtype of unsigned.
>>>
>>> greetings
>>>
>>>
>>
>> Let's backup a little.  I think you have identified some areas for
>> improvement but I don't think you have discovered something
>> fundamentally broken.  I believe your goal is to become more familiar
>> with MyHDL and while trying it out you have found some items that you
>> would like some clarification, correct?
>>
>> A couple examples might help.  Your original and subsequent post had to
>> do with bit assignments to an intbv.  We can create examples for the
>> bitwise operators and the numerical operators, run a simulation, and see
>> what happens.  And remove confusion by not using a module name that
>> implies some functionality we are not trying to implement.
>>
>> ~~~[Code Example]~~~
>> from myhdl import *
>> import traceback
>> import sys
>>
>> def TryBitAssignXorOperator(a,b,c):
>>
>>       @always_comb
>>       def hdl_assign():
>>           c.next[0] = a[0] ^ b[0]
>>
>>       return hdl_assign
>>
>> def TryBitAssignAddOperator(a,b,c):
>>
>>       @always_comb
>>       def hdl_assign():
>>           c.next[0] = a[0] + b[0]
>>
>>       return hdl_assign
>>
>> def test():
>>
>>       Nbits = 3
>>       a = Signal(intbv(0)[Nbits:])
>>       b = Signal(intbv(0)[Nbits:])
>>       c_xor = Signal(intbv(0)[Nbits:])
>>       c_add = Signal(intbv(0)[Nbits:])
>>
>>       d_xor = TryBitAssignXorOperator(a,b,c_xor)
>>       d_add = TryBitAssignAddOperator(a,b,c_add)
>>
>>       @instance
>>       def tb_stimulus():
>>           for ii in xrange(2**Nbits):
>>               for jj in xrange(2**Nbits):
>>                   a.next = ii
>>                   b.next = jj
>>                   yield delay(1)
>>                   print("a:%s b:%s c_xor:%s c_add:%s" % \
>>                         (bin(a,Nbits), bin(b,Nbits),
>>                          bin(c_xor,Nbits),
>>                          bin(c_add,Nbits)))
>>
>>
>>
>>           raise StopSimulation
>>
>>       return d_xor, d_add, tb_stimulus
>>
>> if __name__ == '__main__':
>>       Simulation(test()).run()
>>
>> ~~~[End Code Example]~~~
>>
>> ~~~[Output]~~~
>> a:000 b:000 c_xor:000 c_add:000
>> a:000 b:001 c_xor:001 c_add:001
>> a:000 b:010 c_xor:000 c_add:000
>> a:000 b:011 c_xor:001 c_add:001
>> a:000 b:100 c_xor:000 c_add:000
>> a:000 b:101 c_xor:001 c_add:001
>> a:000 b:110 c_xor:000 c_add:000
>> a:000 b:111 c_xor:001 c_add:001
>> a:001 b:000 c_xor:001 c_add:001
>> ...
>> python2.7/site-packages/myhdl/_intbv.py", line 183, in __setitem__
>>       "            i == %s " % i
>> ValueError: intbv[i] = v requires v in (0, 1)
>>               i == 0
>>
>> ~~~[End Output]~~~
>>
>> In this example you see that it fails the *add* when "a" and "b" are 1
>> (it doesn't print out a:001 b:001 because the exception occurs as soon
>> as the assignment happens)!  This is because the result of an intbv 1+1
>> requires two bits.  An intbv is a constraint integer, it works more like
>> VHDL signed/unsigned and not like std_logic_vector or Verilog's
>> wire/reg.  You can't do a single bit assignment to a value that requires
>> more than 1 bit.
>>
>> It is my guess that the conversion doesn't handle this error case better
>> because, usually, simulation is run and the error would be flagged in
>> simulation.  Hence, there has been no need to convert code like your
>> example.
>>
>> What we can do is create an enhancement proposal that outlines a set of
>> bit assign tests to extend to the conversion regression tests and
>> specify useful error messages to be thrown if invalid conversion is
>> attempted.  I don't think this was the goal of your inquiry (was it?).
>> But rather to get some more experience with MyHDL.
>>
>> In summary, what you are trying to do is invalid.  Simulation will flag
>> this correctly.
>>
>> Hope this helps,
>> Chris
>
>
>
>> Let's backup a little.  I think you have identified some areas for
>> improvement but I don't think you have discovered something
>> fundamentally broken.
>
> I have too admit that even the title of this topic ->  "conversion error to
> vhdl" is kind
> of rough, because we are all  (at least in my case) taught in school, high
> school, whatever, that making an error
> is not so good. And even if you dont read the title consciously, your
> subconscious will put all the feelings and associations
> forward which are connected to it. Since some of them have been in school
> where we have been conditioned to errors such a title
> can have an enormous effect. For example making the reader think, that the
> writer think, that the reader have done something bad.
> But i think the only bad errors which are made, are made by people on
> purpose to hurt (in this or another way) someone.
> And the other errors, kind of must be there as a source of improvement or
> re-adjustment or whatsoever. So its about improvement.

Not sure what this commentary is about.  If you think you a detecting 
defensiveness, your not.  I do find the MyHDL project useful and I am 
trying to contribute where I can.  Jan Decaluwe has done a great job 
with MyHDL and improvements are the goal.  On that note, Jan is the main 
developer and maintainer, and at this point all changes go through Jan. 
  Jan is currently incommunicado, see this post, http://bit.ly/v0eI9C, 
for more information.

>
>> I believe your goal is to become more familiar
>> with MyHDL
>
> true.
>
>
>
> Ok so what is about:
>
>    c.next[0] = a[0] * b[0]<---- multiplikation instead of addition
>
> here the simulation runs successfully (100% testcoverage), and so the
> conversion no warning and no errors diplayed
>
> this gets converted to
>
> c(0)<= to_std_logic(to_unsigned(a(0), 1) * to_unsigned(b(0), 1));
>
> which is not synthesisable.

The '*' is a valid example that fails.  This should convert and a 
version of to_std_logic supporting unsigned should be added.  But I 
don't think the change should be as simple as that.  Per the previous 
discussions if the '+' is converted in the same matter the simulation 
and cosimulation (simulation of converted code) will not match.  If the 
to_std_logic(unsigned) is to be added the non valid cases need to be 
caught and an error thrown.  Which makes the change more complicated.

>
>
>
> The thing about the simulation is that if you have a unit with a lot of
> inputs you cannot possible run
> all number of combinations on the input. If you have internal registers
> the thing gets even worse. So you can
> only get a Testcoverage below 100%, so there is the change to actually
> miss this case if you have an addition.

This is kinda a can of worms.  I don't think having a larger number of 
inputs is a good reason not to *test* with simulation.  Yes, you might 
not ever be able to get 100% coverage.  Which, in my mind, is a reason 
why you would not want the convert invalid code (the case of the '+'). 
Not obtaining 100% coverage is not a good reason not to achieve some 
coverage with simulation.


>
> Anyway it seem to be possible to get into a situation where the simulation
> will run successfully (lets say 99% coverage or in the case of
> the multiplikation even 100%), the conversion runs
> successfully, but the generated code is not synthesisable.
> I mean i like the fact that the bitwith of the operand of an addition is
> automatically enhanced to create an extra bit for the overflow bit if the
> code is generated in vhdl, but i think this should not necessarily include
> that if you want to reduce the bitwith of the result by giving a
> bitwith constraint to the result, that the conversion result is not valid.
>
>
>
> I think the easiest way to fix this is to just add a to_std_logic( a
> unsigned ) to the pck_myhdl_07.vhd generator code. But, maybe this has
> some other implications.?

As discussed, the multiplication case is a valid error.  This could be 
entered as a bug.  The add/subtract doesn't hold as much water 
(converted HDL will not act the same as the MyHDL) but this can be an 
enhancement to throw an error instead of creating VHDL that will not 
compile/synthesize.  We should also compare how the Verilog converter 
works but otherwise I think I am convinced that the 
to_std_logic(unsigned) should be added.  We can make a patch but I think 
also a MEP should be created to address the '+'/'-'.

If you want you can follow the guidelines here, http://bit.ly/ueKy5Y, 
and create a patch.

Regards,
Chris

>
> greetings Norbert
>
>
> ------------------------------------------------------------------------------
> Cloud Services Checklist: Pricing and Packaging Optimization
> This white paper is intended to serve as a reference, checklist and point of
> discussion for anyone considering optimizing the pricing and packaging model
> of a cloud services business. Read Now!
> http://www.accelacomm.com/jaw/sfnl/114/51491232/