myhdl-list

[myhdl-list] [PATCH 0 of 2 V3] Add support for negative indexes _and_ slice limits

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

This third version of this patch series adds support for converting slices with negative limits.

The only limitation (other than the ones that also apply for indexes) is that you cannot mix positive and negative limits on the same slice.

That is, this will convert fine:

    s_my_signal[-1:-4]

But this will not work right:

    s_my_signal[-1:3]

This is because the size inference does not work right in this last case.

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

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

# 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
             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:

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

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

# HG changeset patch
# User Angel Ezquerra <ang...@gm...>
# Date 1350086923 -7200
# Branch 0.8-dev
# Node ID 2ec0657ef6dbb262b3fc5fed0f9d4f30f82f528b
# Parent  c641c1ed333d6d03d9cee4a63df5111fe98574c8
toVHDL: add support for negative indexes and slice limits

This patch tries to detect negative indexes and slice limits and converts them
into indexes or limits relative to the highest bit position on the corresponding
bit vector (with -1 corresponding to the highest bit).

This is similar to what Python does with negative indexes for regular lists.

Two types of "negative indexes" are supported with this patch:

- Explicit negative numbers (e.g. -1, -3, etc)
- Explicit negative unary operator surrounding any other operation
(e.g. -(2 * 3))

Other cases in which the index may be negative are not detected. Despite this
limitation I think that this functionality can be quite useful.

As an example, the following simple test case converts fine with this patch:

~~~
#!/usr/env python
from myhdl import *

def test_module(rst, clk):
    '''This block generates the radio frame synchronization signal'''
    s_output = Signal(intbv(0)[16:])
    s_input = Signal(intbv(0)[16:])

    @always(clk.posedge, rst.posedge)
    def p_process():
        if rst == 1:
            s_input[-1] = 1
            s_input[-(3*2)] = 1
            s_output.next[-2] = s_input[-3]
            s_input[-1:-3] = 1
            s_input[-8:-14] = 1
        else:
            pass


    return instances()

clk, rst = [Signal(bool()) for n in range(2)]

if __name__ == '__main__':
    sync_inst = toVHDL(test_module3, rst, clk)
~~~

* ISSUES:

Mixing negative and positive limits on a single slice does not work fine. The
conversion side works ok, but the slice size inference is wrong.

diff --git a/myhdl/conversion/_toVHDL.py b/myhdl/conversion/_toVHDL.py
--- a/myhdl/conversion/_toVHDL.py
+++ b/myhdl/conversion/_toVHDL.py
@@ -1241,6 +1241,15 @@
         else:
             self.accessIndex(node)
 
+    def _isnegativeindex(self, astvalue):
+        if isinstance(astvalue, ast.Num):
+            if astvalue.value < 0:
+                return True
+        elif isinstance(astvalue, ast.UnaryOp):
+            if isinstance(astvalue.op, ast.USub):
+                return True
+        return False
+
     def accessSlice(self, node):
         if isinstance(node.value, ast.Call) and \
            node.value.func.obj in (intbv, modbv) and \
@@ -1278,11 +1287,17 @@
         if lower is None:
             self.write("%s" % node.obj._nrbits)
         else:
+            if self._isnegativeindex(lower):
+                # convert negative indexes into indexes from the MSB
+                self.write('%d + ' % node.value.obj._nrbits)
             self.visit(lower)
         self.write("-1 downto ")
         if upper is None:
             self.write("0")
         else:
+            if self._isnegativeindex(upper):
+                # convert negative indexes into indexes from the MSB
+                self.write('%d + ' % node.value.obj._nrbits)
             self.visit(upper)
         self.write(")")
         self.write(suf)
@@ -1293,6 +1308,11 @@
         self.visit(node.value)
         self.write("(")
         #assert len(node.subs) == 1
+        
+        if self._isnegativeindex(node.slice.value):
+            # convert negative indexes into indexes from the MSB
+            self.write('%d + ' % node.value.obj._nrbits)
+
         self.visit(node.slice.value)
         self.write(")")
         self.write(suf)

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

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

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
>

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