fix openmp unwork issue in cmake version below 3.9.0 #679

kalcohol · 2018-11-29T17:02:43Z

change cmake minimum required version from 2.8.10 to 2.9.0 to avoid openmp bad behavior in ubuntu 16.04 or linux likely system(need more test).

…penmp bad behavior in ubuntu 16.04 or linux likely system

kalcohol · 2018-11-29T17:03:33Z

definitely not all the homework.

nihui · 2018-11-30T02:00:37Z

what is the bad openmp behavior ?

kalcohol · 2018-11-30T05:37:33Z

liekkas, qq 458083152, reported this issue in 637093648 qq group@2018/11/29, as these listed below:

liekkas(458083152) 18:52:56
@食言而肥我发现是一个提交导致的，a2611f47206ef0fe74f140954bd4c0534fb47a50
liekkas(458083152) 18:53:48
我测试ncnn::get_omp_num_threads()，也是总得到1，不管set的是多少
liekkas(458083152) 18:54:16
log上说要升级cmake 版本到3.9+
liekkas(458083152) 18:54:27
我正在尝试
liekkas(458083152) 18:54:55
实在不行就reset到250f6b8bdd2161aea0a177cdad6f4094a34b9143 @一品秋阳亲测这个版本可以
liekkas(458083152) 2018-11-29 23:53:01
@一品秋阳 @食言而肥确定是cmake版本的问题，需要cmake version > 3.9，最新的commit可以正常运行
liekkas(458083152) 2018-11-30 13:21:02
openmp不能正常工作，多线程设置不了
liekkas(458083152) 2018-11-30 13:21:36
不报错，只是不生效
liekkas(458083152) 2018-11-30 13:22:37
对，速度不变，get threads number，永远得到1
liekkas(458083152) 2018-11-30 13:23:47
16c
liekkas(458083152) 2018-11-30 13:25:00
小米 max2，小米8se，小米8
liekkas(458083152) 2018-11-30 13:25:09
都测过，一样的

ubuntu 16.04, NDK 16C, arm64-v8a, XiaoMi series mobile phone.
It seems need to trace this issue with more test.

zchrissirhcz · 2018-12-03T09:50:45Z

老铁们，那么我们应该用cmake3.9.0还是cmake2.9.0?看到提交的代码中minimum version是2.9.0？

kalcohol · 2018-12-03T12:23:46Z

老铁们，那么我们应该用cmake3.9.0还是cmake2.9.0?看到提交的代码中minimum version是2.9.0？

PR 还没完成，今晚不通宵的话，有时间改一下。或者 up 主可以改。

ayounes-nviso · 2018-12-03T15:19:53Z

Hello,
It seems that after my commit using "modern cmake" to find OpenMP, the minimum version of cmake shall be 3.1. In any case Ubuntu-16.04 comes with cmake 3.5 so this should not be an issue.

ayounes-nviso · 2018-12-03T15:23:01Z

CMakeLists.txt

