Re: [myhdl-list] [PATCH 1 of 2 V3] intbv: add support for negative indexes and slice limits

[Angel E. <ang...@gm...>]

Thank you for looking into this, Benoit,

I think this feature is interesting to be consistent with how regular
Python lists work. This is something that I expected to work out of
the box and I was surprised when it didn't. I also think it highlights
how working with a higher language such as Python can make it possible
to do things which are very hard or tedious using a lower level
language such as VHDL.

I hope Jan can comment on whether he thinks this is a good idea or
not. In any case it was a good opportunity for me to look at the
conversion code.

Regarding the tests, I did run the non conversion tests and they did
work fine. I agree that I should add some tests. I can add regular,
non-conversion tests for the first patch in the series, but adding the
conversion tests would be harder since I cannot run them! :-(

As for your comments regarding the code I agree that the error message
should be improved. I had it on my TODO but I forgot about it.

Once I get more feedback I'll change that along with any other issue
that my arise.

I think this feature should be added to Verilog as well, but I have no
experience with Verilog...

Cheers,

Angel


On Mon, Oct 15, 2012 at 12:17 PM, Ben <ben...@gm...> wrote:
> Hi Angel,
>
> Other than a preliminary discussion on the topic, and about why this
> feature is interesting, this patch should also greatly benefit from
> tests.
>
> While I wasn't able to get the conversion tests running on my system
> either (vlog, vcom, ... missing), the "core" one do work fine (once
> you installed pytest, and set the PYTHONPATH in the Makefiles). Could
> you look at this ?
>
> Stylistic remark inlined.
>
> Regards,
> Benoît.
>
> On Sun, Oct 14, 2012 at 12:54 AM, Angel Ezquerra
> <ang...@gm...> wrote:
>> # HG changeset patch
>> # User Angel Ezquerra <ang...@gm...>
>> # Date 1350077846 -7200
>> # Branch 0.8-dev
>> # Node ID c641c1ed333d6d03d9cee4a63df5111fe98574c8
>> # Parent  6f6571dbd495f197531f0406763ff10e4ce3b3ee
>> intbv: add support for negative indexes and slice limits
>>
>> Negative indexes are supported on regular python lists. They are very useful
>> when you want to refer to a position from the end of the list. This same concept
>> can be applied to intbv indexes and slices. Supporting negative indexes reduces
>> the need to use the len() function to calculate a position from the MSB of the
>> intbv.
>>
>> Note that converting negative indexes is not supported yet.
>>
>> diff --git a/myhdl/_intbv.py b/myhdl/_intbv.py
>> --- a/myhdl/_intbv.py
>> +++ b/myhdl/_intbv.py
>> @@ -127,11 +127,15 @@
>>                  j = 0
>>              j = int(j)
>>              if j < 0:
>> -                raise ValueError, "intbv[i:j] requires j >= 0\n" \
>> -                      "            j == %s" % j
>> +                j = self._nrbits + j
>> +                if j < 0:
>> +                    raise ValueError, "intbv[i:j] requires j >= 0\n" \
>> +                        "            j == %s" % j
>
> The extra indent is not needed over there, and the error message could
> benefit from clarification.
>
>>              if i is None: # default
>>                  return type(self)(self._val >> j)
>>              i = int(i)
>> +            if i < 0:
>> +                i = self._nrbits + i
>>              if i <= j:
>>                  raise ValueError, "intbv[i:j] requires i > j\n" \
>>                        "            i, j == %s, %s" % (i, j)
>> @@ -139,6 +143,8 @@
>>              return res
>>          else:
>>              i = int(key)
>> +            if i < 0:
>> +                i = self._nrbits + i
>>              res = bool((self._val >> i) & 0x1)
>>              return res
>>
>> @@ -153,14 +159,18 @@
>>                  j = 0
>>              j = int(j)
>>              if j < 0:
>> -                raise ValueError, "intbv[i:j] = v requires j >= 0\n" \
>> -                      "            j == %s" % j
>> +                j = self._nrbits + j
>> +                if j < 0:
>> +                    raise ValueError, "intbv[i:j] = v requires j >= 0\n" \
>> +                        "            j == %s" % j
>>              if i is None: # default
>>                  q = self._val % (1L << j)
>>                  self._val = val * (1L << j) + q
>>                  self._handleBounds()
>>                  return
>>              i = int(i)
>> +            if i < 0:
>> +                i = self._nrbits + i
>>              if i <= j:
>>                  raise ValueError, "intbv[i:j] = v requires i > j\n" \
>>                        "            i, j, v == %s, %s, %s" % (i, j, val)
>> @@ -174,6 +184,8 @@
>>              self._handleBounds()
>>          else:
>>              i = int(key)
>> +            if i < 0:
>> +                i = self._nrbits + i
>>              if val == 1:
>>                  self._val |= (1L << i)
>>              elif val == 0:
>>
>> ------------------------------------------------------------------------------
>> Don't let slow site performance ruin your business. Deploy New Relic APM
>> Deploy New Relic app performance management and know exactly
>> what is happening inside your Ruby, Python, PHP, Java, and .NET app
>> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
>> http://p.sf.net/sfu/newrelic-dev2dev
>> _______________________________________________
>> myhdl-list mailing list
>> myh...@li...
>> https://lists.sourceforge.net/lists/listinfo/myhdl-list
>>
>
> ------------------------------------------------------------------------------
> Don't let slow site performance ruin your business. Deploy New Relic APM
> Deploy New Relic app performance management and know exactly
> what is happening inside your Ruby, Python, PHP, Java, and .NET app
> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
> http://p.sf.net/sfu/newrelic-dev2dev
> _______________________________________________
> myhdl-list mailing list
> myh...@li...
> https://lists.sourceforge.net/lists/listinfo/myhdl-list