Giter VIP home page Giter VIP logo

Comments (6)

krame505 avatar krame505 commented on August 26, 2024

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.

from bsc.

krame505 avatar krame505 commented on August 26, 2024

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?

from bsc.

quark17 avatar quark17 commented on August 26, 2024

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.

from bsc.

quark17 avatar quark17 commented on August 26, 2024

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.

from bsc.

quark17 avatar quark17 commented on August 26, 2024

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.

from bsc.

nanavati avatar nanavati commented on August 26, 2024

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

from bsc.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.