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

Cannot pass views to functions #189

Open
rachitnigam opened this issue May 2, 2019 · 6 comments
Open

Cannot pass views to functions #189

rachitnigam opened this issue May 2, 2019 · 6 comments

Comments

@rachitnigam
Copy link
Member

Views implementation doesn't allow us to pass views into arrays. The core issue is that for something like:

decl a: ...;
view v_a = a[4 * i: ];
foo(v_a)

the codegen phase needs to rewrite it to something of the form:

decl a: ...;
foo(&a[4 * i]);

it's unclear what Vivado will do if we pass it something of this form.

@sampsyo suggested doing one of two things:

  1. If we have to implement the above, it would be easier instead do:
decl a: ...;
int* v_a = &a[4 * i];
foo(v_a);
  1. If performance tanks because of this implementation, we can implement language level function inlining for defs.

Before committing to a strategy, we need to try both of them on Vivado and see what happens.

@sampsyo
Copy link
Contributor

sampsyo commented May 2, 2019

To expand on my suggestion, it was inspired by what the actual hardware would do: we'd generate a small hardware gadget at the point where the view was created, and we'd pass "wires" into this gadget into the functions that consume it. The hardware generated for the function would not need to be aware that these wires are connected to a view gadget instead of a physical array.

@rachitnigam
Copy link
Member Author

This example passes synthesis with the attached report:

#define I 4
#define J 4
#define K 4

int add_slice(int* a, int* b) {
  int x = 0;
  for (int i = 0; i < 2; i++) {
    x += a[i] + b[i];
  }
  return x;
}

void test(int m1[8], int m2[8], int res[4]) {
  for (int i = 0; i < 4; i++) {
    res[i] = add_slice(&m1[2 * i], &m2[2 * i]);
  }
}

Command:

sds++ -perf-est-hw-only -sds-pf zed -sds-hw test test.cpp -sds-end -clkid 3 -poll-mode 1 -verbose  -Wall -O3 -c test.cpp -o test.o

Report: sds_test.txt

@sa2257
Copy link
Collaborator

sa2257 commented May 2, 2019

the codegen phase needs to rewrite it to something of the form:

If we have to implement the above, it would be easier instead do..

Would you mind explaining what's the expected hardware here?

And why not simply pass the view array itself?

@rachitnigam
Copy link
Member Author

Since functions have inlining semantics, there should be no additional hardware. We might choose to have a small gadget as adrian mentioned above but that's a cost associated with views, not functions.

If Fuse was doing function inlining automatically, it would copy the function body into call and replace the parameters with the view. Once we have that, the compiler can rewrite view accesses as normal.

And why not simply pass the view array itself?

There is no "views array". Accesses on views are rewritten to accesses on the underlying array.

@rachitnigam
Copy link
Member Author

I don't think the first suggestion of using &v[2] works for multi dimensional arrays:

decl a: bit<32>[10][20];
view v_a = a[_ :2][_ :2];

would get compiled to:

int a[10][20];
int v_a = &(a[2][2]); 

Here, v_a is a simple pointer. To allow for normal array access, multi-dimensional access, we need to have a double pointer.

A naive solution to the problem would be to start flattening arrays again, but that would conflict with #175....

@sampsyo
Copy link
Contributor

sampsyo commented May 4, 2019

Aha, indeed—that would be a problem! And flattening arrays wouldn't really fix it either; a 2x2 window of a larger two-dimensional array won't be continuous in the flattened representation.

Fundamentally, it seems like this is a situation where our language is more expressive than the HLS tools we're using as a backend—which means anything we do here will necessarily be a hack. For example, we could try generating little array-lookup functions to encapsulate the index arithmetic (and to represent the hardware gadget that implements the view), but it seems dicey that Vivado would do the right thing with that…

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

3 participants