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.