[WIP] CMake NNPack support #9860
Conversation
MXInitialise Readies mxnet for use. While this is possible with some other commands this function makes more sense and does not require unneeded variables. MXNDArrayLoadListFromMemory Duplicates MXNDArrayLoad but from a location in memory. This was needed for my work as we bundle models into another file.
Merge branch 'master' of git://github.com/apache/incubator-mxnet
|
The test that failed is flaky #9856 |
MXInitialise Readies mxnet for use. While this is possible with some other commands this function makes more sense and does not require unneeded variables. MXNDArrayLoadListFromMemory Duplicates MXNDArrayLoad but from a location in memory. This was needed for my work as we bundle models into another file.
MXInitialise Readies mxnet for use. While this is possible with some other commands this function makes more sense and does not require unneeded variables. MXNDArrayLoadListFromMemory Duplicates MXNDArrayLoad but from a location in memory. This was needed for my work as we bundle models into another file.
CMakeLists.txt
Outdated
| # ---[ NNPack | ||
| if(USE_NNPACK) | ||
| if (USE_MKLDNN) | ||
| message(WARNING "With MKLDNN enabled NNPack operators will not run.") |
There was a problem hiding this comment.
Why? What happens in that case?
There was a problem hiding this comment.
both MKLDNN and NNPack define CPU versions of some operators, while there won't be an issue with compilation, when these operators are invoked they will need to use only the one backend.
There was a problem hiding this comment.
What happens if a user invokes one of these operators?
There was a problem hiding this comment.
The way I'm coding it, it will use the mkldnn backend. Once I finish that maybe some testing would show that NNPack is faster in which case it can be changed to the default.
There was a problem hiding this comment.
I'm not a fan of such implicit actions. It should be up to the user to decide which Backend to use. To me, setting both options seems like a conflict and should fail.
There was a problem hiding this comment.
I'll change it to fail in that case.
|
I wasn't going to push but I had conflicts with the upstream/master that needed to be resolved. The problem is that none of the old implementation of the operators would work, but I would like to keep them as a reference while I code new versions. |
| set(CONFU_DEPENDENCIES_BINARY_DIR "${CMAKE_BINARY_DIR}/3rdparty/NNPACK/deps" | ||
| CACHE PATH "Confu-style dependencies source directory") | ||
|
|
||
| add_subdirectory("${NNPACK_SOURCE_DIR}") |
There was a problem hiding this comment.
I'd suggest to do set the following variables:
set(NNPACK_BUILD_TESTS OFF CACHE BOOL "")
set(NNPACK_BUILD_BENCHMARKS OFF CACHE BOOL "")
set(NNPACK_LIBRARY_TYPE "static" CACHE STRING "")
set(PTHREADPOOL_LIBRARY_TYPE "static" CACHE STRING "")
set(CPUINFO_LIBRARY_TYPE "static" CACHE STRING "")
NNPACK_BUILD_TESTS and NNPACK_BUILD_BENCHMARKS disabled building NNPACK's own tests and benchmarks (which are enabled by default), and setting NNPACK_LIBRARY_TYPE, PTHREADPOOL_LIBRARY_TYPE and CPUINFO_LIBRARY_TYPE as "static" would build these projects as static libraries even if BUILD_SHARED_LIBS is ON.
After add_subdirectory, you'd likely want to add
set_property(TARGET nnpack PROPERTY POSITION_INDEPENDENT_CODE ON)
set_property(TARGET pthreadpool PROPERTY POSITION_INDEPENDENT_CODE ON)
set_property(TARGET cpuinfo PROPERTY POSITION_INDEPENDENT_CODE ON)
to build nnpack, pthreadpool, and cpuinfo with -fPIC even through they are static libraries.
|
Hi, the community has passed to vote about associating the code changes with JIRA (https://lists.apache.org/thread.html/ab22cf0e35f1bce2c3bf3bec2bc5b85a9583a3fe7fd56ba1bbade55f@%3Cdev.mxnet.apache.org%3E) We have updated the guidelines for contributors in https://cwiki.apache.org/confluence/display/MXNET/Development+Process, please ensure that you have created a JIRA at https://issues.apache.org/jira/projects/MXNET/issues/ to describe your work in this pull request and include the JIRA title in your PR as [MXNET-xxxx] your title where MXNET-xxxx is the JIRA id Thanks! |
|
Hi @dabraude. Thanks for the patch and for proposing the support. Given that this PR hasn't been updated for quite a while and the many changes in our CI, it would be great if you could create a new PR for this change. Thank you. |
Description
This is the first part of #9719
Checklist
Essentials
make lint)Changes
Comments
@Maratyszcza may want to check that everything is correct with regards to imports
Next up is refactoring the operators