-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Added support to build onnxruntime with ACL #3586
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -482,6 +482,7 @@ if (onnxruntime_USE_ACL) | |
| source_group(TREE ${ONNXRUNTIME_ROOT}/core FILES ${onnxruntime_providers_acl_cc_srcs}) | ||
| add_library(onnxruntime_providers_acl ${onnxruntime_providers_acl_cc_srcs}) | ||
| onnxruntime_add_include_to_target(onnxruntime_providers_acl onnxruntime_common onnxruntime_framework onnx onnx_proto protobuf::libprotobuf) | ||
| target_link_libraries(onnxruntime_providers_acl -L$ENV{LD_LIBRARY_PATH}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why using LD_LIBRARY_PATH? that's normally used at run time.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to link the ACL so files. Without this build fails with this error: "Unable to load arm_compute arm_compute_graph arm_compute_core".
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point was that LD_LIBRARY_PATH should not be used for that purpose.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if you make user pass ACL library location, the user will still need to set LD_LIBRARY_PATH in order to use onnxruntime_perf_test otherwise get this error: "error while loading shared libraries: libarm_compute.so: cannot open shared object file: No such file or directory."
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, but LD_LIBRARY_PATH is used at runtime. it's not meant to be used during build time (which is what the cmake files are for) |
||
| add_dependencies(onnxruntime_providers_acl ${onnxruntime_EXTERNAL_DEPENDENCIES}) | ||
| set_target_properties(onnxruntime_providers_acl PROPERTIES FOLDER "ONNXRuntime") | ||
| target_include_directories(onnxruntime_providers_acl PRIVATE ${ONNXRUNTIME_ROOT} ${eigen_INCLUDE_DIRS} ${ACL_INCLUDE_DIR}) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,7 +208,7 @@ Status Conv<T>::Compute(OpKernelContext* context) const { | |
| if(optimizable) { | ||
| //optimized depthwise convolution | ||
| #if defined(ACL_1902) || defined(ACL_1905) | ||
| auto layer = std::make_shared<arm_compute::NEDepthwiseConvolutionLayer3x3>(); | ||
| auto layer = std::make_shared<arm_compute::NEDepthwiseConvolutionLayer>(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the reason of this change? NEDepthwiseConvolutionLayer3x3 is an optimized version for depthwise 3x3 which increases the performance significantly for mobilenets and other models.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it was accidental.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was getting error(NEDepthwiseConvolutionLayer3x3 not defined) and had to make this change to build. I'll update my ACL repo and retry.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got the issue, this should be only used for ACL_1902. I'll create a new PR to use NEDepthwiseConvolutionLayer3x3 and fix the error.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| #endif | ||
| #ifdef ACL_1908 | ||
| auto layer = std::make_shared<arm_compute::NEDepthwiseConvolutionLayerOptimized>(); | ||
|
|
||
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 an example for someone who wants to build ACL. If they have a pre-built version, they may use it as mentioned above.