Feature consideration?

[KelvinChung2000]

After exploring and playing with pymtl3, I can feel how this can speed up hardware development. I have some suggestions for features that might enhance the experience of using pymtl3.

Better slicing Allow the Bits slice to work like Python slicing. For example allow, a[start:] or a[:end] for accessing a[start:nBits] or a[0:end].

Singed bit Allow Bits to be signed or provide a $signed like Verilog does. This is just something nice to have out of the box instead of me needing to write it when simulating and testing with negative values.

Making more function and class static For example, things like b1 or b32 are statically available, which allows for static analysis from IDE and hinting. Things added in simulation pass like sim_tick, letting those also be statically available. Just seeing what function is available by the IDE static analysis saves a lot of time in finding relevant information. Another benefit is you can document each of the individual functions within the code, and the IDE will display them, which results in less documentation needing to be maintained. A good example is Networkx. They have the best open-source documentation I have ever seen.

Introducing match Starting from Python3.10, the match statement is available, and it would be nice if we could use them for constructing hardware similar to how Verilog have a case statement.

Variable naming control Some constant variables will be named __const__something or __tmpvar__something. It would be nice if passes were available for renaming them.

Loop unrolling Including passes that unroll for loop statements. This might defeat the purpose of generating readable Verilog. Sometimes, reading the statement as it is rolled is more difficult, but easier to construct with a loop.

Once again, thanks for the great work.

[ptpan]

Thanks for these comments! I think many of your proposals are very related to the translation mechanism of PyMTL. Slicing, match statements, and variable naming are feasible with the current implementation of translators. Translation support for loop unrolling would be tricky though, especially if you are referring to loops in those behavioral @update blocks. Currently the translator performs a source-to-source translation from PyMTL DSL to Verilog, and unrolling loops in update blocks could be challenging in this implementation.

I think a $signed free helper function should not be too hard? As for making classes/functions static, I'm wondering if having a type annotation stub to the pymtl3 module could help. Python typeshed has been pretty good at providing hints for static analysis, and having something similar for pymtl doesn't sound bad at all?

[cbatten]

@KelvinChung2000 Excellent suggestions! We are very much interested in growing the base of PyMTL3 developers. If you are interested in maybe taking a stab at one of these and creating a pull request we would be happy to work with you to get it merged in ... I think supporting the extra slicing syntax would be a relatively straight-forward place to start? @ptpan do you agree? If so, and if @KelvinChung2000 you are interested let us know and we can point you to where in the code to maybe take a stab add support for the extra slicing syntax ...

[KelvinChung2000]

I am happy to do some development when I have time.

[KelvinChung2000]

Hi, I would like to know if there is any current implementation of the VcdGenerationPass that will separate bitstruct into individual signals?

[cbatten]

We don't currently have such an implementation ... would definitely be useful though ... not sure how much work it would be? @jsn1993 any thoughts?

[KelvinChung2000]

If you can point me to how to tell if a type is a bitstruct type, I might be able to do something.

[yo96]

Hi Kevin,

There is an is_bistruct_inst function to check if an object is a bitstruct instance, and an is_bitstruct_class function to check if a type is a bitstruct class. You can import them as the following:

from pymtl3.datatypes.bistructs import is_bitstruct_inst, is_bitstruct_class

Hope this helps!

Best, Yanghui

On Tue, Mar 26, 2024 at 11:40 AM Kelvin Chung @.***> wrote:

If you can point me to how to tell if a type is a bitstruct type, I might be able to do something.

— Reply to this email directly, view it on GitHub https://github.com/pymtl/pymtl3/issues/263#issuecomment-2020776709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFFR62PQEIHPUPPVHPTKTUDY2GCE5AVCNFSM6AAAAABALOWFBCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRQG43TMNZQHE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[cbatten]

I think this is where bitstructs get flattened?

https://github.com/pymtl/pymtl3/blob/4b3bc183b14ffacd069f48dc5997b00045aea53f/pymtl3/passes/tracing/VcdGenerationPass.py#L237-L244

So we would need to do an explicit check first to see if signal is a bitstruct. So first thing would be to create a very simple minimal example and then maybe use this:

https://github.com/pymtl/pymtl3/blob/4b3bc183b14ffacd069f48dc5997b00045aea53f/pymtl3/datatypes/bitstructs.py#L78-L80

To print out if the signal is a bitstruct ... if that works then we would need to recurse into the fields ... but just detecting bitstructs during VCD gen might be a first step? I gave this a shot. First, I a created the following simple test case:

from pymtl3 import *

@bitstruct
class Pair:
  a : Bits8
  b : Bits8

class BitStructPassThru( Component ):

  def construct( s ):

    s.in_   = InPort ( Pair )
    s.out   = OutPort( Pair )

    s.in_a  = InPort ( Bits8 )
    s.in_b  = InPort ( Bits8 )
    s.out_a = OutPort( Bits8 )
    s.out_b = OutPort( Bits8 )

    s.out   //= s.in_
    s.out_a //= s.in_a
    s.out_b //= s.in_b

  def line_trace( s ):
    return f"{s.in_}|{s.in_a}|{s.in_a} () {s.out}|{s.out_a}|{s.out_b}"

top = BitStructPassThru()
top.apply( DefaultPassGroup(linetrace=True,vcdwave="pymtl3_bitstruct_vcd_test") )

top.sim_reset()
for input_value in [ (0x00,0x00), (0x01,0x10), (0xa0,0xb0), (0xab,0xbc) ]:
  top.in_  @= Pair(input_value[0],input_value[1])
  top.in_a @= input_value[0]
  top.in_b @= input_value[1]
  top.sim_tick()

top.sim_tick()
top.sim_tick()
top.sim_tick()

Then after hacking around a bit I ended up adding this to the VcdGenerationPass.py:

  ...
      for i, (signal, symbol) in enumerate( net_details ):

        # If we encounter a BitStruct then dump it as a concatenation of
        # all fields.
        # TODO: treat each field in a BitStruct as a separate signal?

        if is_bitstruct_class(signal.get_type()):
          print(f"{signal} is of type {signal.get_type()} which is a bitstruct")
          for field_name in eval(repr(signal)).__bitstruct_fields__.keys():
            full_field_name = f"{repr(signal)}.{field_name}"
            print(f"  field {field_name} value is {eval(full_field_name)}")

        try:
          net_bits_bin = eval(repr(signal)).to_bits()
        except Exception as e:
          raise TypeError(f'{e}\n - {signal} becomes another type. Please check your code.')
  ...

As @yo96 mentioned I needed to import is_bitstruct_class like this:

from pymtl3.datatypes.bitstructs import is_bitstruct_class

Running my simple example produces this:

% python pymtl3_bitstruct_vcd_test.py
...
s.in_ is of type <class '__main__.Pair'> which is a bitstruct
  field a value is 01
  field b value is 10
  5: a0:b0|a0|a0 () a0:b0|a0|b0
s.in_ is of type <class '__main__.Pair'> which is a bitstruct
  field a value is a0
  field b value is b0
  6: ab:bc|ab|ab () ab:bc|ab|bc
...

So that is at least a starting point? We would need to update where the header is written to the VCD file and also make sure to handle the default values too on cycle 0. I would get everything working with only simple fields (i.e., all fields are Bits, no fields that are lists or BitStructs).

@yo96 @ptpan @jsn1993 do you think the approach I sketch above is the right way?

Would be awesome to have some more PyMTL hackers though so if you are interested maybe we can work together to add this feature in a PR?

[yo96]

Looks reasonable to me. Another thing to try is to translate to Verilog and dump the VCD from Verilator, which supports bitstruct if I remember correctly.

Best, Yanghui

On Tue, Mar 26, 2024 at 2:11 PM Christopher Batten @.***> wrote:

I think this is where bitstructs get flattened?

https://github.com/pymtl/pymtl3/blob/4b3bc183b14ffacd069f48dc5997b00045aea53f/pymtl3/passes/tracing/VcdGenerationPass.py#L237-L244

So we would need to do an explicit check first to see if signal is a bitstruct. So first thing would be to create a very simple minimal example and then maybe use this:

https://github.com/pymtl/pymtl3/blob/4b3bc183b14ffacd069f48dc5997b00045aea53f/pymtl3/datatypes/bitstructs.py#L78-L80

To print out if the signal is a bitstruct ... if that works then we would need to recurse into the fields ... but just detecting bitstructs during VCD gen might be a first step? I gave this a shot. First, I a created the following simple test case:

from pymtl3 import *

@bitstruct class Pair: a : Bits8 b : Bits8

class BitStructPassThru( Component ):

def construct( s ):

s.in_   = InPort ( Pair )
s.out   = OutPort( Pair )

s.in_a  = InPort ( Bits8 )
s.in_b  = InPort ( Bits8 )
s.out_a = OutPort( Bits8 )
s.out_b = OutPort( Bits8 )

s.out   //= s.in_
s.out_a //= s.in_a
s.out_b //= s.in_b

def linetrace( s ): return f"{s.in}|{s.in_a}|{s.in_a} () {s.out}|{s.out_a}|{s.out_b}"

top = BitStructPassThru() top.apply( DefaultPassGroup(linetrace=True,vcdwave="pymtl3_bitstruct_vcd_test") )

top.sim_reset() for inputvalue in [ (0x00,0x00), (0x01,0x10), (0xa0,0xb0), (0xab,0xbc) ]: top.in @= Pair(input_value[0],input_value[1]) top.in_a @= input_value[0] top.in_b @= input_value[1] top.sim_tick()

top.sim_tick() top.sim_tick() top.sim_tick()

Then after hacking around a bit I ended up adding this to the VcdGenerationPass.py:

... for i, (signal, symbol) in enumerate( net_details ):

    # If we encounter a BitStruct then dump it as a concatenation of
    # all fields.
    # TODO: treat each field in a BitStruct as a separate signal?

    if is_bitstruct_class(signal.get_type()):
      print(f"{signal} is of type {signal.get_type()} which is a bitstruct")
      for field_name in eval(repr(signal)).__bitstruct_fields__.keys():
        full_field_name = f"{repr(signal)}.{field_name}"
        print(f"  field {field_name} value is {eval(full_field_name)}")

    try:
      net_bits_bin = eval(repr(signal)).to_bits()
    except Exception as e:
      raise TypeError(f'{e}\n - {signal} becomes another type. Please check your code.')

...

As @yo96 https://github.com/yo96 mentioned I needed to import is_bitstruct_class like this:

from pymtl3.datatypes.bitstructs import is_bitstruct_class

Running my simple example produces this:

% python pymtl3_bitstruct_vcdtest.py ... s.in is of type <class 'main.Pair'> which is a bitstruct field a value is 01 field b value is 10 5: a0:b0|a0|a0 () a0:b0|a0|b0 s.in_ is of type <class 'main.Pair'> which is a bitstruct field a value is a0 field b value is b0 6: ab:bc|ab|ab () ab:bc|ab|bc ...

So that is at least a starting point? We would need to update where the header is written to the VCD file and also make sure to handle the default values too on cycle 0. I would get everything working with only simple fields (i.e., all fields are Bits, no fields that are lists or BitStructs).

@yo96 https://github.com/yo96 @ptpan https://github.com/ptpan @jsn1993 https://github.com/jsn1993 do you think the approach I sketch above is the right way?

Would be awesome to have some more PyMTL hackers though so if you are interested maybe we can work together to add this feature in a PR?

— Reply to this email directly, view it on GitHub https://github.com/pymtl/pymtl3/issues/263#issuecomment-2021154341, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFFR62JEA2A6KUSAFY2UZXLY2G24XAVCNFSM6AAAAABALOWFBCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRRGE2TIMZUGE . You are receiving this because you were mentioned.Message ID: @.***>

[KelvinChung2000]

I am thinking of doing something similar. I will implement it if I reach a point where I cannot progress without seeing each of the struct fields. For now, I can still get away with putting the fields in a different order.

I have considered moving to a more "traditional" flow, or eventually, I will need to, but I don't want to spend time learning new things if I can do everything well enough in a single language.

[yo96]

Hi Kelvin,

With PyMTL3 you don't have to switch to a "traditional" flow to use Verilator. You can translate your design to Verilog, import it back into PyMTL3, and do Python-Verilator co-simulation. This way you can dump the waveform of your Python simulation from Verilator. The only downside is that you will lose the python line trace.

Here is a quick way to do it with Chris' example above, only with slight modifications:

from pymtl3 import *
from pymtl3.stdlib.test_utils import run_sim, config_model_with_cmdline_opts

@bitstruct
class Pair:
  a : Bits8
  b : Bits8

class BitStructPassThru( Component ):

  def construct( s ):

    s.in_   = InPort ( Pair )
    s.out   = OutPort( Pair )

    s.in_a  = InPort ( Bits8 )
    s.in_b  = InPort ( Bits8 )
    s.out_a = OutPort( Bits8 )
    s.out_b = OutPort( Bits8 )

    s.out   //= s.in_
    s.out_a //= s.in_a
    s.out_b //= s.in_b

  def line_trace( s ):
    return f"{s.in_}|{s.in_a}|{s.in_a} () {s.out}|{s.out_a}|{s.out_b}"

top = BitStructPassThru()
top = config_model_with_cmdline_opts( top, cmdline_opts={
  "test_verilog": "zeros", # xinit value, could be "zeros", "ones", or
"rand"
  "dump_vcd": "pymtl3_bitstruct_vcd_test",
}, duts=[] )

top.apply( DefaultPassGroup(linetrace=True) )

top.sim_reset()
for input_value in [ (0x00,0x00), (0x01,0x10), (0xa0,0xb0), (0xab,0xbc) ]:
  top.in_  @= Pair(input_value[0],input_value[1])
  top.in_a @= input_value[0]
  top.in_b @= input_value[1]
  top.sim_tick()

top.sim_tick()
top.sim_tick()
top.sim_tick()

config_model_with_cmdline_opts is meant to be used with a pytest fixture but here I am directly calling it to apply the passes and set corresponding metadata. You can also manually set the metadata and apply the translate-import pass like the following:

top.set_metadata( VerilogTranslationImportPass.enable, True )
top.set_metadata( VerilogVerilatorImportPass.vl_xinit, "zeros" )
top.set_metadata( VerilogVerilatorImportPass.vl_trace, True )
top.set_metadata( VerilogVerilatorImportPass.vl_trace_filename,
"pymtl3_bitstruct_vcd_test" )

top.apply( VerilogPlaceholderPass() )
top = VerilogTranslationImportPass()( top )

top.apply( DefaultPassGroup(linetrace=True) )

You will see a pymtl3_bitstruct_vcd_test.verilator1.vcd generated by Verilator. This way you remain in the Python land but can get a more full-featured VCD from Verilator.

Best, Yanghui

On Tue, Mar 26, 2024 at 5:11 PM Kelvin Chung @.***> wrote:

I am thinking of doing something similar. I will implement it if I reach a point where I cannot progress without seeing each of the struct fields. For now, I can still get away with putting the fields in a different order.

I have considered moving to a more "traditional" flow, or eventually, I will need to, but I don't want to spend time learning new things if I can do everything well enough in a single language.

— Reply to this email directly, view it on GitHub https://github.com/pymtl/pymtl3/issues/263#issuecomment-2021481273, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFFR62NSC5LATPD7SRRLK4LY2HI5RAVCNFSM6AAAAABALOWFBCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRRGQ4DCMRXGM . You are receiving this because you were mentioned.Message ID: @.***>

[KelvinChung2000]

I don't know this is available at all. Thanks for the information.

[KelvinChung2000]

My design is failing with the following assert

        # NOTE: this assertion can fail due to connections that
        # are made outside the component that has them. so i'm removing
        # this for now until we can figure out a better way to do sanity
        # check here.
        # assert len(ordered_conns) == len(m_conns_set)

        for i, x in enumerate(ordered_conns):
            if x not in m_conns_set:
                x = (x[1], x[0])
                print(x)
                assert x in m_conns_set, (
                    "There is a connection missing from "
                    "connect_order. Please contact PyMTL developers!"
                )
                ordered_conns[i] = x

The connection that causes the problem is this (s.tile[0][0].inputCrossbar.in_[5].rdy, s.tile[0][0].inputCrossbar.in_[5].en)

[cbatten]

Can you show what the line is that is causing the assertion? Is it something like this:

  connect( s.tile[0][0].inputCrossbar.in_[5].rdy, s.tile[0][0].inputCrossbar.in_[5].en )

Is the tile a component and the inputCrossbar is a component inside the tile? If so, then this is a hierarchical structural connection. It is not translatable ... if you were to do something similar in Verilog it also is not synthesizable. It does not model real hardware ... for structural connections to be translatable/synthesizable they need to be contained within one level of the hierarchy ... if a tile contains a crossbar and you want to connect some ports on that crossbar then the tile has to be the one to make those connections not the parent of the tile ... but maybe I am missing something?

[KelvinChung2000]

This is the connection s.inputCrossbar.in_[5].en //= s.inputCrossbar.in_[5].rdy

[cbatten]

Ah ... ok ... that is not a hierarchical reference so ignore what I said above .... looks ok to me ... we would need to see a minimal failing example or at least your code to help debug ... I am assuming in_ is some kind of PyMTL interface?

[KelvinChung2000]

in_ is the following interface. This is the same as the one used in CGRA-Flow.

class RecvIfcRTL( CalleeIfcRTL ):

  def construct( s, Type ):
    super().construct( en=True, rdy=True, MsgType=Type, RetType=None )

  def connect( s, other, parent ):

    # We are doing SendCL (other) -> [ RecvCL -> SendRTL ] -> RecvRTL (s)
    # SendCL is a caller interface
    if isinstance( other, CallerIfcCL ):
      m = RecvCL2SendRTL( s.MsgType )

      if hasattr( parent, "RecvCL2SendRTL_count" ):
        count = parent.RecvCL2SendRTL_count
        setattr( parent, "RecvCL2SendRTL_" + str( count ), m )
      else:
        parent.RecvCL2SendRTL_count = 0
        parent.RecvCL2SendRTL_0 = m

      connect_pairs(
        other,  m.recv,
        m.send.msg, s.msg,
        m.send.en,  s.en,
        m.send.rdy, s.rdy
      )
      parent.RecvCL2SendRTL_count += 1
      return True

    elif isinstance( other, CalleeIfcCL ):
      if s._dsl.level <= other._dsl.level:
        raise InvalidConnectionError(
            "CL2RTL connection is not supported between RecvIfcRTL"
            " and CalleeIfcCL.\n"
            "          - level {}: {} (class {})\n"
            "          - level {}: {} (class {})".format(
                s._dsl.level, repr( s ), type( s ), other._dsl.level,
                repr( other ), type( other ) ) )

      m = RecvCL2SendRTL( s.MsgType )

      if hasattr( parent, "RecvCL2SendRTL_count" ):
        count = parent.RecvCL2SendRTL_count
        setattr( parent, "RecvCL2SendRTL_" + str( count ), m )
      else:
        parent.RecvCL2SendRTL_count = 0
        parent.RecvCL2SendRTL_0 = m

      connect_pairs(
        other,  m.recv,
        m.send.msg, s.msg,
        m.send.en,  s.en,
        m.send.rdy, s.rdy
      )
      parent.RecvCL2SendRTL_count += 1
      return True

    return False

[cbatten]

Oh man .. not sure about that code at all ... FYI, at least in my group, we have moved away from en/rdy and back to val/rdy ... I wonder if part of the issue is you are directly connecting the output rdy signal of an interface to the input en signal of that same interface ... that seems a little strange? And we also moved away from this CL stuff when just doing RTL modeling ... we now just use pure RTL everywhere when doing RTL modeling (including pure RTL sources/sinks for testing) .. simplifies things ... we could maybe take a closer look but you would have to push your code to github and provide step-by-step instructions on how to reproduce the error ... the smaller the failing test case the better :)

[KelvinChung2000]

In the current mast, under pymtl3/stdlib/ifcs/send_recv_ifcs.py, it is still using en/rdy...

I am connecting it this way because I want to invalidate the output when I receive backpressure from the pipeline.

This is the smallest example I can produce

from pymtl3 import *
from pymtl3.passes.backends.verilog import *

class RecvIfcRTL(CalleeIfcRTL):
    def construct(s, Type):
        super().construct(en=True, rdy=True, MsgType=Type, RetType=None)

class testInner(Component):
    def construct(s):
        s.in_ = RecvIfcRTL(mk_bits(2))

        @update
        def upblk():
            s.in_.rdy @= 1

class testOuter(Component):
    def construct(s):
        s.inner = testInner()
        s.inner.in_.en //= s.inner.in_.rdy

if __name__ == "__main__":
    m = testOuter()
    m.elaborate()
    m.set_metadata(VerilogTranslationPass.enable, True)
    m.apply(VerilogTranslationPass())

[yo96]

I think that is because of a limitation of the translation pass. I remember running into this issue a few years ago as well. A workaround is to change the structural connection to a behavioral update block like the following:

        @update
        def up_blk():
          s.inner.in_.en @= s.inner.in_.rdy

Alternatively, you can create an intermediate signal like this:

        s.inner_in_en = Wire()
        s.inner.in_.en  //= s.inner_in_en
        s.inner.in_.rdy //= s.inner_in_en

Basically it confuses the translation pass when you are directly connecting the ports of the same component outside the component. I can't recall the exact details at the moment, but @ptpan is the expert of the translation pass and can provide a more in-depth explanation.

I hope these workarounds help you resolve the issue and get your code up and running smoothly!

[cbatten]

At least internally I have started using the pymtl4.0-dev branch. The whole send/recv thing with en/rdy was a bit of an experiment ... but ultimately I ended up going back to val/rdy for streaming interfaces ...

I just tried your example and was able to reproduce the issue ... if you use this:

class testOuter(Component):
    def construct(s):
        s.inner = testInner()
        s.temp = Wire(1)
        s.inner.in_.en  //= s.temp
        s.inner.in_.rdy //= s.temp

It goes away and the translation works fine ... I am not quite sure why this fixes things to be honest? ... looks like @yo96 was helping debug at the same time!

[KelvinChung2000]

Thanks for the help.