OpenColorIO: Add options for SIMD optimization support

[irieger]

Summary

Changes to recipe: opencolorio/2.3.2 and newer

Motivation

It was discovered, that there might be problems when building OpenColorIO for older systems not supporting all the instruction sets despite the build process trying to estimate what is supported on the given platform.
#25765

Details

Add options for the new flags supported in OpenColorIO since version 2.3.2. Older versions (I only compared the ones in CCI, so might be since 2.3.0 or so) had only a flag for cmake for OCIO_USE_SSE. Since 2.3.2 the pure SSE is no longer supported but OCIO_USE_SSE2 to OCIO_USE_AVX512 and OCIO_USE_F16C.

The change is implemented so that it supports the setting None to use the default and keep the current behaviour. Only overrides the automatism when set to True/False.

Open question: Would it make sense to specifically check for the version and only then set the variables? Up to now OCIO_USE_SSE is set even for the new versions which doesn't error but should also not have an effect as those variables aren't used in the newer CMakeLists. Assume the same for new variables when building 2.2.1 or some other older version.

---

• Read the contributing guidelines
• Checked that this PR is not a duplicate: list of PRs by recipe
• Tested locally with at least one configuration using a recent version of Conan

[irieger]

Trying to understand what goes wrong here.

[irieger]

I don't get why if not self.options.get_safe("use_sse2", None) is None is executed, where None is set in the default_options, while if not self.options.get_safe("use_sse3", None) is None is skipped, where I don't set the option so it uses the default passed to get_safe. Are this different Nones (ok, here the default is 'default' but I changed it to see if an actual value compares, see tests below where I check for default?

[irieger]

use_avx2 is set to default and the if is still true, while I commented out use_avx512 so it falls back to the default-param of get_safe, which is the same anyway. Don't get an output printed for use_avx512 but for use_avx2, where I'd expect both to not output anything in the default case!

[EstebanDugueperoux2]

Hi irieger,

Looking at https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/main/share/cmake/utils/CompilerFlags.cmake it seems that if OCIO_USE_SIMD is enabled, which is the default, all your flags are enabled depending of the detection scripts results (https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/main/share/cmake/utils/CheckSupportX86SIMD.cmake).

[irieger]

Hi irieger,

Looking at https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/main/share/cmake/utils/CompilerFlags.cmake it seems that if OCIO_USE_SIMD is enabled, which is the default, all your flags are enabled depending of the detection scripts results (https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/main/share/cmake/utils/CheckSupportX86SIMD.cmake).

Exactly. And my intention was to not change the behaviour - thus trying to have the None as the default case - and only optionally allow to force On/Off the specific optimizations. This would be so that like in your case, it can easily be overriden.
Setting defaults, besides changing away from the current behaviour, would result in quite a number of conditional which might silently ignore things. Having a default matching the library defaults but & allow to force would match the original behaviour, where the auto-detect is input as defaults for the options offered here.

[memsharded]

Thanks very much for your contribution!
Could you please have a look to the comments?

[memsharded]

The print() must be removed before merging. The ways to display information in recipes is via self.output.info/verbose/warning/.., but it seems this wouldn't be necessary, I'd remove the prints.

[memsharded]

It sounds like this logic could be done in a for-loop in a more compact way.

[memsharded]

This specific OCIO_USE_SSE I don't see it in https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/6fa40a4f6c3f0dd9f52f2476c3279927d5f23d71/CMakeLists.txt#L263, sure it exists?

[EstebanDugueperoux2]

It exists in older release, https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/2.1.1/CMakeLists.txt#L148

[memsharded]

I am trying to understand how the OpenColorIO handle those options.

I am searching in the codebase, and I can't find:

• That these options defined imply adding some flags like /arch:sse4.1 vs -msse4.1 (they are only added for the checks, but apparently not globally).
• These are mapped to some C++ code #define, but I don't find usages of those in the code either
• These options defined are silently ignored if the compiler doesn't support them:

  if(${OCIO_USE_SSE3} AND NOT ${COMPILER_SUPPORTS_SSE3})
   message(STATUS "OCIO_USE_SSE3 requested but compiler does not support, disabling")
   set(OCIO_USE_SSE3 OFF)
  endif()

Then if the usage of these SIMD is completely controlled by the project, no matter what we define (or the project should error, but it does not), they are already enabled by default. And still not fully sure how they affect the binary code.

I am most likely missing something there, any help understanding this would be helpful.

[irieger]

I am trying to understand how the OpenColorIO handle those options.

I am searching in the codebase, and I can't find:

* That these options defined imply adding some flags like ` /arch:sse4.1 vs -msse4.1` (they are only added for the checks, but apparently not globally).

* These are mapped to some C++ code `#define`, but I don't find usages of those in the code either

* These options defined are silently ignored if the compiler doesn't support them:
  ```cmake
  if(${OCIO_USE_SSE3} AND NOT ${COMPILER_SUPPORTS_SSE3})
   message(STATUS "OCIO_USE_SSE3 requested but compiler does not support, disabling")
   set(OCIO_USE_SSE3 OFF)
  endif()
  ```

Then if the usage of these SIMD is completely controlled by the project, no matter what we define (or the project should error, but it does not), they are already enabled by default. And still not fully sure how they affect the binary code.

I am most likely missing something there, any help understanding this would be helpful.

Thanks for your reply. I missed that part in the CMakeLists. Will have a closer look before cleaning up and see what makes sense.

In general the intention was to allow - based on the reports of problems - allow to force-deactivate some SIMD modes and by default fall back to the original behaviour of the recipe to auto-detect. As it can be force off at least, if still feasable - it seems there is other progress in the bug that triggered me to look into this - I can remove the force true if it doesn't work.

[memsharded]

In general the intention was to allow - based on the reports of problems - allow to force-deactivate some SIMD modes and by default fall back to the original behaviour of the recipe to auto-detect. As it can be force off at least, if still feasable - it seems there is other progress in the bug that triggered me to look into this - I can remove the force true if it doesn't work.

Wouldn't a patch to the CMake code that does the detection be better so it does the right thing? Like this seems a bug in the project CMakeLists.txt? Maybe it is worth to report it to the upstream OpenColorIO project?