Unable to use concat with InPort

[KelvinChung2000]

The following example,

from pymtl3 import *


class MasterIfc(Interface):
    def construct(s):
        s.in_ = InPort(b2)


class Test(Component):
    def construct(s):
        s.ins_ = [MasterIfc() for _ in range(4)]
        s.out = OutPort(b8)
        s.out //= concat(s.ins_[0].in_, s.ins_[1].in_, s.ins_[2].in_, s.ins_[3].in_)


if __name__ == "__main__":
    from pymtl3.passes.backends.verilog import VerilogTranslationPass

    m = Test()
    m.elaborate()
    m.set_metadata(VerilogTranslationPass.enable, True)
    m.set_metadata(VerilogTranslationPass.explicit_file_name, "Test.sv")
    m.apply(VerilogTranslationPass())
    m.apply(DefaultPassGroup())

is giving me

  File "/home/kelvin/pymtl3/pymtl3/dsl/Connectable.py", line 184, in __getattr__
    raise AttributeError("{} is not a signals with struct type, and has no attribute '{}'".format( s, name ))
AttributeError: s.ins_[0].in_ is not a signals with struct type, and has no attribute 'nbits'

Is there any way I can work around this?

[cbatten]

How does this relate to your recent pull request?

• Improving concat #278

I am not so sure about this general trend ... I think in general concat is meant for use inside an update block right? Does it work with //= out of the box? This seems like a slippery slope ... should we also make zext and sext work with //= ... we did have support for a lambda with //= like this:

  s.out //= lambda: concat( s.a, s.b )

But in retrospect I think it is just not worth it. I kind of think we should deprecate lambda's with //= ... indeed I think we should maybe just deprecate //= completely and just rely on connect ... thoughts?

[cbatten]

@ptpan @yo96 @jsn1993 thoughts?

[KelvinChung2000]

I want to achieve something like the following:

class MasterIfc(Interface):
    def construct(s):
        s.in_ = InPort(b2)


class Test(Component):
    def construct(s, n):
        s.ins_ = [MasterIfc() for _ in range(n)]
        s.out = OutPort(b8)
        s.out //= concat(i.in_ for i in s.ins_)

I have tried to use s.out //= lambda: concat( i.in_ for i in s.ins_ ) but this will causes the following error with my pull request

  File "/home/kelvin/pymtl3/pymtl3/passes/rtlir/behavioral/BehavioralRTLIRGenL1Pass.py", line 503, in visit_GeneratorExp
    raise PyMTLSyntaxError( s.blk, node, 'invalid operation: generator expression' )
pymtl3.passes.rtlir.errors.PyMTLSyntaxError: 
In file /home/kelvin/fullSystemDesign/_lambda__s_out, Line 1, Col 24:
  def _lambda__s_out():

I would say doing s.out //= lambda: concat( s.a, s.b ) is equivalent to assign out = {a, b};

[cbatten]

I think there are many different questions/issues here.

First, is does concat work with //= normally ... does this work?

s.out //= concat( s.a, s.b )

I am actually not sure ...

Second, is should we support list comprehensions inside a concat. I think these are orthogonal issues because I don't think this works either?

@update
def comb():
  s.out @= concat( i.in for i in s.ins_ )

Third, what does it mean to concat interfaces? I am not sure that is reasonable ... interfaces can and do include ports going in different directions so you cannot concatenate them ...

[KelvinChung2000]

This works (with my pull request):

 s.out //= concat(b2(2) for i in range(4))

This does not:

s.ins2_ = InPort(b2)
s.out //= concat(s.ins2_ for i in range(4))

I think concat on an interface does not make sense since it is difficult to tell how concat aligns the interface. However, I think contacting signals within the interface makes sense, as it is common to pick out specific signals within the interface to do specific operations.

[yo96]

How does this relate to your recent pull request?

• Improving concat #278

I am not so sure about this general trend ... I think in general concat is meant for use inside an update block right? Does it work with //= out of the box? This seems like a slippery slope ... should we also make zext and sext work with //= ... we did have support for a lambda with //= like this:

  s.out //= lambda: concat( s.a, s.b )

But in retrospect I think it is just not worth it. I kind of think we should deprecate lambda's with //= ... indeed I think we should maybe just deprecate //= completely and just rely on connect ... thoughts?

Lambda with //= corresponds to the assign state in Verilog. As I remember it was very challenging to support connecting to an arbitrary statement without the lambda.

