-
Notifications
You must be signed in to change notification settings - Fork 648
fix: Add arm_neon.h include on Windows ARM64 with clang-cl #4691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Add arm_neon.h include on Windows ARM64 with clang-cl #4691
Conversation
Signed-off-by: Anthony Roberts <[email protected]>
|
|
lgritz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Can you do the CLA signing so I can merge it?
|
Yes - just waiting on the relevant person at our company to check it all off for me on the EasyCLA dashboard! |
|
Any luck, @anthony-linaro ? (No rush, just letting you know we have not forgotten about you.) |
|
Not yet, sorry! Chasing with management chain |
|
@anthony-linaro Checking in. Still no rush, just making sure neither of us forgets completely about it. |
|
Sorry no progress :( apparently they're having some sort of issues with the corporate CLA portal |
|
@lgritz The CLA issue is now resolved, apologies for the delay! |
lgritz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
1516226
into
AcademySoftwareFoundation:main
…SoftwareFoundation#4691) On Windows ARM64, when building with clang-cl, LLVM can sometimes use it's own built-in intrin.h file, located in `<LLVM INSTALL PATH>\lib\clang\20\include`. This file, as opposed to the built-in one that ships with the Windows SDK that MSVC uses, doesn't have an inclusion of arm_neon.h. This PR adds a small check to the simd.h file to include this header. I didn't add an extra check to specifically only do this for clang, as it seemed a little messy - I can do this, but it's relatively harmless for MSVC given intrin.h includes it anyway for that compiler. This change has already been in place for some time in Blender, this is just an upstreaming PR for that change: https://projects.blender.org/blender/blender/src/commit/ea604b6b80e49adb51d12df4e4edd5af1c51f446/build_files/build_environment/patches/oiio_windows_arm64.diff
…SoftwareFoundation#4691) On Windows ARM64, when building with clang-cl, LLVM can sometimes use it's own built-in intrin.h file, located in `<LLVM INSTALL PATH>\lib\clang\20\include`. This file, as opposed to the built-in one that ships with the Windows SDK that MSVC uses, doesn't have an inclusion of arm_neon.h. This PR adds a small check to the simd.h file to include this header. I didn't add an extra check to specifically only do this for clang, as it seemed a little messy - I can do this, but it's relatively harmless for MSVC given intrin.h includes it anyway for that compiler. This change has already been in place for some time in Blender, this is just an upstreaming PR for that change: https://projects.blender.org/blender/blender/src/commit/ea604b6b80e49adb51d12df4e4edd5af1c51f446/build_files/build_environment/patches/oiio_windows_arm64.diff
Description
On Windows ARM64, when building with
clang-cl, LLVM can sometimes use it's own built-inintrin.hfile, located in<LLVM INSTALL PATH>\lib\clang\20\include.This file, as opposed to the built-in one that ships with the Windows SDK that MSVC uses, doesn't have an inclusion of
arm_neon.h. This PR adds a small check to thesimd.hfile to include this header.I didn't add an extra check to specifically only do this for clang, as it seemed a little messy - I can do this, but it's relatively harmless for MSVC given
intrin.hincludes it anyway for that compiler.This change has already been in place for some time in Blender, this is just an upstreaming PR for that change: https://projects.blender.org/blender/blender/src/commit/ea604b6b80e49adb51d12df4e4edd5af1c51f446/build_files/build_environment/patches/oiio_windows_arm64.diff
Tests
n/a
Checklist:
[ ] I have updated the documentation, if applicable. (Check if there is noneed to update the documentation, for example if this is a bug fix that
doesn't change the API.)
[ ] I have ensured that the change is tested somewhere in the testsuite(adding new test cases if necessary).
[ ] If I added or modified a C++ API call, I have also amended thecorresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.