[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-594093568 @cjolivier01 please prioritize the PR, as this affects other users. For example https://github.com/apache/incubator-mxnet/issues/17733 Let me know if I may resubmit the MKL static linkage commit earlier included in #17645. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-593526173 @cjolivier01 thanks for volunteering to contribute the PR! Do you have any status update? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-591533630 @cjolivier01 the PR coniststs of two commits. Only the second commit is related to omp and implements the conclusion from the discussion in this issue. The first commit enables testing the cmake build. Currently the cmake build is largely untested and broken. You're free to open a separate PR to improve the fix in #17645, but it's unreasonable to veto that change. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-591266103 @TaoLv @cjolivier01 its not hard to fix. If you look above, I posted the patch to fix it 12 hours ago This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-591019097 > > > @TaoLv > > > what version of gcc? > > > > > > @cjolivier01 this will happen with any version of gcc > > @TaoLv which version of gcc? You're right. I misremembered that fact. libgomp is only a transitive dependency. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-591019224 > even if I remove the openmp build in CMakeLists.txt and build with clang, I get that warning, since it pulls in libomp from clang @cjolivier01 How can we stop clang from pulling in libomp? It doesn't have an effect when static linking, as the symbols are already resolved, but it would be better to not pull libomp in in the first place. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error
URL:
https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-591002985
> > -DMKL_USE_STATIC_LIBS=ON
>
> How broken? When I built with clang, it seems to run ok.
>
> Nevermind, scratch that, it wasn;t finding mkl
You can use this patch as fix
``` diff
diff --git a/cmake/Modules/FindMKL.cmake b/cmake/Modules/FindMKL.cmake
index 51eff8fe0..444a57687 100644
--- a/cmake/Modules/FindMKL.cmake
+++ b/cmake/Modules/FindMKL.cmake
@@ -19,8 +19,6 @@
#
# Options:
#
-# USE_MKLDNN: Search for MKL:ML library variant
-#
# MKL_USE_SINGLE_DYNAMIC_LIBRARY : use single dynamic library interface
# MKL_USE_STATIC_LIBS : use static libraries
# MKL_MULTI_THREADED : use multi-threading
@@ -45,8 +43,10 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains
intel libs")
# ---[ Options
- option(MKL_USE_SINGLE_DYNAMIC_LIBRARY "Use single dynamic library
interface" ON)
- cmake_dependent_option(MKL_USE_STATIC_LIBS "Use static libraries" OFF
"NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
+ # Single dynamic library interface leads to conflicts between intel omp
and llvm omp
+ # https://github.com/apache/incubator-mxnet/issues/17641
+ option(MKL_USE_SINGLE_DYNAMIC_LIBRARY "Use single dynamic library
interface" OFF)
+ cmake_dependent_option(MKL_USE_STATIC_LIBS "Use static libraries" ON "NOT
MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
cmake_dependent_option(MKL_MULTI_THREADED "Use multi-threading" ON "NOT
MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
option(MKL_USE_ILP64 "Use ilp64 data model" OFF)
cmake_dependent_option(MKL_USE_CLUSTER "Use cluster functions" OFF
"CMAKE_SIZEOF_VOID_P EQUAL 4" OFF)
@@ -122,10 +122,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains
intel libs")
list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
endforeach()
-
if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
if (MKL_USE_STATIC_LIBS)
- set(__iomp5_libs iomp5 libiomp5mt.lib)
+ set(__iomp5_libs libiomp5.a libiomp5mt.lib)
else()
set(__iomp5_libs iomp5 libiomp5md.lib)
endif()
@@ -135,15 +134,18 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder
contains intel libs")
list(APPEND __looked_for INTEL_INCLUDE_DIR)
endif()
-find_library(MKL_RTL_LIBRARY ${__iomp5_libs}
+find_library(IOMP_LIBRARY ${__iomp5_libs}
PATHS ${INTEL_RTL_ROOT} ${INTEL_ROOT}/compiler ${MKL_ROOT}/..
${MKL_ROOT}/../compiler
PATH_SUFFIXES ${__path_suffixes}
DOC "Path to Path to OpenMP runtime library")
-list(APPEND __looked_for MKL_RTL_LIBRARY)
-list(APPEND MKL_LIBRARIES ${MKL_RTL_LIBRARY})
+list(APPEND __looked_for IOMP_LIBRARY)
+list(APPEND MKL_LIBRARIES ${IOMP_LIBRARY})
endif()
+ if(MKL_USE_STATIC_LIBS)
+set(MKL_LIBRARIES -Wl,--start-group "${MKL_LIBRARIES}" -Wl,--end-group)
+ endif()
include(FindPackageHandleStandardArgs)
@@ -154,4 +156,3 @@ if(MKL_FOUND)
endif()
mxnet_clear_vars(__looked_for __mkl_libs __path_suffixes __lib_suffix
__iomp5_libs)
-
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-591003298 > > > @TaoLv > > > what version of gcc? > > > > > > @cjolivier01 this will happen with any version of gcc > > it doesn;t happen on my machine, but I am not linking to opencv because it breaks my pytorch, build having it installed. maybe opencv pulls in libgomp? You're right. I misremembered that fact. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590994057 @cjolivier01 this will happen with any version of gcc This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590993751 @TaoLv `-DMKL_USE_STATIC_LIBS=ON` is currently broken on my system and in particular doesn't link `iomp` statically. I'll create a PR to statically link iomp by default if MKL is found. As all OMP related symbols will then be resolved at compile time, it doesn't matter that gcc declares `libgomp.so` and clang `libomp.so` respectively as NEEDED. Further I'll disable building llvm openmp when statically linking to `libiomp.so`. @cjolivier01 does that sound alright to you? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error
URL:
https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590950280
`-DMKL_USE_STATIC_LIBS=ON` will still link `libiomp5` dynamically as per our
`FindMKL.cmake`. That could be fixed, but then `libiomp5` will be statically
linked and llvm `libopenmp` dynamically linked. So both will still be present
and the error reported by @icemelon9 would remain?
I'll verify the following patch for
https://github.com/apache/incubator-mxnet/pull/17645
``` diff
diff --git a/cmake/Modules/FindMKL.cmake b/cmake/Modules/FindMKL.cmake
index 51eff8fe0..346a02103 100644
--- a/cmake/Modules/FindMKL.cmake
+++ b/cmake/Modules/FindMKL.cmake
@@ -45,9 +45,10 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains
intel libs")
# ---[ Options
- option(MKL_USE_SINGLE_DYNAMIC_LIBRARY "Use single dynamic library
interface" ON)
- cmake_dependent_option(MKL_USE_STATIC_LIBS "Use static libraries" OFF
"NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
+ option(MKL_USE_SINGLE_DYNAMIC_LIBRARY "Use single dynamic library
interface" OFF)
+ cmake_dependent_option(MKL_USE_STATIC_LIBS "Use static libraries" ON "NOT
MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
cmake_dependent_option(MKL_MULTI_THREADED "Use multi-threading" ON "NOT
MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
+ cmake_dependent_option(MKL_IOMP "Link with Intel OpenMP" OFF "NOT
MKL_USE_SINGLE_DYNAMIC_LIBRARY" OFF)
option(MKL_USE_ILP64 "Use ilp64 data model" OFF)
cmake_dependent_option(MKL_USE_CLUSTER "Use cluster functions" OFF
"CMAKE_SIZEOF_VOID_P EQUAL 4" OFF)
@@ -121,9 +122,12 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains
intel libs")
list(APPEND __looked_for ${__mkl_lib_upper}_LIBRARY)
list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
endforeach()
+ if(MKL_USE_STATIC_LIBS)
+set(MKL_LIBRARIES -Wl,--start-group "${MKL_LIBRARIES}" -Wl,--end-group)
+ endif()
- if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
+ if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY AND MKL_IOMP)
if (MKL_USE_STATIC_LIBS)
set(__iomp5_libs iomp5 libiomp5mt.lib)
else()
```
> On my machine, I always have another gomp as reported here:
Empirically loading gomp and iomp or llvm openmp does not cause problems.
But loading llvm openmp + intel openmp causes problems. @cjolivier01 may want
to clarify as he vetoes any change.
Ideally for GCC, we should only link `libiomp5.so` if compiled with MKL and
not any llvm openmp. For compiling with clang, we should only link llvm
`libomp.so`, as apparently llvm + intel omp conflict.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590716897 As it's rather difficult to disable clang linking with llvm openmp, we can disable linking intel openmp until a solution is found to only link against intel openmp. For that, it may be required to compile with intel compiler. To stop linking with intel openmp, remove / disable https://github.com/apache/incubator-mxnet/blob/31144c763bfd0fe199b7fe0f23a20555c9731e7a/cmake/Modules/FindMKL.cmake#L126-L145 and set `-DMKL_USE_SINGLE_DYNAMIC_LIBRARY=OFF`. Then we get ``` 0x0001 (NEEDED) Shared library: [libdl.so.2] 0x0001 (NEEDED) Shared library: [libpthread.so.0] 0x0001 (NEEDED) Shared library: [libmkl_intel_lp64.so] 0x0001 (NEEDED) Shared library: [libmkl_intel_thread.so] 0x0001 (NEEDED) Shared library: [libmkl_core.so] 0x0001 (NEEDED) Shared library: [librt.so.1] 0x0001 (NEEDED) Shared library: [libopencv_highgui.so.3.2] 0x0001 (NEEDED) Shared library: [libopencv_imgcodecs.so.3.2] 0x0001 (NEEDED) Shared library: [libopencv_imgproc.so.3.2] 0x0001 (NEEDED) Shared library: [libopencv_core.so.3.2] 0x0001 (NEEDED) Shared library: [liblapack.so.3] 0x0001 (NEEDED) Shared library: [libstdc++.so.6] 0x0001 (NEEDED) Shared library: [libm.so.6] 0x0001 (NEEDED) Shared library: [libomp.so.5] 0x0001 (NEEDED) Shared library: [libgcc_s.so.1] 0x0001 (NEEDED) Shared library: [libc.so.6] 0x0001 (NEEDED) Shared library: [ld-linux-x86-64.so.2] ``` and `libmkl` shouldn't attempt loading `iomp` dynamically. This works because llvm openmp is compatible with `iomp`. @icemelon9 I can't reproduce this problem. Please comment out the lines pointed out above and compile with `cmake -DUSE_CUDA=0 -DUSE_MKLDNN=1 DMKL_USE_SINGLE_DYNAMIC_LIBRARY=OFF -GNinja ..; ninja` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590642092 @TaoLv @pengzhao-intel can you advise about the mkl dynamic loading of iomp? What's your recommendation to fix this issue. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590523257 > However, even if I remove the openmp build in CMakeLists.txt and build with clang, I get that warning, since it pulls in libomp from clang (I am using clang8): When building with MKL, are we supposed to build with `libiomp` as well @cjolivier01 @pengzhao-intel? If so, we need to change our build accordingly. If not, this seems to be a bug in MKL. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error
URL:
https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-590465449
CI can also reproduce this issue. I switched CI to testing CMake builds
instead of Makefile build in
https://github.com/apache/incubator-mxnet/pull/17645 and the Python MKLDNN +
MKL Pipeline fails with this issue:
[Log](http://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/mxnet-validation%2Funix-cpu/detail/PR-17645/30/pipeline/297)
and [Raw
log](http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/mxnet-validation/pipelines/unix-cpu/branches/PR-17645/runs/30/nodes/297/steps/775/log/?start=0)
That pipeline relies on the following build
```
build_ubuntu_cpu_mkldnn_mkl() {
set -ex
cd /work/build
cmake \
-DCMAKE_BUILD_TYPE="RelWithDebInfo" \
-DUSE_MKL_IF_AVAILABLE=ON \
-DBLAS="MKL" \
-DUSE_TVM_OP=ON \
-DUSE_CUDA=OFF \
-DUSE_CPP_PACKAGE=ON \
-G Ninja /work/mxnet
ninja
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589859111 @icemelon9 please provide the a reproducer to trigger the error message. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589858953 @sl1pkn07 please open a separate issue for your problem. This issue is about MKL. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589851161 Yes, that's why `dlopen`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589844040 I think `libmkl_rt` may `dlopen` `libiomp` as per https://github.com/intel/mkl-dnn/issues/230#issuecomment-451082066 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589843442 `libmkl_rt` static links `libiomp` as per https://github.com/intel/mkl-dnn/issues/230#issuecomment-451073275 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589827331 > What is pulling in libiomp5.so ? MKL > btw, cmake files have min cmake at 3.13, but default 18.04 cmake install is cmake 3.10. Does anyone know what the deal is with 3.13? Ubuntu 18.04 is a pretty widely-used release... > I changed back to 3.10 and it seems to build ok. Just `pip install cmake` as per the doc https://mxnet.apache.org/get_started/ubuntu_setup. It'd be harder to explain when users require 3.13 and when 3.X, or 3.Y, than to uniformly require a recent version. There are various bugs fixed in 3.13 that affect MXNet use-cases (eg cuda, https://cmake.org/cmake/help/latest/policy/CMP0077.html for llvm openmp subproject) > Not actually. Due to no legitimate reason to remove it. Speed up developer build. No need to build llvm openmp if system openmp is present. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589750326 @sl1pkn07 given the rapid development of intel-dnnl, MXNet expects a fixed version of intel-dnnl. It's quite unlikely that the system provides that particular version, but patches to improve detection are welcome. Do you want to contribute a PR? But let's track this in a separate issue. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17641: OpenMP Error
leezu commented on issue #17641: OpenMP Error URL: https://github.com/apache/incubator-mxnet/issues/17641#issuecomment-589749070 @cjolivier01 you previously vetoed changing the omp configuration in cmake build, due to a race condition that had not been fixed. As that has been fixed, are you OK with proceeding to prefer system OMP for the CMake build by default? Or what is your recommendation? Static build should still statically build omp. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
