Onednn: Patch for query engine from memory::primitive_desc

Created on 16 May 2017  Â·  5Comments  Â·  Source: oneapi-src/oneDNN

Hi everyone!
It makes me a whole lot easier if this query interface is supported in C++ interface. Or make the same kind of engine of same index a singleton? Thanks!

(A patch file for reference)
patch.zip

enhancement

Most helpful comment

Hi!

I see not any problems in principle, but I think the patch should be improved.

  1. You should wrap c_api:: cals into error::wrap_c_api() for proper error handling.
  2. The 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...

All 5 comments

Hi!

I see not any problems in principle, but I think the patch should be improved.

  1. You should wrap c_api:: cals into error::wrap_c_api() for proper error handling.
  2. The 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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

CaoZhongZ picture CaoZhongZ  Â·  3Comments

xiechao4 picture xiechao4  Â·  3Comments

kruus picture kruus  Â·  6Comments

bhack picture bhack  Â·  7Comments

adhere picture adhere  Â·  5Comments