Add support for nested functions


#1

Hi,

unlike mentioned by josyb here MyHDL’s converters do not seem to support nested functions. Example code:

from __future__ import print_function
from myhdl import *

def State_add(x, y):
    return x + y

def State_update(inState, inValue):
    # # Boring: works as expected
    # return inState + inValue
    # FIXME: MyHDL: AttributeError: '_ConvertFunctionVisitor' object has no attribute 'funcBuf'
    return State_add(inState, inValue)

@block
def Funcs(ioState, inValue, reset):
    @instance
    def logic():
        while True:
            yield ioState, inValue, reset

            if reset != 1:
                ioState.next = modbv(0x00)[32:]
            else:
                ioState.next = State_update(ioState, inValue)

    return logic

inst = Funcs(
    ioState = Signal(intbv(0)[32:]),
    inValue = Signal(intbv(0)[32:]),
    reset   = Signal(bool(1))
)

inst.convert(hdl='VHDL', path='work')
inst.convert(hdl='Verilog', path='work', no_testbench=True)

The commented-out version in State_update() works but not if I call another function State_add(). Exception:

/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py:360: ToVHDLWarning: Output port is read internally: ioState
  category=ToVHDLWarning
Traceback (most recent call last):
  File "./Funcs.py", line 37, in <module>
    inst.convert(hdl='VHDL', path='work')
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/_block.py", line 276, in convert
    return converter(self)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 244, in __call__
    _convertGens(genlist, siglist, memlist, vfile)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 543, in _convertGens
    v.visit(tree)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1339, in visit_Module
    self.visit(stmt)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1650, in visit_FunctionDef
    self.visit(stmt)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1263, in visit_If
    self.mapToIf(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1329, in mapToIf
    self.visit_stmt(node.else_)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1532, in visit_stmt
    self.visit(stmt)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 981, in visit_Assign
    self.visit(rhs)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1113, in visit_Call
    v.visit(node.tree)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1339, in visit_Module
    self.visit(stmt)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1909, in visit_FunctionDef
    self.visit_stmt(node.body)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1532, in visit_stmt
    self.visit(stmt)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1918, in visit_Return
    self.visit(node.value)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/Volumes/Data/Work/FastCloud/src/__bug/myhdl/conversion/_toVHDL.py", line 1112, in visit_Call
    v = Visitor(node.tree, self.funcBuf)
AttributeError: '_ConvertFunctionVisitor' object has no attribute 'funcBuf'

That feature might be really useful in more complex designs allowing to reuse (python-)code. Can that be implemented? I’d offer to have a look at it. How difficult would it be? I have no experience with the MyHDL internals so far…

ciao,
Mario


#2

Hi Mario,

We didn’t discuss nested functions in the thread you mention, did we?
I rarely used functions in VHDL as you describe here, and I certainly didn’t nest them. So far I haven’t hit this issue in MyHDL either, yet.
I have no real idea how difficult it would be to implement in MyHDL. At the beginning and the end it looks easy, but there is always a lot of work in between :smile:

Regards,
Josy


#3

Hi Mario,

I couldn’t help, and had a quick look into my MyHDL code.
And backported your code to the pre-@block stage I currently use locally.

from __future__ import print_function
from myhdl import always_comb, Signal, intbv, modbv, toVHDL

def State_add(x, y):
    return x + y

def State_update(inState, inValue):
    # # Boring: works as expected
    #     return inState + inValue
    # FIXME: MyHDL: AttributeError: '_ConvertFunctionVisitor' object has no attribute 'funcBuf'
    newstate = State_add(inState, inValue)
    return newstate


def Funcs(ioState, inValue, reset):

    @always_comb
    def logic():
        if reset != 1:
            # modbv(0x00)[32:]
            ioState.next = 0
        else:
            ioState.next = State_update(ioState, inValue)

    return logic

def convert():
    ioState = Signal(intbv(0)[32:])
    inValue = Signal(intbv(0)[32:])
    reset = Signal(bool(1))

    toVHDL(Funcs, ioState, inValue, reset)

if __name__ == '__main__':
    convert()

I get a different error though, but it eventually boils down to MyHDL not expecting you to nest function calls.
I could modify my MyHDL code to digest the nested function call, but _toVHDL then writes the inner nested function call inside the outer function call:

	function State_update(
		inState: in unsigned;
		inValue: in unsigned) return integer is
		variable newstate: integer;
	begin
		newstate := State_add(inState, inValue)	function State_add(
		x: in unsigned;
		y: in unsigned) return integer is
	begin
		return to_integer(x + y);
	end function State_add;

	;
		return newstate;
	end function State_update;

I guess it will take a couple of hours to fix it, but remember : at the beginning …
I don’t have the time either …

Regards,

Josy


#4

Hi Josy,

sorry for having to backport the code, but I’m used to MyHDL trunk as suggested in another post.
And sorry for causing confusion. The question was indeed about nested function calls not definitions (which is rarely useful in any programming language).

However, the assertion is caused by the fact that the base class _ConvertVisitor (line 1112) tries to access an attribute funcBuf which it does not define itself. As far as I understand the code, most sub-classes define it, but _ConvertFunctionVisitor does not. Instead it passes the funcBuf to its base class constructor (stored there as buf). Unfortunately, I don’t understand the meaning of those buffer objects yet. Does anyone know the the translation works in general? And yes: Also my attempts resulted in both functions being mangled together.

PS: Generally speaking, it’s bad practice to access attributes defined in sub-classes in base class code.

ciao,
Mario


#5

Mario,

The backport is my problem because I haven’t merged the @block code in my local MyHDL code. I am also not happy that that will break all my RTL code :disappointed:

I did see that funcBuf behaviour but didn’t have a quick solution, not one in that 'stolen’half-hour :slight_smile:
Basically we need to make a list of funcBuf, one for every ‘function’ and play them back reversed.
At the moment toVHDL creates a unique function for every call, something I have changed in my local code.

Now I didn’t write the original code, so no offence taken :slight_smile:

Regards,

Josy