Skip to content

Add host API#220

Merged
asroy merged 23 commits into
developfrom
jd/host_api
May 12, 2022
Merged

Add host API#220
asroy merged 23 commits into
developfrom
jd/host_api

Conversation

@JehandadKhan
Copy link
Copy Markdown
Contributor

@JehandadKhan JehandadKhan commented May 3, 2022

No description provided.

asroy
asroy reviewed May 4, 2022
View reviewed changes
Comment thread example/CMakeLists.txt Outdated
@JehandadKhan JehandadKhan requested a review from asroy May 5, 2022 17:49
@JehandadKhan JehandadKhan requested a review from aosewski May 5, 2022 22:40
aosewski
aosewski reviewed May 6, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aosewski aosewski left a comment

Choose a reason for hiding this comment

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

Overall looks good. I'd just add proper documentation for host API.

Comment thread CMakeLists.txt Outdated
add_subdirectory(library)
add_subdirectory(example)
add_subdirectory(test)
add_subdirectory(test EXCLUDE_FROM_ALL)
Copy link
Copy Markdown
Collaborator

@aosewski aosewski May 6, 2022

Choose a reason for hiding this comment

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

Maybe we should rather have a cmake option like -DWITH_TESTS=ON instead of excluding by default all test targets from all target?

Copy link
Copy Markdown
Contributor Author

@JehandadKhan JehandadKhan May 6, 2022

Choose a reason for hiding this comment

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

I removed and fixed the issue elsewhere. The cause for this was that gmock was complaining on the warnings we set in our repos which has been overridden in the googletest.cmake file.

Comment thread Config.cmake.in Outdated
Comment thread Jenkinsfile
Comment thread library/include/ck/library/host/host_interface.hpp Outdated
Comment thread library/include/ck/library/host/host_interface.hpp
Comment thread library/include/ck/library/host_tensor/device.hpp Outdated
Comment thread library/include/ck/library/host_tensor/device.hpp Outdated
Comment thread test/client_app/CMakeLists.txt Outdated
@JehandadKhan JehandadKhan self-assigned this May 6, 2022
@JehandadKhan JehandadKhan requested a review from aosewski May 6, 2022 22:30
@asroy asroy removed the CI - Pass label May 7, 2022
aosewski
aosewski previously approved these changes May 9, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aosewski aosewski left a comment

Choose a reason for hiding this comment

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

Thanks for adding changes in googltest.cmake. Actually I also came across those warnings but the fun thing is that only after submitting the PR with googletest :):)

@JehandadKhan JehandadKhan requested a review from aosewski May 10, 2022 15:55
@JehandadKhan
Copy link
Copy Markdown
Contributor Author

JehandadKhan commented May 10, 2022

@asroy CI passes and all reviews have been addressed

asroy
asroy previously approved these changes May 12, 2022
View reviewed changes
@asroy asroy self-requested a review May 12, 2022 14:20
@asroy asroy merged commit cec69bc into develop May 12, 2022
@junliume junliume deleted the jd/host_api branch October 21, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aosewski aosewski Awaiting requested review from aosewski aosewski is a code owner

1 more reviewer

@asroy asroy asroy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JehandadKhan JehandadKhan

Labels

CI - Pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JehandadKhan @aosewski @asroy