GH-49579: [C++] Fix xsimd 14.1.0 build failure#49580
GH-49579: [C++] Fix xsimd 14.1.0 build failure#49580AntoinePrv wants to merge 1 commit intoapache:mainfrom
Conversation
|
|
|
@github-actions crossbow submit cran |
|
Revision: a242f77 Submitted crossbow builds: ursacomputing/crossbow @ actions-ef8fb85463
|
| } | ||
|
|
||
| // TODO(xsimd) bug fixed likely in xsimd>14.0.0 | ||
| // TODO(xsimd) bug fixed in xsimd 14.1.0 |
There was a problem hiding this comment.
Can we remove TODO because the bug was fixed?
| // TODO(xsimd) bug fixed in xsimd 14.1.0 | |
| // For a bug fixed in xsimd 14.1.0 |
There was a problem hiding this comment.
I think we should leave the TODO as this points to code to remove in the future.
|
|
||
| // Architecture for which there is no variable right shift but a larger fallback exists. | ||
| /// TODO(xsimd) Tracking for Avx2 in https://github.com/xtensor-stack/xsimd/pull/1220 | ||
| // TODO(xsimd) Tracking for Avx2 in https://github.com/xtensor-stack/xsimd/pull/1220 |
There was a problem hiding this comment.
It seems that the PR is also merged. Do we need #if ... for this too?
There was a problem hiding this comment.
We still need to keep it because some builds use a version of xsimd that does not yet have the fix.
Do we need
#if ...for this too?
Contrary to the fix in this PR, in this case the back-ported fix also works with newer xsimd versions, so we don't really need to.
| } | ||
|
|
||
| // TODO(xsimd) bug fixed likely in xsimd>14.0.0 | ||
| // TODO(xsimd) bug fixed in xsimd 14.1.0 |
There was a problem hiding this comment.
Can we remove TODO because the bug was fixed?
| // TODO(xsimd) bug fixed in xsimd 14.1.0 | |
| // Fox a bug fixed in xsimd 14.1.0 |
There was a problem hiding this comment.
IMHO we keep the TODO until (hopefully sooner rather than later) we bump again the minimum version of xsimd and delete that piece of code.
Rationale for this change
Fix the build on MacOS with newest xsimd version.
A fix for an internal bug in xsimd had been added in GH-49449 GH-49450 to boost performances.
Now that the fix has been merged in xsimd 14.1.0, the internals have changed leading to the failure.
The ported fix is kept for older xsimd versions until we can set a minimum xsimd version of 14.1.0.
What changes are included in this PR?
Remove a backported fix on xsimd versions that include it.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.