myhdl-list

[myhdl-list] intbv wrap

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

At this point in time I am going to defer converting the wrap function 
as briefly discussed in other threads.  It is beyond my current 
capabilities.  But this doesn't change the approach.  It simply means 
the wrap can only be used as an attribute of the intbv object, thank you 
Tom Dillon for the pointer here.

It also seemed appropriate to add a property for _val, see the following.

     # val property
     def _get_val(self):
         return self._val

     def _set_val(self, pval):

         if self._wrap:
             self._val = self.wrap(pval)
         else:
             self._val = pval

         self._checkBounds()

     val = property(_get_val, _set_val)


This had been discussed in some older threads.  Are there any objections 
against this type of implementation?  The _checkBounds and wrap will be 
called only from the _set_val property function.

Above self._wrap is a data member initialized in the constructor based 
on a parameter.

def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):

Thanks
Chris Felton

Re: [myhdl-list] intbv wrap

[Tom D. <td...@di...>]

On 05/04/2011 03:00 AM, Christopher Felton wrote:
> At this point in time I am going to defer converting the wrap function
> as briefly discussed in other threads.  It is beyond my current
> capabilities.  But this doesn't change the approach.  It simply means
> the wrap can only be used as an attribute of the intbv object, thank you
> Tom Dillon for the pointer here.

Do you mean we won't have wrap support in conversion? Would it be 
possible to put an error or warning in so you would know the logic won't 
match the MyHDL code?

It is beyond my capabilities too, I still conversion is magic.

> It also seemed appropriate to add a property for _val, see the following.
>
>       # val property
>       def _get_val(self):
>           return self._val
>
>       def _set_val(self, pval):
>
>           if self._wrap:
>               self._val = self.wrap(pval)
>           else:
>               self._val = pval
>
>           self._checkBounds()
>
>       val = property(_get_val, _set_val)
>
>
> This had been discussed in some older threads.  Are there any objections
> against this type of implementation?  The _checkBounds and wrap will be
> called only from the _set_val property function.

Will intbv work as it does now? I am a little rusty on properties.

Re: [myhdl-list] intbv wrap

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

On 5/4/2011 9:11 AM, Tom Dillon wrote:
>
>
> On 05/04/2011 03:00 AM, Christopher Felton wrote:
>> At this point in time I am going to defer converting the wrap function
>> as briefly discussed in other threads.  It is beyond my current
>> capabilities.  But this doesn't change the approach.  It simply means
>> the wrap can only be used as an attribute of the intbv object, thank you
>> Tom Dillon for the pointer here.
>
> Do you mean we won't have wrap support in conversion? Would it be
> possible to put an error or warning in so you would know the logic won't
> match the MyHDL code?

No, it will be *available* in conversion.  And the Verilog/VHDL will 
match.

In my previous post I mentioned there might be two approaches supported.

   1.  x = Signal(intbv(0, wrap=True)
       x.next[:] = x + 1  # automatically wraps
This method is automatically handled by the conversion, nothing has to 
be done.  But the designer needs to remember which Signal(intbv) have 
been setup to wrap.

   2.  x.next[:] = x.wrap(x + 1).
This approach is more explicit and follows current approach of 
explicitly indicating a wrap (for unsigned).  I think both approaches 
would be nice, but with approach 1 conversion is free and approach 2 
conversion is not free :(

>
> It is beyond my capabilities too, I still conversion is magic.
>
>> It also seemed appropriate to add a property for _val, see the following.
>>
>>        # val property
>>        def _get_val(self):
>>            return self._val
>>
>>        def _set_val(self, pval):
>>
>>            if self._wrap:
>>                self._val = self.wrap(pval)
>>            else:
>>                self._val = pval
>>
>>            self._checkBounds()
>>
>>        val = property(_get_val, _set_val)
>>
>>
>> This had been discussed in some older threads.  Are there any objections
>> against this type of implementation?  The _checkBounds and wrap will be
>> called only from the _set_val property function.
>
> Will intbv work as it does now? I am a little rusty on properties.

Yes, this does not change the functionality only the implementation.  A 
property is syntax sugar, when you have a data member _val you can 
assign getter and setter functions using the property.  Instead of using 
this._val = x, this.val = x is used and will call _set_val(x).

This way the wrap checking and bound checking which would be included 
with each _val assignment can be confined to one spot.  Hopefully 
minimize human errors without too much overhead.

This was discussed previously, that a property be added as an 
alternative to assign values to intbv

x = intbv(0, min=-8, max=8)
x[:]  = 4
x.val = 4

Both accomplish the same thing.  This was mentioned as a *possible* 
addition in the past.  I forget if there were any reasons not to do this.

Chris

Re: [myhdl-list] intbv wrap

[Ben <ben...@gm...>]

On Wed, May 4, 2011 at 10:00, Christopher Felton <chr...@gm...> wrote:
> At this point in time I am going to defer converting the wrap function
> as briefly discussed in other threads.  It is beyond my current
> capabilities.  But this doesn't change the approach.  It simply means
> the wrap can only be used as an attribute of the intbv object, thank you
> Tom Dillon for the pointer here.

Looks like a good idea. See my comments bellow.

>
> It also seemed appropriate to add a property for _val, see the following.
>
>     # val property
>     def _get_val(self):
>         return self._val
>
>     def _set_val(self, pval):
>
>         if self._wrap:
>             self._val = self.wrap(pval)

do both _wrap and wrap exists ?

>         else:
>             self._val = pval
>
>         self._checkBounds()
>
>     val = property(_get_val, _set_val)

I guess you'd better like to call the property _val and put its
internal value inside __val. This way you minimize your impact on the
rest of the code.

>
>
> This had been discussed in some older threads.  Are there any objections
> against this type of implementation?  The _checkBounds and wrap will be
> called only from the _set_val property function.
>
> Above self._wrap is a data member initialized in the constructor based
> on a parameter.
>
> def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):

My understanding is that in the constructor, if wrap is a boolean
(isinstance(wrap, boolean)), and equal to True, then do the regular
wrapping (modulo max), otherwise if set to False, don't do wrapping
(same as now), in a third case, one could give a function to use as
parameter like for instance one that throw an overflow exception when
it occurs:

def mywrapfunc(self, val):
    if val > self._max or val < self._min:
      raise OverflowException()
    self._val = val

this function would simply be given as parameter to the constructor
like intbv(wrap=mywrapfunc).

I can try to put together a real patch with tests this evening ...
can't promise it though.

@Tom: backward compatibility is a must there.

Regards,
Benoît

Re: [myhdl-list] intbv wrap

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

On 5/4/2011 9:30 AM, Ben wrote:
> On Wed, May 4, 2011 at 10:00, Christopher Felton<chr...@gm...>  wrote:
>> At this point in time I am going to defer converting the wrap function
>> as briefly discussed in other threads.  It is beyond my current
>> capabilities.  But this doesn't change the approach.  It simply means
>> the wrap can only be used as an attribute of the intbv object, thank you
>> Tom Dillon for the pointer here.
>
> Looks like a good idea. See my comments bellow.
>
>>
>> It also seemed appropriate to add a property for _val, see the following.
>>
>>      # val property
>>      def _get_val(self):
>>          return self._val
>>
>>      def _set_val(self, pval):
>>
>>          if self._wrap:
>>              self._val = self.wrap(pval)
>
> do both _wrap and wrap exists ?
>
>>          else:
>>              self._val = pval
>>
>>          self._checkBounds()
>>
>>      val = property(_get_val, _set_val)
>
> I guess you'd better like to call the property _val and put its
> internal value inside __val. This way you minimize your impact on the
> rest of the code.

Good point, I think the issue will be if we want val to be published as 
a public or private property?

>
>>
>>
>> This had been discussed in some older threads.  Are there any objections
>> against this type of implementation?  The _checkBounds and wrap will be
>> called only from the _set_val property function.
>>
>> Above self._wrap is a data member initialized in the constructor based
>> on a parameter.
>>
>> def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):
>
> My understanding is that in the constructor, if wrap is a boolean
> (isinstance(wrap, boolean)), and equal to True, then do the regular
> wrapping (modulo max), otherwise if set to False, don't do wrapping
> (same as now), in a third case, one could give a function to use as
> parameter like for instance one that throw an overflow exception when
> it occurs:
>
> def mywrapfunc(self, val):
>      if val>  self._max or val<  self._min:
>        raise OverflowException()
>      self._val = val
>
> this function would simply be given as parameter to the constructor
> like intbv(wrap=mywrapfunc).