[KelvinChung2000]

Is the limitation caused by an older version of Python or something else?

[cbatten]

It is really just a constraint of the underlying framework ... //= was always supposed to be syntactic sugar for connect ... but then we figured out how use a lambda to create an update block on the fly which seemed cool ... but in retrospect it just adds complexity .... so I really do not think we can support this syntax:

  s.out //= concat( s.a, s.b )

[KelvinChung2000]

However with

class MasterIfc(Interface):
    def construct(s):
        s.in_ = InPort(b2)


class Test(Component):
    def construct(s):
        s.ins_ = [MasterIfc() for _ in range(4)]
        s.out = OutPort(b8)
        connect(
            s.out, concat(s.ins_[0].in_, s.ins_[1].in_, s.ins_[2].in_, s.ins_[3].in_)
        )

It is still not working at the moment and yielding the following error:

  File "/home/kelvin/pymtl3/pymtl3/dsl/Connectable.py", line 184, in __getattr__
    raise AttributeError("{} is not a signals with struct type, and has no attribute '{}'".format( s, name ))
AttributeError: s.ins_[0].in_ is not a signals with struct type, and has no attribute 'nbits'

I like the //= syntax as it is simple and makes sense. It shows the direction of the assignment as well, which feels similar to assign a = b;

[KelvinChung2000]

Will the recent Typing system help with the problem? Pymtl will perform type-checking to force things to be evaluated based on the given type.

[cbatten]

no ... the problem is you would have to create an update block on the fly to do the concat operation ... it would be very hard to do ...

[KelvinChung2000]

If I understand correctly, the on-the-fly update block creation might be difficult to support in a pure Python simulation environment. If that's the problem, is there a way for me to bypass this constraint and go for a verilator-based simulation or any other way such I can describe what I would like to write rather than something like the following?

class Test(Component):
    def construct(s):
        s.ins_ = [MasterIfc() for _ in range(4)]
        s.out = OutPort(b8)

        for c, i in enumerate(s.ins_):
            b = c * i.in_.get_type().nbits
            w = i.in_.get_type().nbits
            s.out[b : b + w] //= s.ins_[c].in_

[cbatten]

Actually it is not really even the pure-Python simulation part ... the RTL translation pass does not parse //= statements ... the pass works on the elaborated model after all //= have turned into structural connections ...

Maybe you can create your own custom connect function? That is definitely doable and very much in the spirit of what you are trying to do here ... you can use a function to encapsulate this connection operation.

def connect_concat_master_ifc( out, ifcs ):
  for c, i in enumerate(ifcs):
    b = c * i.in_.get_type().nbits
    w = i.in_.get_type().nbits
    connect( out[b : b + w], ifcs[c].in_ )

maybe something like that? Not sure if that will work though ...

[KelvinChung2000]

I will also need to add something to search for specific signals within the interface I want to connect. So I will need something like:

class MasterIfc(Interface):
    def construct(s):
        s.in_ = InPort(b2)
        s.in_2 = InPort(b2)


def ifcConcat(out, ifc, sig):
    for c, i in enumerate(ifc):
        ifcSig = getattr(i, sig)
        b = c * ifcSig.get_type().nbits
        w = getattr(i, sig).get_type().nbits
        connect(out[b : b + w], ifcSig)


class Test(Component):
    def construct(s):
        s.ins_ = [MasterIfc() for _ in range(4)]
        s.out = OutPort(b8)
        s.out1 = OutPort(b8)

        ifcConcat(s.out, s.ins_, "in_")
        ifcConcat(s.out1, s.ins_, "in_2")

and this will translate to:

module Test_noparam
(
  input  logic [0:0] clk ,
  output logic [7:0] out ,
  output logic [7:0] out1 ,
  input  logic [0:0] reset ,
  input logic [1:0] ins___in_ [0:3] ,
  input logic [1:0] ins___in_2 [0:3] 
);

  assign out[1:0] = ins___in_[0];
  assign out[3:2] = ins___in_[1];
  assign out[5:4] = ins___in_[2];
  assign out[7:6] = ins___in_[3];
  assign out1[1:0] = ins___in_2[0];
  assign out1[3:2] = ins___in_2[1];
  assign out1[5:4] = ins___in_2[2];
  assign out1[7:6] = ins___in_2[3];

endmodule

which works.

[cbatten]

The power of python!