Skip to content

Conversation

@patlefort
Copy link

This PR check for compiler flags and disable corresponding SIMD support when the flag is not detected.

@aous72
Copy link
Owner

aous72 commented Oct 10, 2025

Dear @patlefort ,

Thank you for this PR. This is a useful idea indeed.
I am traveling now, and this will take some time for me to integrate it.

Do you know if this works for macOS multi-architecture build? using -DCMAKE_OSX_ARCHITECTURES="arm64;x86_64"?

Thank you.

Kind regards,
Aous.

@patlefort
Copy link
Author

I'm guessing that it won't. I don't have a mac to test, but after research, I'm assuming that it will try to compile both architectures and would have to succeed for both. It might be more prudent to check those flag in the c++ code, but it will be less pretty. Should I do that?

@patlefort
Copy link
Author

I didn't realize it was doing runtime check for simd support. I still think some platform checks could be replaced by specific simd flags instead. As for multi architecture builds, won't setting simd flags (

set_source_files_properties(codestream/ojph_codestream_sse.cpp PROPERTIES COMPILE_FLAGS -msse)
) in your cmake file cause issues, if one platform doesn't support it?

It's possible to combine multiple binaries to a single universal binary using lipo later, so I'm not convinced that using CMAKE_OSX_ARCHITECTURES with multiple architectures is worth it. CMake is not well equipped to handle that case.

In any cases, I'm going to make some changes.

@aous72
Copy link
Owner

aous72 commented Oct 11, 2025

Hi @patlefort,

I do not want you to spend a lot of time on this; everyone's time is precious.

As you realized, the code does runtime detection of CPU features.
Then, the library uses different code paths depending on the SIMD features that are supported by the CPU.
The SIMD flags here are for debugging and to support compilers that might not have these SIMD flags.

Did you face a compiler that does not support some of these SIMD flags?
If not, then perhaps, we should not introduce additional options until they are needed; compilers are usually the first to implement SIMD instructions before libraries start using them.
Also in the current library, end users can disable some SIMD instructions if they wish to.

I have an old mac machine, and I was thinking of running this PR with multi-architecture build and see what happens.

Kind regards,
Aous.

@fundawang
Copy link

fundawang commented Oct 18, 2025

I think at least neon and sse(including avx etc.) should be distinguished. Because neon does not exsit in x86 targets, the same for SSEses. There is no need build them under different targets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants