Skip to content

fix(simd): link simd_test against simd#1857

Open
wxyucs wants to merge 3 commits intomainfrom
fix-1846-simd-test-linkage
Open

fix(simd): link simd_test against simd#1857
wxyucs wants to merge 3 commits intomainfrom
fix-1846-simd-test-linkage

Conversation

@wxyucs
Copy link
Copy Markdown
Collaborator

@wxyucs wxyucs commented Apr 16, 2026

Summary

  • make simd_test explicitly link against simd so the test archive carries its direct SIMD implementation dependency
  • fix tests/unittests link failures when distribution SIMD flags do not include x86 ISA implementations, such as the scenario reported in build failed on ARM #1846
  • keep the change minimal and scoped to test target linkage without changing runtime dispatch behavior

Validation

  • cmake --build build-repro-issue1846 --target tests/unittests -j96
  • ./build-repro-issue1846/tests/unittests --list-tests

Closes #1846

Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
@wxyucs wxyucs requested review from LHT129 and inabao as code owners April 16, 2026 11:26
Copilot AI review requested due to automatic review settings April 16, 2026 11:26
@wxyucs wxyucs requested a review from jiaweizone as a code owner April 16, 2026 11:26
@mergify mergify bot added the module/simd label Apr 16, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Apr 16, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Require kind label

Wonderful, this rule succeeded.
  • label~=^kind/

🟢 Require version label

Wonderful, this rule succeeded.
  • label~=^version/

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the target_link_libraries for the simd_test target in src/simd/CMakeLists.txt to include the simd library. The reviewer suggested changing the linkage of the simd library from PUBLIC to PRIVATE for consistency and correcting the indentation to align with the project's style.

Comment thread src/simd/CMakeLists.txt Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a unit test link failure by making the simd_test static test library explicitly link against the simd implementation library, ensuring SIMD implementation symbols are available when tests/unittests links test archives on platforms/configurations where those symbols aren’t otherwise pulled in (e.g., ARM / limited distro SIMD flags).

Changes:

  • Update simd_test’s link libraries to include simd as a transitive dependency.

Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
Copilot AI review requested due to automatic review settings April 20, 2026 07:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread src/simd/CMakeLists.txt
Comment thread src/utils/CMakeLists.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build failed on ARM

2 participants