You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/03/03 18:55:53 UTC

[GitHub] [incubator-mxnet] leezu opened a new pull request #17751: Fix MKL static link & default to static link on unix

leezu opened a new pull request #17751: Fix MKL static link & default to static link on unix
URL: https://github.com/apache/incubator-mxnet/pull/17751
 
 
   Fixes https://github.com/apache/incubator-mxnet/issues/17641
   
   Changes
   - Fix MKL Static linkage and default to it on Unix
   - Don't build llvm openmp when linking to MKL. If MKL is present, intel omp is also 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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17751: Fix MKL static link & default to static link on unix

Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17751: Fix MKL static link & default to static link on unix
URL: https://github.com/apache/incubator-mxnet/pull/17751#discussion_r387342850
 
 

 ##########
 File path: CMakeLists.txt
 ##########
 @@ -442,8 +440,11 @@ if(USE_OPENMP)
     if(OPENMP_FOUND)
       set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
       set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
-      set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
-      set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+      if(NOT BLAS STREQUAL "MKL")
+        # Linker flags for Intel OMP are already set in case MKL is used. Only set if not MKL
+        set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+      endif()
 
 Review comment:
   FYI @cjolivier01, based on this, clang won't add omp.
   
   ```
   ubuntu@ip-172-31-24-41:~/mxnet/build$ ldd libmxnet.so | grep omp
           libiomp5.so => /usr/local/lib/libiomp5.so (0x00007f982da3d000)
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu merged pull request #17751: Fix MKL static link & default to static link on unix

Posted by GitBox <gi...@apache.org>.
leezu merged pull request #17751: Fix MKL static link & default to static link on unix
URL: https://github.com/apache/incubator-mxnet/pull/17751
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] TaoLv commented on issue #17751: Fix MKL static link & default to static link on unix

Posted by GitBox <gi...@apache.org>.
TaoLv commented on issue #17751: Fix MKL static link & default to static link on unix
URL: https://github.com/apache/incubator-mxnet/pull/17751#issuecomment-594354616
 
 
   @leezu We're working on a proposal for the build logic of DNNL/MKL related stuffs. You can decide to merge this change now if urgent or wait for the proposal after it's ready. Thanks! @pengzhao-intel @zixuanweeei 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17751: Fix MKL static link & default to static link on unix

Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17751: Fix MKL static link & default to static link on unix
URL: https://github.com/apache/incubator-mxnet/pull/17751#discussion_r387342850
 
 

 ##########
 File path: CMakeLists.txt
 ##########
 @@ -442,8 +440,11 @@ if(USE_OPENMP)
     if(OPENMP_FOUND)
       set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
       set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
-      set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
-      set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+      if(NOT BLAS STREQUAL "MKL")
+        # Linker flags for Intel OMP are already set in case MKL is used. Only set if not MKL
+        set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+      endif()
 
 Review comment:
   FYI @cjolivier01, based on this, clang won't add omp.
   
   ```
   ubuntu@ip-172-31-24-41:~/mxnet/build$ ldd libmxnet.so | grep omp
           libiomp5.so => /usr/local/lib/libiomp5.so (0x00007f982da3d000)
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on issue #17751: Fix MKL static link & default to static link on unix

Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17751: Fix MKL static link & default to static link on unix
URL: https://github.com/apache/incubator-mxnet/pull/17751#issuecomment-594658172
 
 
   @TaoLv thanks. Let's merge it now as building with mkl is currently broken without this PR. I look forward to your proposal 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services