Don't build against lzcnt by default and enable USE_SSE42 only on Intel architecture#5871
Merged
sturnclaw merged 3 commits intopioneerspacesim:masterfrom Aug 12, 2024
Merged
Conversation
- Move the lzcnt instruction flag to AVX2 builds (as it was introduced with Haswell) - Only default USE_SSE42 on for x86/64 builds; better support ARM64 builds - Add proper build flags for the target architecture
Member
Author
|
CC @pcercuei - are there any other changes that should be made to make the Flatpak build easier to maintain? |
Contributor
|
Your TargetArchitecture.cmake looks overly complicated, all it needs is if (${CMAKE_SYSTEM_PROCESSOR} MATCHES x86|x64)
set(PIONEER_TARGET_INTEL ON)
endif()You could check for ARM in there too, but you're not using it anywhere, so just introduce it when you need it. Note that for Flatpaks I'll have to force-disable those two options anyway. |
- We don't currently have any code that benefits from AVX2 except compiler autovectorization, so there's little point to building and hosting artifacts related to AVX2.
Member
Author
|
Thanks! I hadn't realized CMake was smart enough for regex matches, that makes this code much easier to express. Regarding feature flags that need to be disabled - AVX2 is no longer enabled by default in any capacity, and SSE4.2 is default-enabled only on Intel targets for parity between user-compiled checkouts of Pioneer and the official builds we ship. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses some slightly over-zealous feature enablement from the previous release cycle that we are not yet materially using. The build system now only defaults the USE_SSE42 flag to 'on' if we're compiling for a non-ARM64 target, and we no longer enable utilization of the
lzcntinstruction for SSE4.2 builds - it was introduced in the Haswell microarchitecture alongside AVX2 and as such has been moved to that build option.I've also added feature-test flags for the build script to use in the future to detect whether we're compiling for an ARM64 target or an Intel target (and whether we're compiling for 32 or 64-bit x86 architecture).