-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
ARROW-17511: [C++] Add support for xsimd 9.0.0 #13958
Conversation
|
@@ -2253,7 +2279,7 @@ else() | |||
endif() | |||
|
|||
if(ARROW_USE_XSIMD) | |||
resolve_dependency(xsimd REQUIRED_VERSION "8.1.0") | |||
resolve_dependency(xsimd REQUIRED_VERSIONS "9.0.0" "8.1.0") |
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.
Do we have to maintain this list manually in the future? What do you think @cyb70289 ?
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.
Not sure of the problem but looks inconvenient to maintain the list.
Can we only support the latest version?
Is it necessary to also update xsimd version in thirdparty/version.txt?
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.
Or can we support by default all versions above 8.1.0? If xsimd breaks compatibility, we can perhaps revisit.
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.
OK. I'll implement 8.1.0 or later condition.
FYI: I explain this case more:
- A CMake package can specify compatibility policy: https://cmake.org/cmake/help/latest/module/CMakePackageConfigHelpers.html#generating-a-package-version-file
- xsimd uses
SameMajorVersion
that means8.1.0
isn't compatible with9.0.0
- FYI: Our CMake package use
AnyNewerVersion
for convenient butSameMajorVersion
may be better because we use semver: https://github.com/apache/arrow/blob/master/cpp/cmake_modules/BuildUtils.cmake#L443
- FYI: Our CMake package use
- We can use
find_package()
to find a CMake package: https://cmake.org/cmake/help/latest/command/find_package.html - We can specify required (compatible) versions by range to
find_package()
but it requires CMake 3.19: https://cmake.org/cmake/help/latest/command/find_package.html#basic-signature-
A version range with the format
versionMin...[<]versionMax
where versionMin and versionMax have the same format and constraints on components being integers as the single version. By default, both end points are included. By specifying<
, the upper end point will be excluded. Version ranges are only supported with CMake 3.19 or later.
-
- But version range requires
versionMax
.- It means that we can't implement
AnyNewerVersion
behavior for aSameMajorVersion
CMake package only withfind_package()
options.
- It means that we can't implement
@github-actions crossbow submit test-conda-cpp-valgrind example-python-minimal-build-fedora-conda |
Revision: e3cb212 Submitted crossbow builds: ursacomputing/crossbow @ actions-7cc578a853
|
@github-actions crossbow submit test-conda-cpp-valgrind example-python-minimal-build-fedora-conda |
Revision: 805cfae Submitted crossbow builds: ursacomputing/crossbow @ actions-96469e3440
|
@github-actions crossbow submit test-conda-cpp-valgrind example-python-minimal-build-fedora-conda |
Revision: 63e745b Submitted crossbow builds: ursacomputing/crossbow @ actions-a68107599e
|
@github-actions crossbow submit test-conda-cpp-valgrind example-python-minimal-build-fedora-conda |
Revision: a81feaa Submitted crossbow builds: ursacomputing/crossbow @ actions-86064c15f3
|
+1 |
Benchmark runs are scheduled for baseline = f9c469d and contender = 04d2403. 04d2403 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
No description provided.