Bug 924932 - OpenImageIO doesn't build because of an 'pause' assembler instruction
Summary: OpenImageIO doesn't build because of an 'pause' assembler instruction
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: OpenImageIO
Version: 19
Hardware: arm
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Richard Shaw
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-03-22 21:00 UTC by William Henry
Modified: 2013-03-31 23:59 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-03-31 23:59:52 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Change to thread.h to allow for compile/build on arm (592 bytes, patch)
2013-03-22 21:00 UTC, William Henry
no flags Details | Diff

Description William Henry 2013-03-22 21:00:18 UTC
Created attachment 714789 [details]
Change to thread.h to allow for compile/build on arm

Description of problem:

The build fails due to an included file containing some assembler:

[  4%] Building CXX object libOpenImageIO/CMakeFiles/OpenImageIO.dir/imageoutput.cpp.o
cd /builddir/build/BUILD/oiio-Release-1.1.3/build/linux/libOpenImageIO && /usr/bin/c++   -DBOOST_TEST_DYN_LINK -DEMBED_PLUGINS=1 -DOpenImageIO_EXPORTS -DPTEX_EXPORTS -DUSE_BOOST_ASIO=1 -DUSE_FIELD3D=1 -DUSE_FREETYPE -DUSE_OCIO=1 -DUSE_OPENJPEG -DUSE_TBB=0 -DUSE_WEBP=1 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -march=armv7-a -mfpu=vfpv3-d16  -mfloat-abi=hard  -O2 -g -DNDEBUG -fPIC -I/usr/include/OpenEXR -I/builddir/build/BUILD/oiio-Release-1.1.3/src/include -I/builddir/build/BUILD/oiio-Release-1.1.3/build/linux/include -I/usr/include/freetype2    -Wall -o CMakeFiles/OpenImageIO.dir/imageoutput.cpp.o -c /builddir/build/BUILD/oiio-Release-1.1.3/src/libOpenImageIO/imageoutput.cpp
{standard input}: Assembler messages:
{standard input}:7336: Error: bad instruction `pause'
{standard input}:7798: Error: bad instruction `pause'
{standard input}: Assembler messages:
{standard input}:5994: Error: bad instruction `pause'
{standard input}:6212: Error: bad instruction `pause'
{standard input}:6517: Error: bad instruction `pause'
make[2]: *** [libOpenImageIO/CMakeFiles/OpenImageIO.dir/imageio.cpp.o] Error 1

The offending file is:
oiio-Release-1.1.3/src/include/thread.h

The preprocessor #if defined had (__GNUC__)
there is a handler for archs that don't have 'pause'

This diff (attached) works to build it. I added a comment to where a specific __arm__ might be desirable.

diff: 

--- oiio-Release-1.1.3/src/include/thread.h.arm	2013-03-22 16:18:55.853070396 -0400
+++ oiio-Release-1.1.3/src/include/thread.h	2013-03-22 16:16:50.022584176 -0400
@@ -324,7 +324,7 @@
 {
 #if USE_TBB
     __TBB_Pause(delay);
-#elif defined(__GNUC__)
+#elif defined(__i386__) || defined(__x86_64__)
     for (int i = 0; i < delay; ++i) {
         __asm__ __volatile__("pause;");
     }
@@ -336,6 +336,7 @@
         _asm  pause
 #endif /* _WIN64 */
     }
+/* may need to #elif defined(__arm__) here */
 #else
     // No pause on this platform, just punt
     for (int i = 0; i < delay; ++i) ;


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 William Henry 2013-03-22 21:05:51 UTC
Carlos O'Donell made a suggestion about using usleep(). It might be worth considering and could be put where I have the # if defined (__arm__) commented placeholder.  But it builds as is - not sure how it runs :)

His suggestion:
"this can actually be
interrupted by a signal causing usleep to return EINTR
before having slept all of DELAY useconds. I figured
this is OK given the inaccurate nature of the function
and the use of x86 asm `pause.' I again assume that 
this pause () function is probably just used to yield
a busy-wait loop rather than to measure accurate time.

If you want a better implementation then you need to
start looking at ARM's YIELD, and only use it when
available in hardware e.g. >= ARMv6K || ARMv6T2.
On older hardware it will be treated as a NOP, which
might not yield a sufficient delay and burn power.
I don't know if userspace can actually use the YIELD
instruction since I've never tried it, but use usleep()
will certainly enter the kernel via nanosleep and yield
anyway."

Comment 2 Dan Horák 2013-03-22 21:41:28 UTC
FWIW build on s390(x) fails with the same symptom

Comment 3 Richard Shaw 2013-03-23 02:26:37 UTC
Upstream is quite active and willing to accept patches. I'll email the list and reference this bug to get their input.

Comment 4 Richard Shaw 2013-03-24 01:26:09 UTC
See the list thread here for suggested workarounds:

http://lists.openimageio.org/pipermail/oiio-dev-openimageio.org/2013-March/005953.html

Comment 5 Dan Horák 2013-03-24 12:48:26 UTC
Please keep in mind when communicating with upstream that we need also a generic solution, when an arch specific assembler code isn't provided. Thanks, Dan

Comment 6 Richard Shaw 2013-03-24 12:58:17 UTC
(In reply to comment #5)
> Please keep in mind when communicating with upstream that we need also a
> generic solution, when an arch specific assembler code isn't provided.
> Thanks, Dan

I don't know enough to even be dangerous, but this solution seems to fit that requirement:

http://lists.openimageio.org/pipermail/oiio-dev-openimageio.org/2013-March/005955.html

Comment 7 Peter Robinson 2013-03-31 23:59:52 UTC
Pulled in the upstream patch


Note You need to log in before you can comment on or make changes to this bug.