@@ -11,7 +11,7 @@ set(CMAKE_INSTALL_PREFIX "${CMAKE_BINARY_DIR}/install" CACHE PATH "Installation
 endif()
 message(STATUS "CMAKE_INSTALL_PREFIX = ${CMAKE_INSTALL_PREFIX}")

-cmake_minimum_required(VERSION 2.8.10)
+cmake_minimum_required(VERSION 2.9.0)


shall me 3.1.0 after #a2611f47206ef0fe
(16.04 comes with 3.5.1)

sorry for this pr, i make a mistake, version should be above 3.9.0.
but i post a second commit, it's no need to update cmake to version 3.9.0. pls check this commit, thanks.

kalcohol · 2018-12-04T14:41:59Z

after second commit, it's no need to update cmake version to 3.9.0 or above, thanks to @CorentinB
's work in jolibrain/deepdetect#518.

coveralls · 2018-12-04T16:58:21Z

Coverage remained the same at ?% when pulling 47399c7 on kalcohol:master into ed2a24c on Tencent:master.

zchrissirhcz · 2018-12-05T01:49:38Z

Hi, @kalcohol
I try to verify if your commit works.
Platform: XiaoMi 8 (CPU: qc845)
Host System: Win10
CMake Version: CMake3.6.4111459(via Android Studio installed this cmake)
(Actually, I tried CMake2.8.10, which is the minimum version required in your committed CMakeLists.txt, however, my toolchain file will complain this version too old:

C:/Users/Chris/AppData/Local/Android/Sdk/ndk-bundle/build/cmake/android.toolchain.cmake:40 
(cmake_minimum_required):
  CMake 3.6.0 or higher is required.  You are running version 2.8.10

Thus I only successully compiled ncnn and run ncnn benchmark on my rooted android phone.

CMake3.6+lastest official ncnn

D:\work\test\ncnn\build>adb shell "cd /data; ./benchncnn 8 2 0"
loop_count = 8
num_threads = 2
powersave = 0
      squeezenet  min =   55.74  max =   57.00  avg =   56.42
       mobilenet  min =   87.77  max =   91.04  avg =   89.64
    mobilenet_v2  min =   58.21  max =   58.87  avg =   58.48
      shufflenet  min =   25.15  max =   25.50  avg =   25.31
       googlenet  min =  232.54  max =  246.66  avg =  239.52
        resnet18  min =  210.77  max =  217.06  avg =  212.65
         alexnet  min =  347.42  max =  361.23  avg =  351.04
           vgg16  min = 1258.66  max = 1292.40  avg = 1276.59
  squeezenet-ssd  min =  131.90  max =  135.03  avg =  132.82
   mobilenet-ssd  min =  169.19  max =  171.08  avg =  169.81
  mobilenet-yolo  min =  387.05  max =  390.97  avg =  389.68

CMake3.6+latest kalcohol ncnn

D:\work\test\kalcohol_ncnn\build>adb shell "cd /data; ./benchncnn 8 2 0"
WARNING: linker: Warning: unable to normalize "C"
WARNING: linker: Warning: unable to normalize "\Users\Chris\AppData\Local\Android\Sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib64\clang\6.0.2\lib\linux\arm"
loop_count = 8
num_threads = 2
powersave = 0
      squeezenet  min =   31.25  max =   32.31  avg =   31.87
       mobilenet  min =   47.41  max =   53.39  avg =   49.58
    mobilenet_v2  min =   32.68  max =   32.87  avg =   32.74
      shufflenet  min =   16.22  max =   16.96  avg =   16.59
       googlenet  min =  124.43  max =  125.37  avg =  124.98
        resnet18  min =  115.85  max =  116.68  avg =  116.38
         alexnet  min =  195.55  max =  197.82  avg =  196.60
           vgg16  min =  649.50  max =  654.11  avg =  651.25
  squeezenet-ssd  min =   76.96  max =   78.66  avg =   77.91
   mobilenet-ssd  min =   94.22  max =   96.59  avg =   95.09
  mobilenet-yolo  min =  218.03  max =  220.50  avg =  219.33

From the above result, the speed is actually faster, which I think openmp works correctly.

onexuan · 2018-12-05T07:44:25Z

But I tested my model and it became unstable, Sometimes fast and sometimes slow
android-armv7

kalcohol · 2018-12-05T09:44:22Z

But I tested my model and it became unstable, Sometimes fast and sometimes slow
android-armv7

pls check over heat issue, if openmp work, cpu will over heat very easily. if your device is a dev-board, not a cell phone, it's a easy way to conform the issue - use a fan direct to the main chip to cooling down cpu.

ayounes-nviso

Please check #689, this should fix your issues.

ayounes-nviso · 2018-12-04T15:10:01Z

CMakeLists.txt

@@ -11,7 +11,7 @@ set(CMAKE_INSTALL_PREFIX "${CMAKE_BINARY_DIR}/install" CACHE PATH "Installation
 endif()
 message(STATUS "CMAKE_INSTALL_PREFIX = ${CMAKE_INSTALL_PREFIX}")

-cmake_minimum_required(VERSION 2.9.0)
+cmake_minimum_required(VERSION 2.8.10)


As I said in your other PR, current master needs 3.1 so no need to go back, a PR must be created to actually bump it to 3.1

ayounes-nviso · 2018-12-04T15:10:58Z

CMakeLists.txt

@@ -32,6 +32,7 @@ option(NCNN_CMAKE_VERBOSE "print verbose cmake messages" OFF)

 if(NCNN_OPENMP)
    find_package(OpenMP)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")


With cmake >= 3.1 this should not be necessary (and is not recommended by "modern cmake" anyway).

nihui · 2018-12-06T06:44:01Z

close as #689 merged.

change cmake minimum required version from 2.8.10 to 2.9.0 to avoid o…

23ae58e

…penmp bad behavior in ubuntu 16.04 or linux likely system

ayounes-nviso reviewed Dec 3, 2018

View reviewed changes

fix openmp working bug below 3.9.0

47399c7

kalcohol changed the title ~~change cmake minimum required version from 2.8.10 to 2.9.0~~ fix openmp unwork issue in cmake version below 3.9.0 Dec 4, 2018

ayounes-nviso reviewed Dec 5, 2018

View reviewed changes

nihui closed this Dec 6, 2018

zhu-zhaofei mentioned this pull request Dec 19, 2021

PNNX is an open standard for PyTorch model interoperability #3262

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix openmp unwork issue in cmake version below 3.9.0 #679

fix openmp unwork issue in cmake version below 3.9.0 #679

kalcohol commented Nov 29, 2018

kalcohol commented Nov 29, 2018

nihui commented Nov 30, 2018

kalcohol commented Nov 30, 2018

zchrissirhcz commented Dec 3, 2018

kalcohol commented Dec 3, 2018

ayounes-nviso commented Dec 3, 2018

ayounes-nviso Dec 3, 2018

kalcohol Dec 4, 2018

kalcohol commented Dec 4, 2018

coveralls commented Dec 4, 2018

zchrissirhcz commented Dec 5, 2018

onexuan commented Dec 5, 2018 •

edited

Loading

kalcohol commented Dec 5, 2018

ayounes-nviso left a comment

ayounes-nviso Dec 4, 2018

ayounes-nviso Dec 4, 2018

nihui commented Dec 6, 2018

fix openmp unwork issue in cmake version below 3.9.0 #679

fix openmp unwork issue in cmake version below 3.9.0 #679

Conversation

kalcohol commented Nov 29, 2018

kalcohol commented Nov 29, 2018

nihui commented Nov 30, 2018

kalcohol commented Nov 30, 2018

zchrissirhcz commented Dec 3, 2018

kalcohol commented Dec 3, 2018

ayounes-nviso commented Dec 3, 2018

ayounes-nviso Dec 3, 2018

Choose a reason for hiding this comment

kalcohol Dec 4, 2018

Choose a reason for hiding this comment

kalcohol commented Dec 4, 2018

coveralls commented Dec 4, 2018

zchrissirhcz commented Dec 5, 2018

onexuan commented Dec 5, 2018 • edited Loading

kalcohol commented Dec 5, 2018

ayounes-nviso left a comment

Choose a reason for hiding this comment

ayounes-nviso Dec 4, 2018

Choose a reason for hiding this comment

ayounes-nviso Dec 4, 2018

Choose a reason for hiding this comment

nihui commented Dec 6, 2018

onexuan commented Dec 5, 2018 •

edited

Loading