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

Adds additional cases for remove_redundant_buf optimization #682

Merged
merged 8 commits into from
Dec 25, 2024

Conversation

sjalander
Copy link
Collaborator

No description provided.

@sjalander sjalander enabled auto-merge (squash) December 16, 2024 04:55
Copy link
Owner

@phate phate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, could we get some unit tests on this?

jlm/hls/backend/rvsdg2rhls/remove-redundant-buf.cpp Outdated Show resolved Hide resolved
jlm/hls/backend/rvsdg2rhls/remove-redundant-buf.cpp Outdated Show resolved Hide resolved
jlm/hls/backend/rvsdg2rhls/remove-redundant-buf.cpp Outdated Show resolved Hide resolved
@sjalander sjalander requested a review from phate December 16, 2024 09:48
@sjalander sjalander requested a review from haved December 16, 2024 16:56
Copy link
Owner

@phate phate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unit tests were merged in another PR, right?

@sjalander
Copy link
Collaborator Author

A unit test has not been created for this PR. The reason is that when I started looking into the functionality of this optimization, I'm not able to determine its purpose. The name seems to be somewhat misleading. From what I can tell, it replaces pass through buffers with non-pass through buffers in some specific cases. However, it doesn't seem to have an effect on the hls_test_suite as when I comment out the optimization, then I get no change in the number of cycles.

@davidmetz Do you have a use case where this optimization makes a difference?

@sjalander sjalander merged commit 39c662b into phate:master Dec 25, 2024
12 checks passed
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

Successfully merging this pull request may close these issues.

2 participants