Bug 270348 - graphics/openexr: build fails on armv7
Summary: graphics/openexr: build fails on armv7
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: arm Any
: --- Affects Some People
Assignee: Matthias Andree
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2023-03-19 19:25 UTC by Robert Clausecker
Modified: 2023-03-21 19:24 UTC (History)
0 users

See Also:
mandree: maintainer-feedback+


Attachments
try to fix ARMv7 compilation, patch #1 of 2 (746 bytes, patch)
2023-03-19 23:20 UTC, Matthias Andree
no flags Details | Diff
try to fix ARMv7 compilation, patch #2 of 2 (330 bytes, patch)
2023-03-19 23:22 UTC, Matthias Andree
no flags Details | Diff
openexr-3.1.6 armv7 build log w/ attachments 24099{5,6} (108.80 KB, text/plain)
2023-03-21 13:14 UTC, Robert Clausecker
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Clausecker freebsd_committer freebsd_triage 2023-03-19 19:25:02 UTC
The port doesn't build on armv7.  I can try to investigate, but will be busy for the next few days.
The undeclared function looks like it's a NEON intrinsic.  So probably some compiler option is missing or it tries to build aarch64 NEON code on armv7.

[  6% 18/283] /usr/local/libexec/ccache/cc -DOPENEXRCORE_EXPORTS -DOpenEXRCore_EXPORTS -I/wrkdirs/usr/ports/graphics/openexr/work/.build/src/lib/OpenEXRCore -I/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore -I/wrkdirs/usr/ports/graphics/openexr/work/.build/cmake -isystem /usr/local/include -isystem /usr/local/include/Imath -O2 -pipe  -fstack-protector-strong -fno-strict-aliasing -O2 -pipe  -fstack-protector-strong -fno-strict-aliasing  -DNDEBUG -fPIC -fvisibility=hidden -MD -MT src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o -MF src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o.d -o src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o -c /wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c
FAILED: src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o 
/usr/local/libexec/ccache/cc -DOPENEXRCORE_EXPORTS -DOpenEXRCore_EXPORTS -I/wrkdirs/usr/ports/graphics/openexr/work/.build/src/lib/OpenEXRCore -I/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore -I/wrkdirs/usr/ports/graphics/openexr/work/.build/cmake -isystem /usr/local/include -isystem /usr/local/include/Imath -O2 -pipe  -fstack-protector-strong -fno-strict-aliasing -O2 -pipe  -fstack-protector-strong -fno-strict-aliasing  -DNDEBUG -fPIC -fvisibility=hidden -MD -MT src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o -MF src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o.d -o src/lib/OpenEXRCore/CMakeFiles/OpenEXRCore.dir/internal_zip.c.o -c /wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c
/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c:118:17: warning: implicit declaration of function 'vqtbl1q_u8' is invalid in C99 [-Wimplicit-function-declaration]
        vPrev = vqtbl1q_u8 (d, shuffleMask);
                ^
/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c:118:15: error: assigning to 'uint8x16_t' (vector of 16 'uint8_t' values) from incompatible type 'int'
        vPrev = vqtbl1q_u8 (d, shuffleMask);
              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c:190:25: warning: implicit declaration of function 'vzip1q_u8' is invalid in C99 [-Wimplicit-function-declaration]
        uint8x16_t lo = vzip1q_u8 (a, b);
                        ^
/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c:190:20: error: initializing 'uint8x16_t' (vector of 16 'uint8_t' values) with an expression of incompatible type 'int'
        uint8x16_t lo = vzip1q_u8 (a, b);
                   ^    ~~~~~~~~~~~~~~~~
/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c:191:25: warning: implicit declaration of function 'vzip2q_u8' is invalid in C99 [-Wimplicit-function-declaration]
        uint8x16_t hi = vzip2q_u8 (a, b);
                        ^
/wrkdirs/usr/ports/graphics/openexr/work/openexr-3.1.6/src/lib/OpenEXRCore/internal_zip.c:191:20: error: initializing 'uint8x16_t' (vector of 16 'uint8_t' values) with an expression of incompatible type 'int'
        uint8x16_t hi = vzip2q_u8 (a, b);
                   ^    ~~~~~~~~~~~~~~~~
3 warnings and 3 errors generated.
Comment 1 Matthias Andree freebsd_committer freebsd_triage 2023-03-19 21:49:26 UTC
Robert, thanks for the report. I'll poke at it.
Comment 2 Matthias Andree freebsd_committer freebsd_triage 2023-03-19 22:30:54 UTC
Looks like a genuine upstream bug introduced into 3.1.6 with https://github.com/AcademySoftwareFoundation/openexr/pull/1348 or https://github.com/AcademySoftwareFoundation/openexr/commit/677c6a52d8cde3b8630932a93b631d2d4e68ab52

This seems to only branch on __ARM_NEON without looking further if it's on __aarch64 or not.

I have forwarded this upstream to https://github.com/AcademySoftwareFoundation/openexr/issues/1365

Robert: if I were to back out that particular commit only for ARMv7 in case they don't manage a fix in time, how would I word the if I am on "ARM but not AARCH64" in the port's Makefile?
Comment 3 Matthias Andree freebsd_committer freebsd_triage 2023-03-19 23:20:10 UTC
Created attachment 240995 [details]
try to fix ARMv7 compilation, patch #1 of 2
Comment 4 Matthias Andree freebsd_committer freebsd_triage 2023-03-19 23:22:01 UTC
Created attachment 240996 [details]
try to fix ARMv7 compilation, patch #2 of 2

Robert, please put both patches into ports/graphics/openexr/files/ and recompile on ARMv7 to see if that fixes the issue. If you can, please also try on AArch64 and check if it still enables the reconstruct_neon and interleave_neon there.
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-03-19 23:29:14 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=c185d09d3e1e2e3881284b8eca93028db5575c47

commit c185d09d3e1e2e3881284b8eca93028db5575c47
Author:     Matthias Andree <mandree@FreeBSD.org>
AuthorDate: 2023-03-19 23:26:53 +0000
Commit:     Matthias Andree <mandree@FreeBSD.org>
CommitDate: 2023-03-19 23:28:16 +0000

    graphics/openexr: mark BROKEN on armv7

    Upstream bug report:
    https://github.com/AcademySoftwareFoundation/openexr/issues/1365

    PR:             270348

 graphics/openexr/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Comment 6 Matthias Andree freebsd_committer freebsd_triage 2023-03-20 10:01:37 UTC
Upstream project's contributor wrote he will follow a similar approach as proposed here later today.
Comment 7 Robert Clausecker freebsd_committer freebsd_triage 2023-03-20 12:41:02 UTC
I'll go ahead and test your patch as soon as my armv7 box is done build-testing my current set of ports patches.
Comment 8 Robert Clausecker freebsd_committer freebsd_triage 2023-03-21 13:14:29 UTC
Created attachment 241037 [details]
openexr-3.1.6 armv7 build log w/ attachments 24099{5,6}

The patch set you posted does not fix the build.

Please see attached build log.
Comment 9 Matthias Andree freebsd_committer freebsd_triage 2023-03-21 15:20:09 UTC
Comment on attachment 241037 [details]
openexr-3.1.6 armv7 build log w/ attachments 24099{5,6}

Uh. Can you re-run with "make -k" so we trap all errors? ImfDwaCompressorSimd.h wasn't on my desk so far, and also isn't addressed by upstream.
Comment 10 Robert Clausecker freebsd_committer freebsd_triage 2023-03-21 15:28:56 UTC
I don't have time for additional runs today.  Maybe tomorrow.  I can give you access to an armv7 box for testing if you like (send me an email).
Comment 11 Matthias Andree freebsd_committer freebsd_triage 2023-03-21 15:36:07 UTC
I should be more precise, and the failures you are now posting seem to be related to https://github.com/AcademySoftwareFoundation/openexr/commit/436fcd2829ae9a8965af1db15ac8531fdc8b96ce

I've sent ssh keys by direct mail to your FreeBSD address.
Comment 12 Matthias Andree freebsd_committer freebsd_triage 2023-03-21 17:01:31 UTC
DwaCompressor NEON on ARMv7 bugs filed as https://github.com/AcademySoftwareFoundation/openexr/issues/1367.

I have local patches that, together with PR1366 from upstream, fix the ARMv7 builds, but I need to have a look at the ARMv8/AArch64 output files to make sure we don't void the AARCH64 (NEON) optimizations there.
Comment 13 commit-hook freebsd_committer freebsd_triage 2023-03-21 18:25:33 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=54d6860aac464b690f37007cfaef52c5a0b427b0

