Skip to content
New issue

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

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

Already on GitHub? # to your account

Program scope hostpipe support #284

Merged
merged 2 commits into from
Apr 5, 2023
Merged

Conversation

zibaiwan
Copy link
Contributor

@zibaiwan zibaiwan commented Apr 4, 2023

  1. Support program scope hostpipe, CSR pipe and non-CSR pipe.
  2. Register program scoped hostpipes during program creation.
  3. Create new command op and device op for the hostpipe read and write
  4. Unit test is only for the auto-discovery string. Other unit tests will be added through other PR.
  5. Most of the Klocwork issues are the existing issues but listed as the new issues due to I modified the source code for the test, but I will attempt to fix all of them.
  6. More error checking needs to be done. Will add through a new PR.

@zibaiwan zibaiwan requested a review from sophimao April 4, 2023 13:57
@zibaiwan
Copy link
Contributor Author

zibaiwan commented Apr 4, 2023

@sophimao , while I am still working on the Klocwork fixes, please take a look at the change. Thanks!

sophimao
sophimao previously approved these changes Apr 4, 2023
Copy link
Contributor

@sophimao sophimao left a comment

Choose a reason for hiding this comment

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

Thanks Zibai! Overall looks good to me, just 2 minor comments as listed below.

src/acl_auto_configure.cpp Show resolved Hide resolved
src/acl_hostch.cpp Outdated Show resolved Hide resolved
src/acl_hostch.cpp Outdated Show resolved Hide resolved
1. Support program scope hostpipe, CSR pipe and non-CSR pipe.
2. Register program scoped hostpipes during program creation.
3. Create new command op and device op for the hostpipe read and write.
4. Create unit tests for the new autodiscovery string entry
5. Fix Klocwork issues in the deivce op unit test as they are flagged as new failures (they are not) due to my change in the device op
@zibaiwan
Copy link
Contributor Author

zibaiwan commented Apr 5, 2023

@sophimao , thanks for your review. I have addressed your comments, squashed the commits. Please take a look when you have a chance!

Copy link
Contributor

@sophimao sophimao left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants