Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPP IW source is now fixed, but still can't be used #138

Closed
native-api opened this issue Oct 29, 2018 · 13 comments
Closed

IPP IW source is now fixed, but still can't be used #138

native-api opened this issue Oct 29, 2018 · 13 comments

Comments

@native-api
Copy link
Contributor

native-api commented Oct 29, 2018

Expected behaviour

opencv/opencv#10411 has been fixed since 3.4.2 (but they only notified me now).
So I tried to re-enable the IPP library in manylinux1 build.

Actual behaviour

Warning during compilation:

/io/_skbuild/linux-x86_64-2.7/cmake-build/3rdparty/ippicv/ippiw_lnx/src/iw_core.c:68:21: note: #pragma message: Atomic operations are not supported by this compiler. Some features my not be thread-safe.
             #pragma message("Atomic operations are not supported by this compiler. Some features my not be thread-safe.")

Running OpenCV test suite does show additional failures in imgproc suite.

Steps to reproduce

export MB_PYTHON_VERSION=2.7
export ENABLE_CONTRIB=0
export ENABLE_HEADLESS=0

    set -e
    # Multibuild doesn't have releases, so --depth would break eventually (see
    # https://superuser.com/questions/1240216/server-does-not-allow-request-for-unadvertised)
    git submodule update --init multibuild
    source multibuild/common_utils.sh
    # https://github.com/matthew-brett/multibuild/issues/116
    if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then export ARCH_FLAGS=" "; fi
    source multibuild/travis_steps.sh
    # This sets -x
    source multibuild_customize.sh
    echo $ENABLE_CONTRIB > contrib.enabled
    echo $ENABLE_HEADLESS > headless.enabled
    before_install
    # Not interested in travis internal scripts' output
    set +x

git clone git://github.com/opencv/opencv_extra.git
(cd opencv_extra; git checkout 3.4.3)

docker run -ti --rm -e 'BUILD_COMMANDS=build_wheel .' -e PYTHON_VERSION=2.7 -e UNICODE_WIDTH=32 -e BUILD_COMMIT= -e CONFIG_PATH= -e ENV_VARS_PATH= -e WHEEL_SDIR=wheelhouse -e MANYLINUX_URL=https://5cf40426d9f06eb7461d-6fe47d9331aba7cd62fc36c7196769e4.ssl.cf2.rackcdn.com -e BUILD_DEPENDS= -e USE_CCACHE= -e REPO_DIR=. -e PLAT=x86_64 -v `pwd`:/io -v /root:/parent-home quay.io/skvark/manylinux1_x86_64 bash --login
  • In Docker, build and run tests:
cd /io
export OPENCV_PYTHON_BINARY=/opt/python/cp27-cp27mu/bin/python
export OPENCV_TEST_DATA_PATH=/io/opencv_extra/testdata
$OPENCV_PYTHON_BINARY setup.py build
$OPENCV_PYTHON_BINARY opencv/modules/ts/misc/run.py _skbuild/linux-x86_64-2.7/cmake-build/ -v -a -t imgproc

(omit -t <...> to run all tests)

  • operating system

CentOS 7 x64

  • opencv-python version

master

@skvark
Copy link
Member

skvark commented Oct 29, 2018

Atomics were introduced in C++11 and to my understanding the compiler in manylinux1 images supports C++11. Maybe some compiler flag is required to enable it.

@native-api
Copy link
Contributor Author

native-api commented Oct 29, 2018

[root@76d301cae8e0 io]# g++ opencv/cmake/checks/cxx11.cpp
opencv/cmake/checks/cxx11.cpp:4:2: error: #error "C++11 is not supported"
 #error "C++11 is not supported"

Judging by the source of iw_core.c from https://github.com/opencv/opencv_3rdparty/blob/ippicv/master_20180723/ippicv/ippicv_2019_lnx_intel64_general_20180723.tgz and by https://stackoverflow.com/questions/20326604/stdatomic-h-in-gcc-4-8, the fix is to upgrade GCC to 4.9 or to use OpenMP.

I'll try the latter now.

@native-api
Copy link
Contributor Author

Maybe some compiler flag is required to enable it.

Oh, yes, with --std=c++11, the check passes. However that doesn't seem to be revelant, iw_core.c rather checks _OPENMP and GCC version.

@alalek
Copy link
Member

alalek commented Nov 1, 2018

For C++11 support in OpenCV consider using of "ENABLE_CXX11=ON" CMake variable (for OpenCV 3.x, OpenCV 4.0 requires/enables C++11 by default).

@native-api
Copy link
Contributor Author

Okay, I've got OpenMP running:

  • First, OpenMP detection for C in cmake is broken. (The code used for check produces warnings, some of which are treated as errors due to a -Werror=<...> in CMAKE_C_FLAGS.) Need this patch:
    FindOpenMP.cmake.patch.zip

The cmake 3.9 installation is not present in the stock manylinux1 image, so I guess you're in charge of maintaining it.

However, after all this, the warning no longer appears but the above-mentioned tests still fail :\

@skvark
Copy link
Member

skvark commented Nov 1, 2018

Yes, I have updated Cmake in the extended manylinux1 images because there were issues with the Cmake version in the stock images (related to opencv contrib builds). Maybe OpenCV devs can help with the failing tests. Unfortunately I don't have experience with the IPP + OpenMP combo.

@native-api
Copy link
Contributor Author

native-api commented Nov 6, 2018

In the meanwhile, I tried to compile it with GCC 4.9.4 (the last 4.x release), too. Same problem. When I ran tests on the GCC itself, tests connected with stdatomic failed.

In Fedora 28 (GCC 8.2.1) and MacOS 10.13.6 (Clang clang-1000.10.44.4), tests pass.

This suggests a problem with GCC 4.

@skvark
Copy link
Member

skvark commented Nov 6, 2018

Unfortunately GCC cannot be (easily) upgraded due to the manylinux1 requirements, see pypa/manylinux#118. Manylinux2 could maybe help (https://github.com/pypa/manylinux/pull/152/files#r165837491) but the development seems to be stalled due to lack of developers.

@native-api
Copy link
Contributor Author

As a side note, CMake's OpenMP search logic was fixed in 3.10.0, and I've just had a PR accepted that fixes the other warning that will be in 3.13.0.

@native-api
Copy link
Contributor Author

It is likely possible to adapt the devtoolset-2's gcc RPM to compile 4.9.4 instead. It has many patches that might fix whatever causes the problem, but some patches (predictably) do not apply to 4.9.4 and need to be adapted.

Alternatively, there's devtoolset-3 with 4.9.2 . It may be possible to rebuild it for CentOS 5, probably with minimal changes to .spec. But I can't find SRPMs for it.

@native-api
Copy link
Contributor Author

native-api commented Nov 8, 2018

For now, I'll just change the comment in setup.py to point to this issue instead.

I've already spent more than reasonable effort on this, so I'm going to call it a day.
Unless someone finds the SRPMs for devtoolset-3, I don't see a potential in struggling further: if the problem is in stdatomic support, patches for 4.8.2 aren't going to have a fix for it.

@skvark
Copy link
Member

skvark commented Nov 8, 2018

Thank you for your efforts. I really appreciate the work you have put into opencv-python.

@pigubaoza
Copy link

Hi all, does this mean that the latest wheel on pip (4.2.0.32 as of this time) does not support IPP? what does this mean for AVX2 and SSE4.x? Are these instructions sets still used when IPP is turned off?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants