[Enhancement](compile) Specific x86-64-v3 arch when AVX2 enabled#62585
[Enhancement](compile) Specific x86-64-v3 arch when AVX2 enabled#62585zclllyybb wants to merge 1 commit intoapache:masterfrom
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
OpenCode automated review failed and did not complete. Error: Review step was skipped (possibly timeout or cancelled) Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
There was a problem hiding this comment.
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-v3to BE compile options whenUSE_AVX2is enabled.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
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 onx86-64-v3CPUs, 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_AVX2effectively 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-v3CPUs is clear, but it comes with an unhandled compatibility regression. - Other issues: No additional blockers beyond the compatibility-contract mismatch below.
|
run buildall |
|
run beut |
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
skip buildall |
AVX2is only a part ofx86-64-v3. when avx2 supported, it can be basically determined thatx86-64-v3is also supported. Enabling it introduces some performance benefits.