Created attachment 150626 [details] patch to unbreak graphics/OpenEXR build when CPUTYPE is set If CPUTYPE is set in make.conf to a value that enables SSE2 when building graphics/OpenEXR with gcc on i386, the build fails with this error: gcc48 -MT ImfSystemSpecific.lo -MD -MP -MF .deps/ImfSystemSpecific.Tpo -c ImfSystemSpecific.cpp -fPIC -DPIC -o .libs/ImfSystemSpecific.o libtool: compile: g++48 -DHAVE_CONFIG_H -I. -I../config -I.. -I../config -pthread -I/usr/local/include/OpenEXR -I. -I../IlmImf -pipe -O2 -pipe -march=pentium-m -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing -Wl,-rpath=/usr/local/lib/gcc48 -MT ImfCompositeDeepScanLine.lo -MD -MP -MF .deps/ImfCompositeDeepScanLine.Tpo -c ImfCompositeDeepScanLine.cpp -o ImfCompositeDeepScanLine.o >/dev/null 2>&1 ImfSystemSpecific.cpp: In constructor 'Imf_2_2::CpuId::CpuId()': ImfSystemSpecific.cpp:51:29: error: inconsistent operand constraints in an 'asm' : /* Clobber */); ^ ImfSystemSpecific.cpp:51:29: error: inconsistent operand constraints in an 'asm' : /* Clobber */); ^ Makefile:669: recipe for target 'ImfSystemSpecific.lo' failed gmake[2]: *** [ImfSystemSpecific.lo] Error 1 The problem is in this block of code: #if defined(IMF_HAVE_SSE2) && defined(__GNUC__) // Helper functions for gcc + SSE enabled void cpuid(int n, int &eax, int &ebx, int &ecx, int &edx) { __asm__ __volatile__ ( "cpuid" : /* Output */ "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx) : /* Input */ "a"(n) : /* Clobber */); } #else // IMF_HAVE_SSE2 && __GNUC__ The problem is that this code is clobbering the ebx register which is reserved for the global offset table when compiling -fPIC on i386. The attached patch reimplments the cpuid() function using __cpuid() from the gcc include file <cpuid.h> in place of the inline asm. See also: <https://github.com/openexr/openexr/issues/128>
Auto-assigned to maintainer mandree@FreeBSD.org
If you can successfully build a package with the port and OpenEXR run-time tests succeed for you, permission granted to commit. Feel free to reassign back to me if you want me to run tests or handle the commit.
Created attachment 151374 [details] patch to unbreak graphics/OpenEXR build when CPUTYPE is set and unbreak regression test compilation Same CPUTYPE fix as in the previous patch. The regression test does not compile on FreeBSD. Include <unidstd.h> as is done for Linux and Darwin to unbreak it. Remove an extraneous blank line in the Makefile, which is now a portlint fatal error.
Created attachment 151375 [details] .tar.gz archive of poudriere build logs and regression results on FreeBSD 8.4 and 10.0, and both amd64 and i386 This attachment is a .tar.xz archive of the poudriere build logs and the results of the port regression tests for FreeBSD 8.4 (using gcc from ports) and FreeBSD 10.0 (using clang). All were built with CPUTYPE=athlon64. Only the 8.4/i386 version should be affected by the CPUTYPE patch. In all cases, the regression test fails because the file comp_dwaa_v1.exr is missing. The tests up to that point seem to pass. On FreeBSD 10.0, both i386 and amd64 give nearly identical results. On FreeBSD 8.4, both i386 and amd64 give nearly identical results.
Created attachment 151695 [details] updated regression test results on FreeBSD 8.4 i386 The previous regression test failures were caused by a number of .exr files that were ommitted from the source tarball, as mentioned in this thread: <https://www.mail-archive.com/openexr-devel@nongnu.org/msg01877.html> After downloading the missing .exr files and copying them to the appropriate directory, the regression tests run further until I hit the NaN vs. NaN comparison issue mentioned in this thread: <https://lists.gnu.org/archive/html/openexr-devel/2014-08/msg00030.html>. The workaround of adding either -fno-inline or -ffloat-store to CFLAGS when building the test suite did not work, so I make this change instead: --- IlmImfTest/testOptimizedInterleavePatterns.cpp.orig 2014-08-09 19:03:49.000000000 -0700 +++ IlmImfTest/testOptimizedInterleavePatterns.cpp 2015-01-15 00:52:18.000000000 -0800 @@ -226,7 +226,8 @@ writtenHalf=half(i.slice().fillValue); } - if (writtenHalf.bits()!=readHalf.bits()) + if (writtenHalf.bits()!=readHalf.bits() && + !(writtenHalf.isNan() && readHalf.isNan())) { if (nonfatal) { @@ -235,7 +236,8 @@ else { cout << "\n\nerror reading back channel " << i.name() << " pixel " << x << ',' << y << " got " << readHalf << " expected " << writtenHalf << endl; - assert(writtenHalf.bits()==readHalf.bits()); + assert(writtenHalf.bits()==readHalf.bits() || + (writtenHalf.isNan() && readHalf.isNan())); exit(1); } } That allowed the library test suite to run to completion. The utility test suite then failed to compile because it had not been patched to #include <unistd.h>. In any case, I believe this shows that the patched library is functional.
I am currently in the process of testing things on 9.3 i386 (bare metal) and amd64 (poudriere), and moving the regression test fixes into the port as well. Currently looking good, I could reproduce the problem on 9.3 with CPUTYPE?=k8-sse3 and saw it go away with the patch. Don, this has been an enormous help - thank you very much. Would you like to take over maintainership for OpenEXR and ilmbase, you seem to be using it to some more extent than I do.
(That is not to say I want to get rid of it, I am only asking if there is someone better suited to maintaining it than I am; I am also happy to keep it.)
(In reply to Matthias Andree from comment #7) I don't really use this port. It's just required by some other ports that I do use.
Fixed in r377376. 9.3 amd64 and i386 now pass.
A commit references this bug: Author: mandree Date: Mon Jan 19 01:46:17 UTC 2015 New revision: 377376 URL: https://svnweb.freebsd.org/changeset/ports/377376 Log: Fix compilation if SSE2 is enabled on i386. While here, fix regression tests and see that things are fine. (Some regression-test issues remain for largestack, the default build passes make regression now on i386 and amd64). (No revision bump because these are build fixes, and the regression test stuff does not become part of the installed material.) Assisted by: truckman@ PR: 196014 Submitted by: truckman@ Changes: head/graphics/OpenEXR/Makefile head/graphics/OpenEXR/distinfo head/graphics/OpenEXR/files/patch-IlmImfTest__main.cpp head/graphics/OpenEXR/files/patch-IlmImfTest_testOptimizedInterleavePatterns.cpp head/graphics/OpenEXR/files/patch-IlmImfUtilTest_main.cpp head/graphics/OpenEXR/files/patch-IlmImf__ImfSystemSpecific.cpp