The normal behavior now is to throw and exception (assert?) if min or 
max are exceeded (non wrap case).  And the wrap function will verify the 
correct type, the intbv max and min need to be the full range for the 
binary word.

I would be a little worried about a user supplied function because it 
would be difficult to state what is valid.  No additional logic could be 
added only binary word behavior.  I think there are only the two cases, 
wrap case and non-wrap.  In the final HDL nothing actually prevents it 
from wrapping but in simulation/verification this will automatically be 
caught if the min/max should be be exceeded (non-wrap).

>
> I can try to put together a real patch with tests this evening ...
> can't promise it though.

I will provide my latest patch tonight, I believe it should be finished, 
I am still working on the test cases.  If you like you can review my 
patch and send suggestions.  Jan D. published procedures for creating 
patches with Mercurial, http://www.myhdl.org/doku.php/dev:patches.

>
> @Tom: backward compatibility is a must there.
>
> Regards,
> Benoît
>
> ------------------------------------------------------------------------------
> WhatsUp Gold - Download Free Network Management Software
> The most intuitive, comprehensive, and cost-effective network
> management toolset available today.  Delivers lowest initial
> acquisition cost and overall TCO of any competing solution.
> http://p.sf.net/sfu/whatsupgold-sd

Re: [myhdl-list] intbv wrap

[Ben <ben...@gm...>]

On Wed, May 4, 2011 at 16:46, Christopher Felton <chr...@gm...> wrote:
> On 5/4/2011 9:30 AM, Ben wrote:
>> On Wed, May 4, 2011 at 10:00, Christopher Felton<chr...@gm...>  wrote:
>>> At this point in time I am going to defer converting the wrap function
>>> as briefly discussed in other threads.  It is beyond my current
>>> capabilities.  But this doesn't change the approach.  It simply means
>>> the wrap can only be used as an attribute of the intbv object, thank you
>>> Tom Dillon for the pointer here.
>>
>> Looks like a good idea. See my comments bellow.
>>
>>>
>>> It also seemed appropriate to add a property for _val, see the following.
>>>
>>>      # val property
>>>      def _get_val(self):
>>>          return self._val
>>>
>>>      def _set_val(self, pval):
>>>
>>>          if self._wrap:
>>>              self._val = self.wrap(pval)
>>
>> do both _wrap and wrap exists ?
>>
>>>          else:
>>>              self._val = pval
>>>
>>>          self._checkBounds()
>>>
>>>      val = property(_get_val, _set_val)
>>
>> I guess you'd better like to call the property _val and put its
>> internal value inside __val. This way you minimize your impact on the
>> rest of the code.
>
> Good point, I think the issue will be if we want val to be published as
> a public or private property?

val is currently private. to access it directly, you have to call
A._val or use one of the wrapper function (+, -, etc ...)

by wrapping the property "_val", every affectation inside the module
would make use of the wrapping functionnality. currently, only the
__setitem__ method.

I think making val available to the outside as a public property
should be the purpose of a separate patch.

>
>>
>>>
>>>
>>> This had been discussed in some older threads.  Are there any objections
>>> against this type of implementation?  The _checkBounds and wrap will be
>>> called only from the _set_val property function.
>>>
>>> Above self._wrap is a data member initialized in the constructor based
>>> on a parameter.
>>>
>>> def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):
>>
>> My understanding is that in the constructor, if wrap is a boolean
>> (isinstance(wrap, boolean)), and equal to True, then do the regular
>> wrapping (modulo max), otherwise if set to False, don't do wrapping
>> (same as now), in a third case, one could give a function to use as
>> parameter like for instance one that throw an overflow exception when
>> it occurs:
>>
>> def mywrapfunc(self, val):
>>      if val>  self._max or val<  self._min:
>>        raise OverflowException()
>>      self._val = val
>>
>> this function would simply be given as parameter to the constructor
>> like intbv(wrap=mywrapfunc).
>
> The normal behavior now is to throw and exception (assert?) if min or
> max are exceeded (non wrap case).  And the wrap function will verify the
> correct type, the intbv max and min need to be the full range for the
> binary word.

Did not remembered that ... Then consider my example as a
representation of the current situation using your new machinery ;)
BTW, a ValueError is raised.

>
> I would be a little worried about a user supplied function because it
> would be difficult to state what is valid.  No additional logic could be
> added only binary word behavior.  I think there are only the two cases,
> wrap case and non-wrap.  In the final HDL nothing actually prevents it
> from wrapping but in simulation/verification this will automatically be
> caught if the min/max should be be exceeded (non-wrap).

Fair enough, this could be the purpose of a following patch.

>
>>
>> I can try to put together a real patch with tests this evening ...
>> can't promise it though.
>
> I will provide my latest patch tonight, I believe it should be finished,
> I am still working on the test cases.  If you like you can review my
> patch and send suggestions.  Jan D. published procedures for creating
> patches with Mercurial, http://www.myhdl.org/doku.php/dev:patches.

I was not aware of the state of your patch. I thought this was only a
proof of concept. I'd be delighted to review it in its whole !

Regards,
Benoît

As a side note your "I am still working on the test cases" is not
really comforting, as the test case should be finished before you type
the first line of code ;) ... in a perfect world.

Re: [myhdl-list] intbv wrap

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

On 5/4/2011 10:13 AM, Ben wrote:
<snip>
>
> I was not aware of the state of your patch. I thought this was only a
> proof of concept. I'd be delighted to review it in its whole !
>
> Regards,
> Benoît

Thanks for the additional input!  And no problem, I posted the first 
rough patch a week or so ago.  I will think about limiting the change to 
_val only.  Because I have already made the changes and would have to 
undo or start over (which isn't a huge deal) I would prefer to clump the 
two changes together.

Since I have made the change, ran the regression tests, and ran new 
tests I would like to add the public val property in this patch, as 
well, if it is not too big an issue.  Just to save a little of my time. 
  In general, I agree, it is nice to keep changes separate and minimal. 
  Try to avoid including too many changes with one patch.

>
> As a side note your "I am still working on the test cases" is not
> really comforting, as the test case should be finished before you type
> the first line of code ;) ... in a perfect world.
>

I know, sigh, I had a small set of test cases first but I want to add 
more exhaustive tests and incorporate it with py.test.  The gen2 test 
framework in the distro.  For some reason, I have a horrible time 
getting py.py (py.test) installed.  I can't get easy_install installed 
to install py.test, doh.

Chris

Re: [myhdl-list] intbv wrap

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

<snip>
>>>
>>> I can try to put together a real patch with tests this evening ...
>>> can't promise it though.
>>
>> I will provide my latest patch tonight, I believe it should be finished,
>> I am still working on the test cases.  If you like you can review my
>> patch and send suggestions.  Jan D. published procedures for creating
>> patches with Mercurial, http://www.myhdl.org/doku.php/dev:patches.
>
> I was not aware of the state of your patch. I thought this was only a
> proof of concept. I'd be delighted to review it in its whole !
>

Here is the latest I have put together.  It has both the wrap and the 
public val property.

Simple test case
-----------------

from myhdl import *
x = intbv(0, min=-8, max=8, wrap=True)
x[:] = x + 1
assert x == 1
x[:] = x + 2
assert x == 3
x[:] = x + 5
assert x == -8
x[:] = x + 1
assert x == -7
x[:] = x - 5
assert x == 4
x[:] = x - 4
assert x == 0
x[:] = x - 1
assert x == -1

Chris

Re: [myhdl-list] intbv wrap

[Ben <ben...@gm...>]

Hi Christopher,

I took the liberty to review and edit your patch.

As you can see, I added some tests, and removed the inclusion of the
val property. It became quite smaller ! I left your name in it as the
idea and the logic is from you.

I also moved the check from the wrap function to the constructor (no
need to check every time.).

Here is a text version, enclosed a bundle. All tests pass fine.

# HG changeset patch
# User cfelton
# Date 1303212809 18000
# Node ID 145d558e234f363c3e43e9a79ab48ad7e20d7b9b
# Parent  f77cf2c4a5894013ceb2f5c184d84136ea4f04a3
Add a wrap parameter to intbv. When set, no overflow will occur.

diff -r f77cf2c4a589 -r 145d558e234f myhdl/_intbv.py
--- a/myhdl/_intbv.py	Wed May 04 21:33:14 2011 +0200
+++ b/myhdl/_intbv.py	Tue Apr 19 06:33:29 2011 -0500
@@ -31,37 +31,46 @@
 from __builtin__ import max as maxfunc

 class intbv(object):
-    __slots__ = ('_val', '_min', '_max', '_nrbits')
+    __slots__ = ('_min', '_max', '_nrbits', '_wrap', '__val')

-    def __init__(self, val=0, min=None, max=None, _nrbits=0):
+    def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):
         if _nrbits:
             self._min = 0
             self._max = 2**_nrbits
+            self._nrbits = _nrbits
         else:
             self._min = min
             self._max = max
             if max is not None and min is not None:
                 if min >= 0:
-                    _nrbits = len(bin(max-1))
+                    self._nrbits = len(bin(max-1))
                 elif max <= 1:
-                    _nrbits = len(bin(min))
+                    self._nrbits = len(bin(min))
                 else:
                     # make sure there is a leading zero bit in positive numbers
-                    _nrbits = maxfunc(len(bin(max-1))+1, len(bin(min)))
+                    self._nrbits = maxfunc(len(bin(max-1))+1, len(bin(min)))
+            else:
+                self._nrbits = 0
+
+        if wrap:
+            myrange = self._nrbits**2
+            if (abs(self.min) != self.max) or ((abs(self.min) +
self.max) != myrange):
+                raise ValueError, "Cannot use wrapping if -min != max
(%d != %d)" % (-self.min, self.max)
+        self._wrap  = wrap
         if isinstance(val, (int, long)):
             self._val = val
         elif isinstance(val, StringType):
             mval = val.replace('_', '')
+            self._nrbits = len(mval)
             self._val = long(mval, 2)
-            _nrbits = len(val)
         elif isinstance(val, intbv):
             self._val = val._val
             self._min = val._min
             self._max = val._max
-            _nrbits = val._nrbits
+            self._wrap = val._wrap
+            self._nrbits = val._nrbits
         else:
             raise TypeError("intbv constructor arg should be int or string")
-        self._nrbits = _nrbits
         self._checkBounds()

     # support for the 'min' and 'max' attribute
@@ -82,6 +91,20 @@
                 raise ValueError("intbv value %s < minimum %s" %
                                  (self._val, self._min))

+    # val property
+    def _get_val(self):
+        return self.__val
+
+    def _set_val(self, pval):
+
+        if self._wrap:
+            self.__val = self.wrap(pval)
+        else:
+            self.__val = pval
+
+        self._checkBounds()
+
+    _val = property(_get_val, _set_val)

     # hash
     def __hash__(self):
@@ -433,6 +456,21 @@
         return "intbv(" + repr(self._val) + ")"