commit 54d6860aac464b690f37007cfaef52c5a0b427b0
Author:     Matthias Andree <mandree@FreeBSD.org>
AuthorDate: 2023-03-21 16:42:19 +0000
Commit:     Matthias Andree <mandree@FreeBSD.org>
CommitDate: 2023-03-21 18:24:38 +0000

    graphics/openexr: Fix ARMv7 build

    OpenEXR 3.1.6 introduced several NEON-based optimizations
    that implied Aarch64. Add patched, either picked from
    upstream, or written by mandree@, to enable those
    NEON features that also require Aarch64 only there.

    PR-1366 is cherry-picked from upstream, and patch-lib/patch-test files
    are my work but build upon said PR.

    Also cherry-pick PR1354 that adds a missing check for AVX,
    which is why I am bumping PORTREVISION because it might change
    code (I have not checked).

    https://github.com/AcademySoftwareFoundation/openexr/issues/1365

    PR:             270348
    Reported by:    fuz@ (Robert Clausecker)

 graphics/openexr/Makefile                          |  6 +-
 ...-3f97750d1ec203e7d7eb8d5f30f3d5e7e68ad720 (new) | 25 ++++++
 ...-a41a736d64e3d93baffef1042d4a3d1aaf74f1c9 (new) | 98 ++++++++++++++++++++++
 .../patch-lib_OpenEXR_ImfDwaCompressor.cpp (new)   | 11 +++
 .../patch-lib_OpenEXR_ImfDwaCompressorSimd.h (new) | 29 +++++++
 ...est_OpenEXRTest_testDwaCompressorSimd.cpp (new) | 38 +++++++++
 6 files changed, 203 insertions(+), 4 deletions(-)
Comment 14 Matthias Andree freebsd_committer freebsd_triage 2023-03-21 18:40:23 UTC
Local FreeBSD changes forwarded upstream as https://github.com/AcademySoftwareFoundation/openexr/pull/1368 or https://github.com/AcademySoftwareFoundation/openexr/commit/8d374c9e73a4867bacfbae401c34357243641ea0.

files/patch-PR1366-* has obsoleted the two previous patches I had proposed, which had been a partial fix to the issue (as is upstream's pull request PR1366).
Comment 15 commit-hook freebsd_committer freebsd_triage 2023-03-21 19:24:43 UTC
A commit in branch 2023Q1 references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=b7e05a72e644249c32c2e723d015b7af444c8690

commit b7e05a72e644249c32c2e723d015b7af444c8690
Author:     Matthias Andree <mandree@FreeBSD.org>
AuthorDate: 2023-03-21 16:42:19 +0000
Commit:     Matthias Andree <mandree@FreeBSD.org>
CommitDate: 2023-03-21 19:22:30 +0000

    graphics/openexr: Fix ARMv7 build

    OpenEXR 3.1.6 introduced several NEON-based optimizations
    that implied Aarch64. Add patched, either picked from
    upstream, or written by mandree@, to enable those
    NEON features that also require Aarch64 only there.

    PR-1366 is cherry-picked from upstream, and patch-lib/patch-test files
    are my work but build upon said PR.

    Also cherry-pick PR1354 that adds a missing check for AVX,
    which is why I am bumping PORTREVISION because it might change
    code (I have not checked).

    https://github.com/AcademySoftwareFoundation/openexr/issues/1365

    PR:             270348
    Reported by:    fuz@ (Robert Clausecker)

    (cherry picked from commit 54d6860aac464b690f37007cfaef52c5a0b427b0)

 graphics/openexr/Makefile                          |  4 +-
 ...-3f97750d1ec203e7d7eb8d5f30f3d5e7e68ad720 (new) | 25 ++++++
 ...-a41a736d64e3d93baffef1042d4a3d1aaf74f1c9 (new) | 98 ++++++++++++++++++++++
 .../patch-lib_OpenEXR_ImfDwaCompressor.cpp (new)   | 11 +++
 .../patch-lib_OpenEXR_ImfDwaCompressorSimd.h (new) | 29 +++++++
 ...est_OpenEXRTest_testDwaCompressorSimd.cpp (new) | 38 +++++++++
 6 files changed, 203 insertions(+), 2 deletions(-)