lebeg commented on issue #12160: Remove conflicting llvm OpenMP from cmake 
builds
URL: https://github.com/apache/incubator-mxnet/pull/12160#issuecomment-441019573
 
 
   I cross post the things I already sent to the @dev list:
   
   The issue was closed, but I am strong in my opinion that it's the right 
thing to do.
   
   **Background**
   
   If you want to use OpenMP pragmas in your code for parallelization you would 
supply a special flag to the compiler:
   
   - Clang / -fopenmp
   https://openmp.llvm.org/
   
   - GCC / -fopenmp
   https://gcc.gnu.org/onlinedocs/libgomp/Enabling-OpenMP.html
   
   - Intel / [Q]openmp
   
https://software.intel.com/en-us/node/522689#6E24682E-F411-4AE3-A04D-ECD81C7008D1
   
   - Visual Studio: /openmp (Enable OpenMP 2.0 Support)
   https://msdn.microsoft.com/en-us/library/tt15eb9t.aspx
   
   Each of the compilers would enable the `#pragma omp` directive during C/C++ 
compilation and arrange for automatic linking of the OpenMP runtime library 
supplied by each compiler separately.
   
   Thus, to use the advantages of an OpenMP implementation one has to compile 
the code with the corresponding compiler.
   
   Currently, in MXNet CMake build scripts a bundled version of llvm OpenMP is 
used 
([here](https://github.com/apache/incubator-mxnet/blob/master/CMakeLists.txt#L400-L405)
 and [here](https://github.com/apache/incubator-mxnet/tree/master/3rdparty)) to 
replace the OpenMP library supplied by the compiler.
   
   I will quote here the README from the MKL-DNN (Intel(R) Math Kernel Library 
for Deep Neural Networks):
   
   "Intel MKL-DNN uses OpenMP* for parallelism and requires an OpenMP runtime 
library to work. As different OpenMP runtimes may not be binary compatible it's 
important to ensure that only one OpenMP runtime is used throughout the 
application. Having more than one OpenMP runtime initialized may lead to 
undefined behavior resulting in incorrect results or crashes." 
[link](https://github.com/intel/mkl-dnn/blame/master/README.md#L261-L265)
   
   And:
   
   "Using GNU compiler with -fopenmp and -liomp5 options will link the 
application with both Intel and GNU OpenMP runtime libraries. This will lead to 
undefined behavior of the application." 
[link](https://github.com/intel/mkl-dnn/blame/master/README.md#L278-L280)
   
   As can be seen from ldd for MXNet:
   
   ```
   $ ldd build/tests/mxnet_unit_tests | grep omp
       libomp.so => /.../mxnet/build/3rdparty/openmp/runtime/src/libomp.so 
(0x00007f697bc55000)
       libgomp.so.1 => /usr/lib/x86_64-linux-gnu/libgomp.so.1 
(0x00007f69660cd000)
   ```
   
   **Performance**
   
   The only performance data related to OpenMP in MXNet I was able to find is 
here:
   https://github.com/apache/incubator-mxnet/issues/9744#issuecomment-367711172
   
   Which in my understanding is testing impact of different environment 
variables for the same setup (using same bundled OpenMP library).
   
   The libraries may differ in implementation and the [Thread Affinity 
Interface](https://software.intel.com/en-us/node/522691) may have significant 
impact on performance.
   
   All compilers support it:
   
   - Clang / KMP_AFFINITY
   https://github.com/clang-ykt/openmp/blob/master/runtime/src/kmp_affinity.cpp
   
   - GCC / GOMP_CPU_AFFINITY
   
https://gcc.gnu.org/onlinedocs/gcc-4.7.1/libgomp/GOMP_005fCPU_005fAFFINITY.html
   
   - Intel / KMP_AFFINITY
   
https://software.intel.com/en-us/node/522689#6E24682E-F411-4AE3-A04D-ECD81C7008D1
   
   - Visual Studio / SetThreadAffinityMask
   
https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-setthreadaffinitymask
   
   I see no reason why this submodule should be kept.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to