AcademySoftwareFoundation / OpenImageIO

Reading, writing, and processing images in a wide variety of file formats, using a format-agnostic API, aimed at VFX applications.
https://openimageio.readthedocs.org
Apache License 2.0
1.97k stars 595 forks source link

[BUILD] issue on aarch64 neon asm enabled on arm32 #3360

Closed kwizart closed 2 years ago

kwizart commented 2 years ago

Describe the bug When building a dependency with armv7 neon enabled (-fpu=neon), oiio fail with the following error:

In file included from /builddir/build/BUILD/olive-41a49c4880e21b11b08de94d2df1b227bf219fc6/app/codec/ffmpeg/ffmpegdecoder.cpp:32:
In file included from /usr/include/OpenImageIO/imagebuf.h:16:
In file included from /usr/include/OpenImageIO/fmath.h:45:
/usr/include/OpenImageIO/simd.h:4753:12: error: use of undeclared identifier 'vaddvq_s32'; did you mean 'vpaddlq_s32'?
    return vaddvq_s32(v);
           ^
/usr/lib/clang/13.0.1/include/arm_neon.h:18534:16: note: 'vpaddlq_s32' declared here
__ai int64x2_t vpaddlq_s32(int32x4_t __p0) {

To Reproduce Steps to reproduce the behavior:

  1. build (or cross-compile) oiio on armv7 with neon enabled.

Expected behavior Build should succeed, but fails because vaddvq_s32 is only available on aarch64 and not on armv7. Most of the time armv7 (distro/)users don't explicitly enable neon.

Evidence vaddvq_s32 is tagged as aarch64 only in this documentation. https://arm-software.github.io/acle/neon_intrinsics/advsimd.html

Platform information:

lgritz commented 2 years ago

Do you happen to know which preprocessor symbols I can use to distinguish between armv7 vs aarch64?

lgritz commented 2 years ago

I think it's __aarch64__ I will make a patch.

lgritz commented 2 years ago

Proposed fix in #3361. I don't have a way to test this directly. @kwizart can you try that patch on your end to verify?

kwizart commented 2 years ago

Thanks, testing in progress. Hopefully I will report in the next hours.

kwizart commented 2 years ago

Seems like the build is failing with -fpu=neon on armv7hl, other issue arise: https://kojipkgs.fedoraproject.org//work/tasks/7125/83647125/build.log

lgritz commented 2 years ago

Yes, thanks for the help, the logs make it apparent that there are a couple additional instructions we were using that are only available on aarch64, not all NEON. I've updated the patch with some additional fixes.

lgritz commented 2 years ago

@kwizart Did you have a chance to test the latest attempt in #3361? Does that complete the job of fixing this issue?