+    def wrap(self, val):
+        """ Wrap the value back to the range of the inbv
+
+        The following will check that the defined min-max is the full
+        range of the binary word.  If the full range is specified if
+        the value is outside the bounds of the range it will be adjusted
+        to the proper value in bounds.
+        """
+        myrange = 2**self._nrbits
+
+        rval = self.min + ((val +self.max) % myrange)
+
+        return rval
+
+
     def signed(self):
       ''' return integer with the signed value of the intbv instance

diff -r f77cf2c4a589 -r 145d558e234f myhdl/test/core/test_intbv.py
--- a/myhdl/test/core/test_intbv.py	Wed May 04 21:33:14 2011 +0200
+++ b/myhdl/test/core/test_intbv.py	Tue Apr 19 06:33:29 2011 -0500
@@ -576,8 +576,50 @@
                 self.assertEqual(n.max, m.max)
                 self.assertEqual(len(n), len(m))

+class TestIntbvWrap(TestCase):
+
+    def testWrap(self):
+        x = intbv(0, min=-8, max=8, wrap=True)
+        x[:] = x + 1
+        self.assertEqual(1, x)
+        x[:] = x + 2
+        self.assertEqual(3, x)
+        x[:] = x + 5
+        self.assertEqual(-8, x)
+        x[:] = x + 1
+        self.assertEqual(-7, x)
+        x[:] = x - 5
+        self.assertEqual(4, x)
+        x[:] = x - 4
+        self.assertEqual(0, x)
+        x[:] += 15
+        x[:] = x - 1
+        self.assertEqual(-2, x)
+
+    def testInit(self):
+        self.assertRaises(ValueError, intbv, 15, min=-8, max=8)
+        x = intbv(15, min=-8, max=8, wrap=True)
+        self.assertEqual(-1, x)
+
+        self.assertRaises(ValueError, intbv, 5, min=-3, max=8, wrap=True)
+        intbv(5, min=-3, max=8)


+    def testNoWrap(self):
+        x = intbv(0, min=-8, max=8, wrap=False)
+        try:
+            x[:] += 15
+            self.fail()
+        except ValueError:
+            pass
+
+        x = intbv(0, min=-8, max=8)
+        try:
+            x[:] += 15
+            self.fail()
+        except ValueError:
+            pass
+
 if __name__ == "__main__":
     unittest.main()

Re: [myhdl-list] intbv wrap

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

On 5/5/2011 9:21 AM, Ben wrote:
> Hi Christopher,
>
> I took the liberty to review and edit your patch.
>
> As you can see, I added some tests, and removed the inclusion of the
> val property. It became quite smaller ! I left your name in it as the
> idea and the logic is from you.
>
> I also moved the check from the wrap function to the constructor (no
> need to check every time.).

Perfect!  Thanks for the contributions and for the additional test 
cases, very nice.  Also, good catch on the range checking!

Is there an opinion if 'myrange' should be a class member so that it 
doesn't need to be recalculated on each check?  I don't know if there is 
a down side to having too many slots (_range)?

In the future we will need to address if wrap() can be converted or 
changed to a private function?  As discussed in previous threads.

Thanks again Benoît.

Chris

>
> Here is a text version, enclosed a bundle. All tests pass fine.
>
> # HG changeset patch
> # User cfelton
> # Date 1303212809 18000
> # Node ID 145d558e234f363c3e43e9a79ab48ad7e20d7b9b
> # Parent  f77cf2c4a5894013ceb2f5c184d84136ea4f04a3
> Add a wrap parameter to intbv. When set, no overflow will occur.
>
> diff -r f77cf2c4a589 -r 145d558e234f myhdl/_intbv.py
> --- a/myhdl/_intbv.py	Wed May 04 21:33:14 2011 +0200
> +++ b/myhdl/_intbv.py	Tue Apr 19 06:33:29 2011 -0500
> @@ -31,37 +31,46 @@
>   from __builtin__ import max as maxfunc
>
>   class intbv(object):
> -    __slots__ = ('_val', '_min', '_max', '_nrbits')
> +    __slots__ = ('_min', '_max', '_nrbits', '_wrap', '__val')
>
> -    def __init__(self, val=0, min=None, max=None, _nrbits=0):
> +    def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):
>           if _nrbits:
>               self._min = 0
>               self._max = 2**_nrbits
> +            self._nrbits = _nrbits
>           else:
>               self._min = min
>               self._max = max
>               if max is not None and min is not None:
>                   if min>= 0:
> -                    _nrbits = len(bin(max-1))
> +                    self._nrbits = len(bin(max-1))
>                   elif max<= 1:
> -                    _nrbits = len(bin(min))
> +                    self._nrbits = len(bin(min))
>                   else:
>                       # make sure there is a leading zero bit in positive numbers
> -                    _nrbits = maxfunc(len(bin(max-1))+1, len(bin(min)))
> +                    self._nrbits = maxfunc(len(bin(max-1))+1, len(bin(min)))
> +            else:
> +                self._nrbits = 0
> +
> +        if wrap:
> +            myrange = self._nrbits**2
> +            if (abs(self.min) != self.max) or ((abs(self.min) +
> self.max) != myrange):
> +                raise ValueError, "Cannot use wrapping if -min != max
> (%d != %d)" % (-self.min, self.max)
> +        self._wrap  = wrap
>           if isinstance(val, (int, long)):
>               self._val = val
>           elif isinstance(val, StringType):
>               mval = val.replace('_', '')
> +            self._nrbits = len(mval)
>               self._val = long(mval, 2)
> -            _nrbits = len(val)
>           elif isinstance(val, intbv):
>               self._val = val._val
>               self._min = val._min
>               self._max = val._max
> -            _nrbits = val._nrbits
> +            self._wrap = val._wrap
> +            self._nrbits = val._nrbits
>           else:
>               raise TypeError("intbv constructor arg should be int or string")
> -        self._nrbits = _nrbits
>           self._checkBounds()
>
>       # support for the 'min' and 'max' attribute
> @@ -82,6 +91,20 @@
>                   raise ValueError("intbv value %s<  minimum %s" %
>                                    (self._val, self._min))
>
> +    # val property
> +    def _get_val(self):
> +        return self.__val
> +
> +    def _set_val(self, pval):
> +
> +        if self._wrap:
> +            self.__val = self.wrap(pval)
> +        else:
> +            self.__val = pval
> +
> +        self._checkBounds()
> +
> +    _val = property(_get_val, _set_val)
>
>       # hash
>       def __hash__(self):
> @@ -433,6 +456,21 @@
>           return "intbv(" + repr(self._val) + ")"
>
>
> +    def wrap(self, val):
> +        """ Wrap the value back to the range of the inbv
> +
> +        The following will check that the defined min-max is the full
> +        range of the binary word.  If the full range is specified if
> +        the value is outside the bounds of the range it will be adjusted
> +        to the proper value in bounds.
> +        """
> +        myrange = 2**self._nrbits
> +
> +        rval = self.min + ((val +self.max) % myrange)
> +
> +        return rval
> +
> +
>       def signed(self):
>         ''' return integer with the signed value of the intbv instance
>
> diff -r f77cf2c4a589 -r 145d558e234f myhdl/test/core/test_intbv.py
> --- a/myhdl/test/core/test_intbv.py	Wed May 04 21:33:14 2011 +0200
> +++ b/myhdl/test/core/test_intbv.py	Tue Apr 19 06:33:29 2011 -0500
> @@ -576,8 +576,50 @@
>                   self.assertEqual(n.max, m.max)
>                   self.assertEqual(len(n), len(m))
>
> +class TestIntbvWrap(TestCase):
> +
> +    def testWrap(self):
> +        x = intbv(0, min=-8, max=8, wrap=True)
> +        x[:] = x + 1
> +        self.assertEqual(1, x)
> +        x[:] = x + 2
> +        self.assertEqual(3, x)
> +        x[:] = x + 5
> +        self.assertEqual(-8, x)
> +        x[:] = x + 1
> +        self.assertEqual(-7, x)
> +        x[:] = x - 5
> +        self.assertEqual(4, x)
> +        x[:] = x - 4
> +        self.assertEqual(0, x)
> +        x[:] += 15
> +        x[:] = x - 1
> +        self.assertEqual(-2, x)
> +
> +    def testInit(self):
> +        self.assertRaises(ValueError, intbv, 15, min=-8, max=8)
> +        x = intbv(15, min=-8, max=8, wrap=True)
> +        self.assertEqual(-1, x)
> +
> +        self.assertRaises(ValueError, intbv, 5, min=-3, max=8, wrap=True)
> +        intbv(5, min=-3, max=8)
>
>
> +    def testNoWrap(self):
> +        x = intbv(0, min=-8, max=8, wrap=False)
> +        try:
> +            x[:] += 15
> +            self.fail()
> +        except ValueError:
> +            pass
> +
> +        x = intbv(0, min=-8, max=8)
> +        try:
> +            x[:] += 15
> +            self.fail()
> +        except ValueError:
> +            pass
> +
>   if __name__ == "__main__":
>       unittest.main()
>
>
>
> ------------------------------------------------------------------------------
> WhatsUp Gold - Download Free Network Management Software
> The most intuitive, comprehensive, and cost-effective network
> management toolset available today.  Delivers lowest initial
> acquisition cost and overall TCO of any competing solution.
> http://p.sf.net/sfu/whatsupgold-sd
>
>
>
> _______________________________________________
> myhdl-list mailing list
> myh...@li...
> https://lists.sourceforge.net/lists/listinfo/myhdl-list

Re: [myhdl-list] intbv wrap

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

I made a couple of minor adjustments to this patch.

   1. Made the wrap function private (_wrap) and moved _wrap to __wrap. 
  The wrap function should not be used directly because it will not 
covert correctly.  I think for a future (todo) enhancement the wrap 
function can be made public and handled by the converter.  This is an 
involved modification.  For now I think it is good to advertise the wrap 
function was private.

   2. Added myrange to a slot, _range.

   3. Modified a couple comments.

wrap4.patch is the text version.

Thanks
Chris Felton


On 5/5/11 4:25 PM, Christopher Felton wrote:
> On 5/5/2011 9:21 AM, Ben wrote:
>> Hi Christopher,
>>
>> I took the liberty to review and edit your patch.
>>
>> As you can see, I added some tests, and removed the inclusion of the
>> val property. It became quite smaller ! I left your name in it as the
>> idea and the logic is from you.
>>
>> I also moved the check from the wrap function to the constructor (no
>> need to check every time.).
>
> Perfect!  Thanks for the contributions and for the additional test
> cases, very nice.  Also, good catch on the range checking!
>
> Is there an opinion if 'myrange' should be a class member so that it
> doesn't need to be recalculated on each check?  I don't know if there is
> a down side to having too many slots (_range)?
>
> In the future we will need to address if wrap() can be converted or
> changed to a private function?  As discussed in previous threads.
>
> Thanks again Benoît.
>
> Chris
>
>>
>> Here is a text version, enclosed a bundle. All tests pass fine.
>>
>> # HG changeset patch
>> # User cfelton
>> # Date 1303212809 18000
>> # Node ID 145d558e234f363c3e43e9a79ab48ad7e20d7b9b
>> # Parent  f77cf2c4a5894013ceb2f5c184d84136ea4f04a3
>> Add a wrap parameter to intbv. When set, no overflow will occur.
>>
>> diff -r f77cf2c4a589 -r 145d558e234f myhdl/_intbv.py
>> --- a/myhdl/_intbv.py	Wed May 04 21:33:14 2011 +0200
>> +++ b/myhdl/_intbv.py	Tue Apr 19 06:33:29 2011 -0500
>> @@ -31,37 +31,46 @@
>>    from __builtin__ import max as maxfunc
>>
>>    class intbv(object):
>> -    __slots__ = ('_val', '_min', '_max', '_nrbits')
>> +    __slots__ = ('_min', '_max', '_nrbits', '_wrap', '__val')
>>
>> -    def __init__(self, val=0, min=None, max=None, _nrbits=0):
>> +    def __init__(self, val=0, min=None, max=None, wrap=False, _nrbits=0):
>>            if _nrbits:
>>                self._min = 0
>>                self._max = 2**_nrbits
>> +            self._nrbits = _nrbits
>>            else:
>>                self._min = min
>>                self._max = max
>>                if max is not None and min is not None:
>>                    if min>= 0:
>> -                    _nrbits = len(bin(max-1))
>> +                    self._nrbits = len(bin(max-1))
>>                    elif max<= 1:
>> -                    _nrbits = len(bin(min))
>> +                    self._nrbits = len(bin(min))
>>                    else:
>>                        # make sure there is a leading zero bit in positive numbers
>> -                    _nrbits = maxfunc(len(bin(max-1))+1, len(bin(min)))
>> +                    self._nrbits = maxfunc(len(bin(max-1))+1, len(bin(min)))
>> +            else:
>> +                self._nrbits = 0
>> +
>> +        if wrap:
>> +            myrange = self._nrbits**2
>> +            if (abs(self.min) != self.max) or ((abs(self.min) +
>> self.max) != myrange):
>> +                raise ValueError, "Cannot use wrapping if -min != max
>> (%d != %d)" % (-self.min, self.max)
>> +        self._wrap  = wrap
>>            if isinstance(val, (int, long)):
>>                self._val = val
>>            elif isinstance(val, StringType):
>>                mval = val.replace('_', '')
>> +            self._nrbits = len(mval)
>>                self._val = long(mval, 2)
>> -            _nrbits = len(val)
>>            elif isinstance(val, intbv):
>>                self._val = val._val
>>                self._min = val._min
>>                self._max = val._max
>> -            _nrbits = val._nrbits
>> +            self._wrap = val._wrap
>> +            self._nrbits = val._nrbits
>>            else:
>>                raise TypeError("intbv constructor arg should be int or string")
>> -        self._nrbits = _nrbits
>>            self._checkBounds()
>>
>>        # support for the 'min' and 'max' attribute
>> @@ -82,6 +91,20 @@
>>                    raise ValueError("intbv value %s<   minimum %s" %
>>                                     (self._val, self._min))
>>
>> +    # val property
>> +    def _get_val(self):
>> +        return self.__val
>> +
>> +    def _set_val(self, pval):
>> +
>> +        if self._wrap:
>> +            self.__val = self.wrap(pval)
>> +        else:
>> +            self.__val = pval
>> +
>> +        self._checkBounds()
>> +
>> +    _val = property(_get_val, _set_val)
>>
>>        # hash
>>        def __hash__(self):
>> @@ -433,6 +456,21 @@
>>            return "intbv(" + repr(self._val) + ")"
>>
>>
>> +    def wrap(self, val):
>> +        """ Wrap the value back to the range of the inbv
>> +
>> +        The following will check that the defined min-max is the full
>> +        range of the binary word.  If the full range is specified if
>> +        the value is outside the bounds of the range it will be adjusted
>> +        to the proper value in bounds.
>> +        """
>> +        myrange = 2**self._nrbits
>> +
>> +        rval = self.min + ((val +self.max) % myrange)
>> +
>> +        return rval
>> +
>> +
>>        def signed(self):
>>          ''' return integer with the signed value of the intbv instance
>>
>> diff -r f77cf2c4a589 -r 145d558e234f myhdl/test/core/test_intbv.py
>> --- a/myhdl/test/core/test_intbv.py	Wed May 04 21:33:14 2011 +0200
>> +++ b/myhdl/test/core/test_intbv.py	Tue Apr 19 06:33:29 2011 -0500
>> @@ -576,8 +576,50 @@
>>                    self.assertEqual(n.max, m.max)
>>                    self.assertEqual(len(n), len(m))
>>
>> +class TestIntbvWrap(TestCase):
>> +
>> +    def testWrap(self):
>> +        x = intbv(0, min=-8, max=8, wrap=True)
>> +        x[:] = x + 1
>> +        self.assertEqual(1, x)
>> +        x[:] = x + 2
>> +        self.assertEqual(3, x)
>> +        x[:] = x + 5
>> +        self.assertEqual(-8, x)
>> +        x[:] = x + 1
>> +        self.assertEqual(-7, x)
>> +        x[:] = x - 5
>> +        self.assertEqual(4, x)
>> +        x[:] = x - 4
>> +        self.assertEqual(0, x)
>> +        x[:] += 15
>> +        x[:] = x - 1
>> +        self.assertEqual(-2, x)
>> +
>> +    def testInit(self):
>> +        self.assertRaises(ValueError, intbv, 15, min=-8, max=8)
>> +        x = intbv(15, min=-8, max=8, wrap=True)
>> +        self.assertEqual(-1, x)
>> +
>> +        self.assertRaises(ValueError, intbv, 5, min=-3, max=8, wrap=True)
>> +        intbv(5, min=-3, max=8)
>>
>>
>> +    def testNoWrap(self):
>> +        x = intbv(0, min=-8, max=8, wrap=False)
>> +        try:
>> +            x[:] += 15
>> +            self.fail()
>> +        except ValueError:
>> +            pass
>> +
>> +        x = intbv(0, min=-8, max=8)
>> +        try:
>> +            x[:] += 15
>> +            self.fail()
>> +        except ValueError:
>> +            pass
>> +
>>    if __name__ == "__main__":
>>        unittest.main()
>>
>>
>>
>> ------------------------------------------------------------------------------
>> WhatsUp Gold - Download Free Network Management Software
>> The most intuitive, comprehensive, and cost-effective network
>> management toolset available today.  Delivers lowest initial
>> acquisition cost and overall TCO of any competing solution.
>> http://p.sf.net/sfu/whatsupgold-sd
>>
>>
>>
>> _______________________________________________
>> myhdl-list mailing list
>> myh...@li...
>> https://lists.sourceforge.net/lists/listinfo/myhdl-list
>
>
>
> ------------------------------------------------------------------------------
> WhatsUp Gold - Download Free Network Management Software
> The most intuitive, comprehensive, and cost-effective network
> management toolset available today.  Delivers lowest initial
> acquisition cost and overall TCO of any competing solution.
> http://p.sf.net/sfu/whatsupgold-sd

Re: [myhdl-list] intbv wrap

[Ben <ben...@gm...>]

Hi Chris,

I am following your struggle with this feature with great interest, as
I do think this would be an interesting addition to MyHDL, however, as
an hypothetical future extension to MyHDL, I think this would deserve
a MEP ... For now, If I want to understand your design decisions, I
have to dig into my mails and look for the relevant part. I for one
have for instance a question about the restrictions you enforce. See
below.

On Tue, May 10, 2011 at 13:27, Christopher Felton
<chr...@gm...> wrote:
> I made a couple of minor adjustments to this patch.
>
>  1. Made the wrap function private (_wrap) and moved _wrap to __wrap.  The
> wrap function should not be used directly because it will not covert
> correctly.  I think for a future (todo) enhancement the wrap function can be
> made public and handled by the converter.  This is an involved modification.
>  For now I think it is good to advertise the wrap function was private.

I don't really understand the reason of such an enforcement on min and
max for the wrapping. Let's be for a moment the devil's advocate and
say I want my intbv to be between -3 and 12. Why would MyHDL prevent
me to have it wrapped ?

>
>  2. Added myrange to a slot, _range.

This is a coding style decision. This _range is used at only one
place, and completely redundant with the existence of _nrbits. I don't
think it deserves a slot.

>
>  3. Modified a couple comments.
>

BTW, did you got the testsuite to run ? To run the "core" one, I don't
think you would need py.py. It uses unittest, which is included in
your python distribution.

Best Regards
Benoît

Re: [myhdl-list] intbv wrap

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

On 5/11/2011 7:31 AM, Ben wrote:
> Hi Chris,
>
> I am following your struggle with this feature with great interest, as
> I do think this would be an interesting addition to MyHDL, however, as
> an hypothetical future extension to MyHDL, I think this would deserve
> a MEP ... For now, If I want to understand your design decisions, I
> have to dig into my mails and look for the relevant part. I for one
> have for instance a question about the restrictions you enforce. See
> below.
Here is the original MEP I wrote.  It is not complete and has not been 
updated based on the conversations had here.  Also, this MEP simply 
exists in my user space right now.

http://www.myhdl.org/doku.php/user:cfelton:projects:wrap

I will try and update the MEP based on the latest developments when time 
permits.  More comment below.
>
> On Tue, May 10, 2011 at 13:27, Christopher Felton
> <chr...@gm...>  wrote:
>> I made a couple of minor adjustments to this patch.
>>
>>   1. Made the wrap function private (_wrap) and moved _wrap to __wrap.  The
>> wrap function should not be used directly because it will not covert
>> correctly.  I think for a future (todo) enhancement the wrap function can be
>> made public and handled by the converter.  This is an involved modification.
>>   For now I think it is good to advertise the wrap function was private.
>
> I don't really understand the reason of such an enforcement on min and
> max for the wrapping. Let's be for a moment the devil's advocate and
> say I want my intbv to be between -3 and 12. Why would MyHDL prevent
> me to have it wrapped ?

The reason is, we want to model natural behavior of a n-bit word.  An 
n-bit word automatically wraps, by truncating the most-significant bits, 
when assigned.  We want to prevent the automatic wrap in the example 
because additional logic is required.  This is the same for the 
saturate.  This enhancement is simply trying to add the natural 
occurring wrap of a n-bit binary word.  To model this behavior correctly 
the min and max need to be the full range of the binary word.

This is a little confusing because the intbv lets you assign arbitrary 
min and max for design checking (which is a good thing).  But note, this 
checking (bound checking) is only enforced during the design, MyHDL 
simulations.

To do the wrapping in your example, the logic would explicitly need to 
be added to the design.  IMO is good and why I think the wrap() will be 
public in the future once direct use of the function is convertible.

This enhancement is intended to handle the generic wrap case.  That 
should not be part of the intbv object.  But rather model the inherent 
wrapping of a bit-vector.

>
>>
>>   2. Added myrange to a slot, _range.
>
> This is a coding style decision. This _range is used at only one
> place, and completely redundant with the existence of _nrbits. I don't
> think it deserves a slot.
>
>>
>>   3. Modified a couple comments.
>>
>
> BTW, did you got the testsuite to run ? To run the "core" one, I don't
> think you would need py.py. It uses unittest, which is included in
> your python distribution.

Yes, your additions are perfect and IMO correct.  I reran the tests in 
the core test suite and they are part of the patch.  Thanks again for 
doing this work!

>
> Best Regards
> Benoît
>
> ------------------------------------------------------------------------------
> Achieve unprecedented app performance and reliability
> What every C/C++ and Fortran developer should know.
> Learn how Intel has extended the reach of its next-generation tools
> to help boost performance applications - inlcuding clusters.
> http://p.sf.net/sfu/intel-dev2devmay

Re: [myhdl-list] intbv wrap

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

On 05/11/2011 04:01 PM, Christopher Felton wrote:
>
> On 5/11/2011 7:31 AM, Ben wrote:
>> Hi Chris,
>>
>> I am following your struggle with this feature with great interest, as
>> I do think this would be an interesting addition to MyHDL, however, as
>> an hypothetical future extension to MyHDL, I think this would deserve
>> a MEP ... For now, If I want to understand your design decisions, I
>> have to dig into my mails and look for the relevant part. I for one
>> have for instance a question about the restrictions you enforce. See
>> below.
> Here is the original MEP I wrote.  It is not complete and has not been
> updated based on the conversations had here.  Also, this MEP simply
> exists in my user space right now.
>
> http://www.myhdl.org/doku.php/user:cfelton:projects:wrap
>
> I will try and update the MEP based on the latest developments when time
> permits.  More comment below.
>>
>> On Tue, May 10, 2011 at 13:27, Christopher Felton
>> <chr...@gm...>   wrote:
>>> I made a couple of minor adjustments to this patch.
>>>
>>>    1. Made the wrap function private (_wrap) and moved _wrap to __wrap.  The
>>> wrap function should not be used directly because it will not covert
>>> correctly.  I think for a future (todo) enhancement the wrap function can be
>>> made public and handled by the converter.  This is an involved modification.
>>>    For now I think it is good to advertise the wrap function was private.
>>
>> I don't really understand the reason of such an enforcement on min and
>> max for the wrapping. Let's be for a moment the devil's advocate and
>> say I want my intbv to be between -3 and 12. Why would MyHDL prevent
>> me to have it wrapped ?
>
> The reason is, we want to model natural behavior of a n-bit word.  An
> n-bit word automatically wraps, by truncating the most-significant bits,
> when assigned.  We want to prevent the automatic wrap in the example
> because additional logic is required.  This is the same for the
> saturate.  This enhancement is simply trying to add the natural
> occurring wrap of a n-bit binary word.  To model this behavior correctly
> the min and max need to be the full range of the binary word.

What is "natural" depends entirely on your perspective. If your
perspective is representation, I agree. But if your perspective is
modeling in terms of modulo arithmetic, the restrictions would be
incomprehensible.

I really think MyHDL should make a difference with Verilog/VHDL here.
We should concentrate on modeling first, and only then on
conversion/synthesizability. As the intbv example shows, sometimes
there isn't even a price to pay! Moreover, we can always impose
additional restrictions for convertiblity, but we may also find
ways to implement more powerful solutions than with Verilog/VHDL.

>
> This is a little confusing because the intbv lets you assign arbitrary
> min and max for design checking (which is a good thing).  But note, this
> checking (bound checking) is only enforced during the design, MyHDL
> simulations.
>
> To do the wrapping in your example, the logic would explicitly need to
> be added to the design.  IMO is good and why I think the wrap() will be
> public in the future once direct use of the function is convertible.
>
> This enhancement is intended to handle the generic wrap case.  That
> should not be part of the intbv object.  But rather model the inherent
> wrapping of a bit-vector.
>
>>
>>>
>>>    2. Added myrange to a slot, _range.
>>
>> This is a coding style decision. This _range is used at only one
>> place, and completely redundant with the existence of _nrbits. I don't
>> think it deserves a slot.
>>
>>>
>>>    3. Modified a couple comments.
>>>
>>
>> BTW, did you got the testsuite to run ? To run the "core" one, I don't
>> think you would need py.py. It uses unittest, which is included in
>> your python distribution.
>
> Yes, your additions are perfect and IMO correct.  I reran the tests in
> the core test suite and they are part of the patch.  Thanks again for
> doing this work!
>
>>
>> Best Regards
>> Benoît
>>
>> ------------------------------------------------------------------------------
>> Achieve unprecedented app performance and reliability
>> What every C/C++ and Fortran developer should know.
>> Learn how Intel has extended the reach of its next-generation tools
>> to help boost performance applications - inlcuding clusters.
>> http://p.sf.net/sfu/intel-dev2devmay
>
>
>
> ------------------------------------------------------------------------------
> Achieve unprecedented app performance and reliability
> What every C/C++ and Fortran developer should know.
> Learn how Intel has extended the reach of its next-generation tools
> to help boost performance applications - inlcuding clusters.
> http://p.sf.net/sfu/intel-dev2devmay


-- 
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
     Analog design automation: http://www.mephisto-da.com
     World-class digital design: http://www.easics.com

Re: [myhdl-list] intbv wrap

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

On 5/11/2011 9:54 AM, Jan Decaluwe wrote:
> On 05/11/2011 04:01 PM, Christopher Felton wrote:
>>
>> On 5/11/2011 7:31 AM, Ben wrote:
>>> Hi Chris,
>>>
>>> I am following your struggle with this feature with great interest, as
>>> I do think this would be an interesting addition to MyHDL, however, as
>>> an hypothetical future extension to MyHDL, I think this would deserve
>>> a MEP ... For now, If I want to understand your design decisions, I
>>> have to dig into my mails and look for the relevant part. I for one
>>> have for instance a question about the restrictions you enforce. See
>>> below.
>> Here is the original MEP I wrote.  It is not complete and has not been
>> updated based on the conversations had here.  Also, this MEP simply
>> exists in my user space right now.
>>
>> http://www.myhdl.org/doku.php/user:cfelton:projects:wrap
>>
>> I will try and update the MEP based on the latest developments when time
>> permits.  More comment below.
>>>
>>> On Tue, May 10, 2011 at 13:27, Christopher Felton
>>> <chr...@gm...>    wrote:
>>>> I made a couple of minor adjustments to this patch.
>>>>
>>>>     1. Made the wrap function private (_wrap) and moved _wrap to __wrap.  The
>>>> wrap function should not be used directly because it will not covert
>>>> correctly.  I think for a future (todo) enhancement the wrap function can be
>>>> made public and handled by the converter.  This is an involved modification.
>>>>     For now I think it is good to advertise the wrap function was private.
>>>
>>> I don't really understand the reason of such an enforcement on min and
>>> max for the wrapping. Let's be for a moment the devil's advocate and
>>> say I want my intbv to be between -3 and 12. Why would MyHDL prevent
>>> me to have it wrapped ?
>>
>> The reason is, we want to model natural behavior of a n-bit word.  An
>> n-bit word automatically wraps, by truncating the most-significant bits,
>> when assigned.  We want to prevent the automatic wrap in the example
>> because additional logic is required.  This is the same for the
>> saturate.  This enhancement is simply trying to add the natural
>> occurring wrap of a n-bit binary word.  To model this behavior correctly
>> the min and max need to be the full range of the binary word.
>
> What is "natural" depends entirely on your perspective. If your
> perspective is representation, I agree. But if your perspective is
> modeling in terms of modulo arithmetic, the restrictions would be
> incomprehensible.

So, "natural" was a wrong choice of words, inherent should have been used.

>
> I really think MyHDL should make a difference with Verilog/VHDL here.
> We should concentrate on modeling first, and only then on
> conversion/synthesizability. As the intbv example shows, sometimes
> there isn't even a price to pay! Moreover, we can always impose
> additional restrictions for convertiblity, but we may also find
> ways to implement more powerful solutions than with Verilog/VHDL.

In general I think this is a good approach, adding more modeling 
features.  But I think MyHDL already provides kick-ass "framework" for 
building all kinds of modeling.  If someone wanted an _interesting_ 
behavior, a derived class with the intbv as the base class, can easily 
be created.  And the user knows this will *not* be convertible (some 
cases it will be convertible).

I am not sure about adding non-convertible features to the intbv class. 
  I think it could be confusing.

I would rather see an approach where a derived class with the new 
modeling is added; to simplify use for developers.  And when or if, it 
has a clear path for conversion then it can be added to the intbv class. 
  I think this would limit confusion and disappointment when trying to 
convert a design.  Also, it adds a little agility by adding classes for 
just for modeling.  These classes can be documented in the manual, etc.

In my mind, the current wrap implementation would be added to the intbv 
to capture the inherent behavior of an n-bit word.  And the arbitrary 
wrapping could be added to an enhanced modeling class.

Chris

Re: [myhdl-list] intbv wrap

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

On 05/11/2011 02:31 PM, Ben wrote:
> Hi Chris,
>
> I am following your struggle with this feature with great interest, as
> I do think this would be an interesting addition to MyHDL, however, as
> an hypothetical future extension to MyHDL, I think this would deserve
> a MEP ... For now, If I want to understand your design decisions, I
> have to dig into my mails and look for the relevant part. I for one
> have for instance a question about the restrictions you enforce. See
> below.
>
> On Tue, May 10, 2011 at 13:27, Christopher Felton
> <chr...@gm...>  wrote:
>> I made a couple of minor adjustments to this patch.
>>
>>   1. Made the wrap function private (_wrap) and moved _wrap to __wrap.  The
>> wrap funct ion should not be used directly because it will not covert
>> correctly.  I think for a future (todo) enhancement the wrap function can be
>> made public and handled by the converter.  This is an involved modification.
>>   For now I think it is good to advertise the wrap function was private.
>
> I don't really understand the reason of such an enforcement on min and
> max for the wrapping. Let's be for a moment the devil's advocate and
> say I want my intbv to be between -3 and 12. Why would MyHDL prevent
> me to have it wrapped ?

In the past days, I have reviewed and thought about all the proposals,
and that is exactly my most important critique. As in the intbv case,
we should consider modeling requirements first, and "efficiency" only
later. (The case of the intbv shows that sometimes you can have easier
modeling without an efficiency price to pay.)

I have also reviewed the concept behind Ada modular types more thorougly,
and I really think this is the way to follow.

I am working on an extensive review and proposal, that I will post
shortly.

>
>>
>>   2. Added myrange to a slot, _range.
>
> This is a coding style decision. This _range is used at only one
> place, and completely redundant with the existence of _nrbits. I don't
> think it deserves a slot.
>
>>
>>   3. Modified a couple comments.
>>
>
> BTW, did you got the testsuite to run ? To run the "core" one, I don't
> think you would need py.py. It uses unittest, which is included in
> your python distribution.
>
> Best Regards
> Benoît
>
> ------------------------------------------------------------------------------
> Achieve unprecedented app performance and reliability
> What every C/C++ and Fortran developer should know.
> Learn how Intel has extended the reach of its next-generation tools
> to help boost performance applications - inlcuding clusters.
> http://p.sf.net/sfu/intel-dev2devmay


-- 
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
     Analog design automation: http://www.mephisto-da.com
     World-class digital design: http://www.easics.com