Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for multiports and banks with widths based on reactor parameters #160

Open
erlingrj opened this issue Dec 10, 2024 · 2 comments

Comments

@erlingrj
Copy link
Collaborator

erlingrj commented Dec 10, 2024

The following is currently not supported:

reactor R(port_width:int=4, bank_width: int=5) {
  output [port_width] out: int

  children = new [bank_width] Child()
}

The reason is that, in the runtime, the port structs are part of the reactor self struct. When we have a multiport we put an array of port structs on the self struct. However, if the width is based on a parameter, then how large should we make the port struct array? It depends where and how it is instantiated.

The solution is likely to put the port structs on the self structs of the parent reactors and passed down as arguments.

@edwardalee
Copy link
Collaborator

I guess I don't see why the structs need to be in the parent reactor's self struct. Isn't there a separate instance of the self struct for each reactor instance? I guess the required memory could be a static array and a static length variable for each reactor instance. The self struct then just gets initialized to point to the array and set the length. But probably you've figured out a better way (or at least one that will work... (I have no idea whether what I'm describing even comes close to solving the problem).

@erlingrj
Copy link
Collaborator Author

You are right, and I think we are describing the same thing actually!

Currently, the self-struct of a reactor with a multiport looks a little like this:

typedef struct {
  Reactor_R_out out[10];
 } Reactor_R;

We would have to change it into

typedef struct {
  Reactor_R_out *out;
  size_t out_width;
} Reactor_R;

We would code-generate a constructor for this reactor that looks like this:

void Reactor_R_ctor(Reactor_R *self, Reactor_R_out *out, size_t out_width);

My initial proposal was that the array was to be put in the parent of Reactor_R, but I see now that this wont work either. Because it could be derived from a parameter arbitrarily high up in the containment hierarchy.

So where should those arrays live? An option would be to have them as global variables with unique names and then just referenced when we call Reactor_R_ctor. A challenge would be that we would need to do an initial pass through the AST and find all multiports and sort out their sizes.

I also see now that this also applies to bank of reactors. As they also appears as arrays on the self structs of the parent reactors.

This actually seems to be a quite hard problem since we decided to not do dynamic memory allocation....

@erlingrj erlingrj changed the title Add support for multiports with widths based on reactor parameters Add support for multiports and banks with widths based on reactor parameters Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants