-
Notifications
You must be signed in to change notification settings - Fork 423
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 general transpose for vivado/vitis #1124
base: main
Are you sure you want to change the base?
Conversation
c4efd51
to
cf72985
Compare
I would propose the following change with this PR since it's no longer needed:
|
transpose_config_template = """struct {config_name} {{ | ||
static const unsigned dims = {dims}; | ||
static const unsigned N = {N}; | ||
static const unsigned* const from_shape; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you tried various things, and I know C++ can be kind of clunky for stuff like this, but is there an easier/less verbose way to initialize these arrays? Would std::arrays be more readable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is due to that vivado/vitis
cannot handle constexpr
array indexing with "variables" (indices can be inferenced, but not literal, like an iterator index in a fixed-boundary for loop).
If we are to drop vivado hls support completely, this can be made constexpr
and initialized with one-line, and recursive template unrolling will need to be used in the caller function to do the index mapping, which only works for vitis hls and takes forever on vivado hls. I did not try std::array
, as I did not expect it to be more compatible than a constexpr raw array.
transpose_include_list = ['nnet_utils/nnet_array.h', 'nnet_utils/nnet_stream.h'] | ||
|
||
def permute_config_gen(name: str, shape: tuple[int, ...], perm: tuple[int, ...]): | ||
new_shape = tuple(shape[i] for i in perm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment or docstring would be useful to understand how this works.
This PR looks to be quite useful and I think should be quite rapidly merged, since it makes pytorch streaming models more parsable (after removing the no longer needed graceful failure, with this PR). |
Description
Add general N-dimensional transpose for vivado/vitis backend. Resource consumption validated to be unchanged for all 2/3d io_parallel Vivado/Vitis HLS (2020.1/2023.2), and 2d io_stream Vitis HLS.
For Vivado HLS there is a resource overhead for io_sream transpose 2d. Template overload is used to use the original impl for io_stream 2d.
Type of change
Tests
test/pytest/test_transpose_concat.py/test_highdim_permute
Checklist
pre-commit
on the files I edited or added.