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

[ONNIFI] Implement onnxSetIOAndRunGraph onnxifi extension #2360

Merged
merged 2 commits into from Feb 6, 2019

Conversation

Projects
None yet
4 participants
@jackm321
Copy link
Contributor

jackm321 commented Feb 6, 2019

Description:
Implement onnxSetIOAndRunGraph ONNXIFI extension.
Right now this is implemented very similarly to onnxSetGraphIO followed by onnxRunGraph but once this is tested and caffe2 switches to using onnxSetIOAndRunGraph, we can simplify this implementation and remove the mutex in onnxifi::graph.
Also implement onnxGetExtensionFunctionAddress for finding ONNXIFI extensions implemented by glow.
Updated onnx to latest version to include onnx/onnxifi_ext.h

Testing:
ninja test
Will test c2 implementation once it's ready

Documentation:
Doxygen

Fixes #2351

@yinghai

yinghai approved these changes Feb 6, 2019

Copy link
Contributor

yinghai left a comment

LGTM

@@ -22,6 +22,7 @@
#include "glow/Support/ThreadPool.h"

#include "onnx/onnxifi.h"
#include "onnx/onnxifi_ext.h"

This comment has been minimized.

@yinghai

yinghai Feb 6, 2019

Contributor

You already updated the onnx third party? Nice!

@@ -487,3 +546,39 @@ GLOW_ONNXIFI_LIBRARY_FUNCTION_WRAPPER(onnxReleaseGraph)(onnxGraph graph) {

return ONNXIFI_STATUS_SUCCESS;
}

// TODO do we need to have one onnxExtensionFunctionPointer per backendId?

This comment has been minimized.

@jackm321

jackm321 Feb 6, 2019

Author Contributor

This comment has been minimized.

@yinghai

yinghai Feb 6, 2019

Contributor

Yeah. Just one copy is fine.

@rdzhabarov
Copy link
Contributor

rdzhabarov left a comment

// TODO do we need to have one onnxExtensionFunctionPointer per backendId?

Good question. In any case it'll point to the same onnxSetIOAndRunGraph, so probably not.

@jackm321 jackm321 merged commit 2a37c5a into pytorch:master Feb 6, 2019

6 checks passed

ci/circleci: ASAN Your tests passed on CircleCI!
Details
ci/circleci: DEBUG Your tests passed on CircleCI!
Details
ci/circleci: RELEASE_WITH_LIT Your tests passed on CircleCI!
Details
ci/circleci: SHARED Your tests passed on CircleCI!
Details
ci/circleci: TSAN Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment