Skip to content

[Enhancement](compile) Specific x86-64-v3 arch when AVX2 enabled#62585

Open
zclllyybb wants to merge 1 commit intoapache:masterfrom
zclllyybb:x86_v3
Open

[Enhancement](compile) Specific x86-64-v3 arch when AVX2 enabled#62585
zclllyybb wants to merge 1 commit intoapache:masterfrom
zclllyybb:x86_v3

Conversation

@zclllyybb
Copy link
Copy Markdown
Contributor

AVX2 is only a part of x86-64-v3. when avx2 supported, it can be basically determined that x86-64-v3 is also supported. Enabling it introduces some performance benefits.

Copilot AI review requested due to automatic review settings April 17, 2026 08:27
@zclllyybb zclllyybb requested a review from BiteTheDDDDt as a code owner April 17, 2026 08:27
@Thearas
Copy link
Copy Markdown
Contributor

Thearas commented Apr 17, 2026

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@zclllyybb
Copy link
Copy Markdown
Contributor Author

/review

@github-actions
Copy link
Copy Markdown
Contributor

OpenCode automated review failed and did not complete.

Error: Review step was skipped (possibly timeout or cancelled)
Workflow run: https://github.com/apache/doris/actions/runs/24555619798

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

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

Updates backend build flags so that when USE_AVX2 is enabled, compilation targets the x86-64-v3 micro-architecture level for potential performance gains.

Changes:

  • Add -march=x86-64-v3 to BE compile options when USE_AVX2 is enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread be/CMakeLists.txt
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Summary

This change is not safe to merge as-is. -march=x86-64-v3 is broader than -mavx2, but Doris' runtime/build/release contract still models this switch as an AVX2-only split.

Critical Checkpoints

  • Goal of current task: Improve BE performance when USE_AVX2=ON. The flag may help on x86-64-v3 CPUs, but the PR does not fully accomplish the task safely because it silently raises the minimum ISA beyond AVX2 without updating the surrounding compatibility contract. No test was added to prove the new baseline is safe.
  • Modification size and focus: The code diff is tiny, but the behavioral impact is broad: every AVX2-enabled x86_64 BE build now targets x86-64-v3. The surrounding contract changes are missing.
  • Concurrency: Not applicable; this is a build-system change only.
  • Lifecycle / static initialization: Applicable indirectly. BE relies on an early startup instruction probe in be/src/service/doris_main.cpp, but that probe no longer matches the compiler target after this change.
  • Configuration items: No new config was added, but the meaning of existing USE_AVX2 effectively changes from "enable AVX2" to "require x86-64-v3". Existing docs, package naming, and runtime guidance are therefore stale.
  • Incompatible changes: Yes. This raises the minimum CPU feature set for the default AVX2-enabled BE binary. Compatibility handling is incomplete.
  • Parallel code paths: Related paths (build.sh, build-for-release.sh, and the BE startup instruction check) were not updated consistently with the new ISA baseline.
  • Special conditional checks: The if (USE_AVX2) condition is no longer precise enough for what is being enabled.
  • Test coverage: No build or runtime validation was added for CPUs/toolchains around the new boundary, and there is no regression coverage for the changed packaging/runtime contract.
  • Test results modified: No.
  • Observability: Existing startup error reporting is insufficient for the newly required feature set.
  • Transaction / persistence: Not applicable.
  • Data writes / modifications: Not applicable.
  • FE-BE variable passing: Not applicable.
  • Performance: The potential upside on x86-64-v3 CPUs is clear, but it comes with an unhandled compatibility regression.
  • Other issues: No additional blockers beyond the compatibility-contract mismatch below.

Comment thread be/CMakeLists.txt
@zclllyybb
Copy link
Copy Markdown
Contributor Author

run buildall

@zclllyybb
Copy link
Copy Markdown
Contributor Author

run beut

@zclllyybb
Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 71.94% (26851/37324)
Line Coverage 55.06% (285421/518395)
Region Coverage 51.99% (235785/453513)
Branch Coverage 53.41% (101839/190687)

@zclllyybb
Copy link
Copy Markdown
Contributor Author

skip buildall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants