Better control of wrapper generation

[krame505]

This is related, but essentially orthogonal to the idea in #713. This is a bit less well-formed of an idea but I wanted to get some notes down before the meeting today.

The motivation is that the wrapper generation logic is rather complex and inflexible. Specifically, the inputs and outputs of action methods are not split into separate ports, which makes interfacing with the generated verilog rather tedious.

The high-level idea is to have a type class Wrapped that maps interface types into a type-level description of the wrapper that should be generated. The description types would contain of tuples of the ports that should be generated. Wrapper generation would then consult the Wrapped type class to determine what ports should be generated.

data (Port :: $ -> # -> *) name n = Port (Bit n)

class Wrapped a b | a -> b where
    wrap :: a -> b
    unwrap :: b -> a

For example, consider the following interfaces:

interface Foo =
  a :: UInt 8
  b :: Bool
 deriving (Bits)

interface Bar =
  c :: Foo
  d :: ActionValue Foo
  e :: Foo -> Action

The wrapper representation for Foo (corresponding to the way wrapper generation currently happens) would be

(Port "a" 8, Port "b" 1)

yielding the wrapper

interface FooWrap =
  a :: Bit 8
  b :: Bit 1

and Bar would be

(Port "c_a" 8, Port "c_b" 1, ActionValue (Port "d" 9), (Port "e_1" 9) -> Action)

yielding the wrapper

interface BarWrap =
  c_a :: Bit 8
  c_b :: Bit 1
  d :: ActionValue_ 9
  e :: Bit 9 -> Action_

It should be possible to generate these instances using generics (with a few complications, that I will get to in a moment.) There could also be an instance of Wrapped for Vector that adds the current behavior of unfolding vectors of interfaces into separate ports. Wrapper generation from these representation types is essentially trivial; the to and from methods can also make use of the wrap and unwrap methods in the Wrapped type class.

The interesting part is that we can define some newtype wrappers for how interface fields/method parameters should be treated, e.g. to indicate that a parameter/action result should be recursively expanded into separate ports:

interface Baz =
  f :: ActionValue (Expand Foo)
  g :: (Expand Foo) -> Action

The type Expand has a special case instance for Wrapped that give the representation type for Baz:

(ActionValue (Port "f_a" 8, Port "f_b" 1), (Port "g_a" 8, Port "g_b" 1) -> Action)

which would yield the wrapper

interface BazWrap =
  f_a :: Bit 8
  f_b :: Bit 1
  g :: Bit 8 -> Bit 1 -> Action_ {-# arg_names = ["g_a", "g_b"] #-}

My biggest uncertainty here is how to handle pragmas on interface fields. Presumably whatever needs to be preserved on the generated wrapper fields should also be encoded in the Port type. Also, the way that port names get specified for method inputs in the generated wrapper types isn't great - perhaps inputs should have their own representation type.

Note that to make the recursive generic unfolding work, we would need to modify the generic info to indicate whether a MetaData is an interface. I'm not sure whether it's better to do that or just add a new MetaIfc metadata that would wrap MetaData? We may need to do something similar to include info about interface pragmas on fields in the generic representation. This is technically a breaking change. But perhaps we can have a policy that introducing new layers of Meta tags in generic representations something that we can do, and any generic instances need to have a default instance that ignores unknown metadata types.

[krame505]

Another potential challenge is that wrapper generation happens quite early in the compilation passes, prior to ctxreduce. I'm not sure how easy it is to solve for a particular type class instance at genwrap. I have noticed that in the output of -dgenwrap, some wrapper fields are Bit types with determined sizes, while some have unresolved Bits contexts, and there doesn't appear (to me) to be a clear pattern as to why.

[krame505]

The consensus from Friday was that the first step should be to add a pragma to specify that an action method input should be split into separate ports, and defer any more general approach using type classes. Splitting up ActionValue methods would also be a bit more difficult, as this would require supporting methods with multiple output ports in synthesis. For example:

interface Foo =
  a :: UInt 8
  b :: Bool
 deriving (Bits)

interface Bar =
  putFoo :: Foo -> Action {#- split_inputs #-}

{-# synthesize #-}
mkBar :: Module Bar
mkBar = ...

would yield the wrapper

interface Bar_ =
  putFoo :: Bit 8 -> Bit 1 -> ActionValue_ 0 {-# prefixs = "putFoo", arg_names = [a, b], result = "putFoo", ready = "RDY_putFoo", enable = "EN_putFoo #-}
  RDY_putFoo :: Bit 1

There are still a few design questions here. One is whether we should only split structs/interface fields that have the pragma, or if we should split input interfaces recursively down to non-interface fields (as is done with outputs.)

One might argue in favor of the former approach, as making structs into interfaces to just get separate ports is arguably an abuse of interfaces. Doing so with outputs makes things more complicated for the scheduler, as this results in extra methods being created.

However I am in favor of the latter approach, for a few reasons. This is more in line with the current behavior for output ports, and one can control whether a nested item should be split by making it a struct or interface. In cases where we do want to flatten out a complex structure of nested interfaces, it is more tedious to specify the pragma on every interface. This also shouldn't make things worse for the scheduler, since flattening into multiple input ports doesn't create additional methods.

I'm also not quite sure how input port naming should work with the existing arg_names pragma - what should happen if split_inputs and arg_names are both specified? Should arg_names override the names of the nested fields (requiring more names to be specified than the top-level method has parameters) or just be ignored for any split parameters?

[quark17]

The consensus from Friday was that the first step should be to add a pragma

I think the first step is just to get BSC generating the separate ports -- I don't think you need to be required to first implement a new pragma just to try doing that. This is why I suggested a (hidden) flag to turn on this behavior as the default, while we're in this experimental stage.

(Also, will you be implementing both BH pragmas and BSV attributes?)

to specify that an action method input should be split into separate ports

I don't think this is specific to Action methods. Struct inputs to any method -- value, action, actionvalue -- should be split-able.

Splitting up ActionValue methods would also be a bit more difficult, as this would require supporting methods with multiple output ports in synthesis.

For one, this isn't just ActionValue, it's also value methods -- any method with an output. Two, I agree that changing the imported module info to allow specifying multiple output ports would be more complicated; however, I don't think that's necessary for implementing this feature. The module info could retain the struct type, and only when BSC generates the output Verilog (in AVerilog) would BSC need to then create separate ports, extracted from the single signal. (And submodule instantiations would do the reverse.) Splitting earlier would mean that maybe the Verilog backend could do more optimizing of the individual fields, which could be interesting; and certainly, supporting multiple output ports could be a nice feature in BSC in cases where you don't want to define a new struct just to do it; but this could be a first step.

So, actually, implementing the splitting of return values might even be easier than splitting input arguments, since it only involves work in AVerilog.hs (and AVerilogUtil.hs). (Or, arguably, splitting input ports could also be done this way in AVerilog instead of GenWrap, if you didn't want the benefits of separate optimization of the inport ports through the stages of BSC.) Though maybe, when we eventually want to control which outputs get split, that info may need to be added to the module info, and that may need to be plumbed through.

However, I'm totally fine with delaying this (in whatever form) until after input splitting has been implemented.

There are still a few design questions here.

Yes, but I think you need to step back. I'd first ask: Should there be multiple behaviors that can be specified? For example, ways to say both "split recusive" and "split non-recursive".

I don't see why we couldn't have both. If we only do one, I assume it would be recursive.

Also, a big thing I should have said up front: Why is Foo an interface in the example!? Interfaces can't be method inputs. We're only talking about structs. (And I'm hoping that splitting of return values will entirely replace the use of interfaces as a workaround.) So the example should look like this:

struct Foo =
  a :: UInt 8
  b :: Bool
 deriving (Bits)

Am I missing something? So in the places where you say the user could change behavior by changing whether the argument is a struct or an interface -- that should go away, it's only structs.

In cases where we do want to flatten out a complex structure of nested interfaces, it is more tedious to specify the pragma on every interface.

Again, I don't understand why we're talking about interfaces here. But I do think people will want attributes in more places. I think in some places they'll want to write one pragma to control the entire interface, and in some places they'll want to control an individual input. For example:

(* split_inputs *)
interface Ifc1;
  method Action m1(Foo x);
  method Action m2(Bar x);
endinterface

interface Ifc2;
  method Action m1( (* split *) Foo x, Bar y);
  method Action m2(Baz x);
endinterface

I'm not yet sure that split_inputs and split are the right names for the attributes, so that's another design decision. Nor do I know how we'd want to specify recursive vs not. We might even consider whether a user would want to specify in these places:

struct Foo =
  a :: Bool
  {-# split #-}
  b :: Bar
 deriving (Bits)

{-# split_inputs #-}
mkMod :: Module Ifc

Also note that, in BSV, attributes are immediately before the thing they are attached to -- so I've done the same in the above example. I'm surprised that you put the split_inputs attribute at the end of the line for a type signature of a method, but maybe that's how those pragmas work in BH -- I don't recall. (I think originally, pragmas in BH were attached to nothing, which is why there is a variant of the pragmas where you specify the top-level name that it should apply to -- possibly because we were sticking to Haskell at the time, which might not have had positional pragmas? And then later we just made it possible for the position of the pragma to imply the top-level definition that it applies to. Does Haskell have any positional pragmas, that could guide or choice here?)

I'm also not quite sure how input port naming should work with the existing arg_names pragma - what should happen if split_inputs and arg_names are both specified? Should arg_names override the names of the nested fields (requiring more names to be specified than the top-level method has parameters) or just be ignored for any split parameters?

The default generated name should be <argname>_<fieldname> where argname is the possibly-renamed argument name and fieldname comes from the field (possibly with several underscore-appended names, if recursively split). If the argname is defined as an empty string (""), then the name is just <fieldname>. For example (in BSV):

(* split_inputs *)
interface Ifc;
  method Action m1( (* port="Data" *) Foo x, Foo y);
  method Action m2( (* port="" *) Foo x);
  (* prefix="" *)
  method Action m3(Foo x);
endinterface

typedef struct {
  UInt#(8) a;
  Bool b;
} Foo
  deriving (Bits);

Here the default names would be m1_x_a and m1_x_b (the x is needed to distinguish from m1_y_a etc) but are renamed Data_a and Data_b. Instead of m2_x_a and m2_x_b, they are named a and b. And instead of m3_x_a and m3_x_b, they should be x_a and x_b.

That said, maybe the user would want finer control over the renaming of the ports -- for example, to rename x_a to x_A. For that, we'd have to think about what pragmas we'd want and where. For example, would we want to support that being specified in the datatype:

typedef struct {
  (* port="A" *)
  UInt#(8) a;
  Bool b;
} Foo
  deriving (Bits);

Or specified somehow on the interface or module? For example, a pragma that specifies that port "x" should be renamed "y":

(* port_rename="m1_Data_a, m1_Data_A" *)
method Action m1( (* port="Data" *) Foo x);

I'm not entirely sure.

[quark17]

Instead of split_inputs and split, maybe just split_struct (or split_structs). I doubt that in the long run people will want an attribute to only split inputs? So specifying split_structs seems like a better name, and we'll just note that it only works for inputs at the moment, but will include outputs later.

Although, if we're thinking in the long run, maybe someone wants to argue that splitting should be the default, and preserving structs as a single port would need an attribute. I dunno.

In BSV, attributes have values. If you just say (* split_structs *), that's an implied (* split_structs=1 *). We could use this to control different behaviors. For example:

(* split_structs="recursive" *)
(* split_structs="off" *)
(* split_structs="one" *)  // meaning one level, not recursive

Or things like that.

[quark17]

Actually, split_type or split_fields might be best. This doesn't have to be limited to structs, if we want to include splitting a Vector into its elements, etc.

[nanavati]

We definitely want to include splitting a vector into its elements. That's an important use case.