Adsk contrib - Add support for neon intrinsic integration

[cedrik-fuoco-adsk]

This is a PR that is replacing the PR about ARM Neon here: #1775.

The new PR contains all the previous code changes that were already accepted in the previous PR and contains new commits that were needed to integrate it with the code from the PR Add AVX2/AVX/SSE2 SIMD accelerated 1D/3D LUTS (merged in main already).

Therefore, please ignore the first commit named "Merging the previous ARM Neon branch".

[remia]

It seems this PR fixes the universal build from Intel based macOS, so it could fix the failing Python wheels nightly build, will be good to check, once this is merged.

[michdolan]

Added a couple really minor notes which could be cleaned up later.

[markreidvfx]

Turning off SSE2NEON with -DOCIO_USE_SSE2NEON=OFF fails to compile. Looks like clang is trying to compile SSE code as arm64.

In file included from /Users/mark/Dev/OpenColorIO/src/OpenColorIO/ops/cdl/CDLOpCPU.cpp:12:
/Users/mark/Dev/OpenColorIO/src/OpenColorIO/SSE.h:45:23: error: unknown type name '__m128'
        static inline __m128 _mm_max_ps(__m128 a, __m128 b)

On apple arm64 i'm getting 5 test_cpu_exec failed tests

[ 153/1049] [CPUProcessor / optimizations                                ] - FAILED
[ 417/1049] [FileFormatCTF / bake_1d_3d                                  ] - FAILED
[ 427/1049] [FileFormatHDL / bake_1d_shaper                              ] - FAILED
[ 462/1049] [FileFormatResolveCube / bake_1d_shaper                      ] - FAILED
[ 471/1049] [FileFormatSpi1D / bake_1d_shaper                            ] - FAILED

I don't think they are related to this pull request as I get the same ones from the current main branch. I haven't dug into it but they look kind of like rounding errors.

All the cpu test pass under rosetta.

[remia]

Commented on a few minor points. Looking at @markreidvfx latest comment, do we need to add / update the macOS build variants in CI to cover all the new configurations?

[cedrik-fuoco-adsk]

Thanks @remia, @markreidvfx and @michdolan for your comments.

Everything should be covered by the new commits.

do we need to add / update the macOS build variants in CI to cover all the new configurations?

At least, I think adding an entry for OCIO_USE_SSE2NEON=OFF in the macOS build variants would be good.
@remia, Is there a limit to the number of tasks in a particular workflow?

[remia]

@cedrik-fuoco-adsk I think only 20 jobs can run in parallel on a workflow but nothing prevent us from having more than 20, it will just take longer. Another possibility is also to add new variants to a nightly build if these are rare build settings.

[markreidvfx]

Building non universal arm64, I still needed to specify -DOCIO_USE_SSE2=ON etc to turn all these on. Even with -DOCIO_USE_SSE2NEON=ON. -DOCIO_USE_SSE2NEON=ON should probably imply all these SSE features are ON.

[cedrik-fuoco-adsk]

They should already be ON by default if building a non-universal arm64.
See in CMakeLists.txt

They are not turned off since the CheckSupportX86SIMD.cmake is not run on an arm64-only build. I've tested on my M1 and I get the expected CPUInfoConfig.h, It should go in that path here:

I've tested with ocioperf using only OCIO_USE_SSE2NEON=ON and OFF. With my heavy_transforms.clf and the line by line test, I get about 226 ms with -DOCIO_USE_SSE2NEON=ON and about 467 ms with -DOCIO_USE_SSE2NEON=OFF.

What do you mean by "still needed"? Did you do another build beforehand or it was from a cleaned directory? @markreidvfx

[markreidvfx]

They are not turning on for me when I build natively on arm64. I've tried cleaning my env several times.
I'm just doing the basic mkdir build && cd build && cmake .. && cmake --build .

and my CPUInfoConfig.h shows

#if (OCIO_ARCH_X86 && !defined(__aarch64__)) || (defined(__aarch64__) && OCIO_USE_SSE2NEON)
    #define OCIO_USE_SSE2 0
    #define OCIO_USE_SSE3 0
    #define OCIO_USE_SSSE3 0
    #define OCIO_USE_SSE4 0
    #define OCIO_USE_SSE42 0

It looks like its because CMAKE_OSX_ARCHITECTURES isn't set to anything when I build it this way.
That section in CMakeLists.txt needs it to be arm64.

(APPLE AND "${CMAKE_SYSTEM_PROCESSOR}" MATCHES "arm64" AND "${CMAKE_OSX_ARCHITECTURES}" MATCHES "arm64")

cmake .. -DCMAKE_OSX_ARCHITECTURES=arm64 works.

Should we set CMAKE_OSX_ARCHITECTURES = CMAKE_SYSTEM_PROCESSOR if undefined?

[cedrik-fuoco-adsk]

Oh right, I get it now. I'll prepare a new commit.

[cedrik-fuoco-adsk]

The latest commit should have fixed the issue (I've tested it on my M1). @markreidvfx

[markreidvfx]

That fixes it for me on my M2 mac too.

[markreidvfx]

I doubled checked, the order the architectures doesn't cause any issues, but we might want to reword this to show you need quotes around the architectures -DCMAKE_OSX_ARCHITECTURES="arm64;x86_64". It took me like 10mins to realize the semicolon ; in bash or zshell ends a command :p

[doug-walker]

Thanks for your help with this @markreidvfx !

We updated the install instructions accordingly. Going to merge this as the expected tests passed before we updated the documentation.