Skip to content

GH-49579: [C++] Fix xsimd 14.1.0 build failure#49580

Open
AntoinePrv wants to merge 1 commit intoapache:mainfrom
AntoinePrv:xsimd-fix
Open

GH-49579: [C++] Fix xsimd 14.1.0 build failure#49580
AntoinePrv wants to merge 1 commit intoapache:mainfrom
AntoinePrv:xsimd-fix

Conversation

@AntoinePrv
Copy link
Contributor

@AntoinePrv AntoinePrv commented Mar 23, 2026

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.

@github-actions
Copy link

⚠️ GitHub issue #49579 has been automatically assigned in GitHub to PR creator.

@raulcd raulcd requested a review from pitrou March 23, 2026 10:26
@pitrou pitrou added the CI: Extra: C++ Run extra C++ CI label Mar 23, 2026
@pitrou
Copy link
Member

pitrou commented Mar 23, 2026

@github-actions crossbow submit cran

@pitrou pitrou requested a review from kou March 23, 2026 13:30
@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 23, 2026
@github-actions
Copy link

Revision: a242f77

Submitted crossbow builds: ursacomputing/crossbow @ actions-ef8fb85463

Task Status
test-r-alpine-linux-cran GitHub Actions
test-r-linux-as-cran GitHub Actions
test-r-macos-as-cran GitHub Actions

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

}

// TODO(xsimd) bug fixed likely in xsimd>14.0.0
// TODO(xsimd) bug fixed in xsimd 14.1.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove TODO because the bug was fixed?

Suggested change
// TODO(xsimd) bug fixed in xsimd 14.1.0
// For a bug fixed in xsimd 14.1.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the PR is also merged. Do we need #if ... for this too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove TODO because the bug was fixed?

Suggested change
// TODO(xsimd) bug fixed in xsimd 14.1.0
// Fox a bug fixed in xsimd 14.1.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants