Hi!
I see not any problems in principle, but I think the patch should be improved.
c_api:: cals into error::wrap_c_api() for proper error handling.return engine(...) returns a weak reference. Is that what you want? It probably is, but I do not know your use case.I can fix the item 1 above myself as long as you don't see any problems with the item 2.
We probably want to have this query implemented for all the primitive_desc's...
Thanks for the reply! Yes I missed the exception part.
It is a weak reference and I think it should be that way. Query a raw engine pointer out of a primitive_desc means it is managed somewhere and will be destroyed by that place. I can't take a hold of that shared "handle" so it is the best shot. Actually I think every primitive_desc created by pass in engine are all "weak" references to it.
And yes, all kinds of primirive_desc should have this query.
And there is a interesting error when compile using gcc 5.3. Not happen when clang-4.0 on OS X or Linux.
cd /home/caozhong/Workspace/mkl-dnn/build/examples && /usr/bin/c++ -DUSE_CBLAS -DUSE_MKL -I/opt/intel/compilers_and_libraries_2017.2.174/linux/mkl/include -I/home/caozhong/Workspace/mkl-dnn/include -fPIC -Wformat -Wformat-security -fstack-protector-strong -fopenmp -Wall -Werror -Wno-unknown-pragmas -DMKLDNN_DLL -DMKLDNN_DLL_EXPORTS -fvisibility=internal -std=c++11 -fvisibility-inlines-hidden -O3 -DNDEBUG -D_FORTIFY_SOURCE=2 -std=gnu++11 -o CMakeFiles/simple-net-cpp.dir/simple_net.cpp.o -c /home/caozhong/Workspace/mkl-dnn/examples/simple_net.cpp
In file included from /home/caozhong/Workspace/mkl-dnn/examples/simple_net.cpp:19:0:
/home/caozhong/Workspace/mkl-dnn/include/mkldnn.hpp: In member function ‘mkldnn::engine mkldnn::memory::primitive_desc::get_engine()’:
/home/caozhong/Workspace/mkl-dnn/include/mkldnn.hpp:390:33: error: call of overloaded ‘engine(mkldnn::c_api::mkldnn_engine&)’ is ambiguous
return engine(engine_q);
^
/home/caozhong/Workspace/mkl-dnn/include/mkldnn.hpp:199:19: note: candidate: mkldnn::engine::engine(mkldnn::c_api::mkldnn_engine)
using handle::handle;
^
/home/caozhong/Workspace/mkl-dnn/include/mkldnn.hpp:232:14: note: candidate: mkldnn::engine::engine(mkldnn::c_api::mkldnn_engine* const&)
explicit engine(const c_api::mkldnn_engine_t& aengine)
^
/home/caozhong/Workspace/mkl-dnn/include/mkldnn.hpp:197:8: note: candidate: mkldnn::engine::engine(const mkldnn::engine&)
struct engine: public handle
OK, the problem of gcc error is 'using handle::handle' brings handle constructor template up in engine which gcc-5.3 complain it conflicted with engine's constructor. It's odd, so I removed 'using handle::handle'.
The use case is for Chainer to reorder output on demand. The need for creating reorder happens randomly, which might be a few python modules away from where the engine was created. The best way to create reorder primitive_descs is to query engine out of existing primitive_descs.
mkl-dnn.patch.zip
Closing since there's a PR pending already.
Most helpful comment
Hi!
I see not any problems in principle, but I think the patch should be improved.
c_api:: calsintoerror::wrap_c_api()for proper error handling.return engine(...)returns a weak reference. Is that what you want? It probably is, but I do not know your use case.I can fix the item 1 above myself as long as you don't see any problems with the item 2.
We probably want to have this query implemented for all the primitive_desc's...