vectorscan: 5.4.10.1 -> 5.4.11 #303207
Merged
vectorscan: 5.4.10.1 -> 5.4.11 #303207
Conversation
This file contains 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
tnias
force-pushed
the
update/vectorscan-5.4.11
branch
3 times, most recently
from
2b14e8c
to
24af5ca
Compare
13 tasks
|
Hey @vlaci, could you check this PR? It uses a more recent boost version directly here in nixpkgs. I believe this allows you to drop some override stuff around libcxx/boost from your pyperscan repo. |
Atemu
reviewed
May 14, 2024
There was a problem hiding this comment.
Some small things, otherwise this LGTM.
Result of nixpkgs-review pr 303207 run on x86_64-linux 1
5 packages built:
- python311Packages.pyperscan
- python311Packages.pyperscan.dist
- python312Packages.pyperscan
- python312Packages.pyperscan.dist
- vectorscan
Comment on lines
63
to
69
| checkPhase = '' | ||
| ./bin/unit-hyperscan | ||
| ''; |
There was a problem hiding this comment.
Suggested change
| checkPhase = '' | |
| ./bin/unit-hyperscan | |
| ''; | |
| checkPhase = '' | |
| runHook preCheck | |
| ./bin/unit-hyperscan | |
| runHook postCheck | |
| ''; |
| # potentially available more modern hardware extensions (e.g. x86_64 with AVX512). | ||
| cmakeFlags = [ (if enableShared then "-DBUILD_SHARED_LIBS=ON" else "BUILD_STATIC_LIBS=ON") ] | ||
| ++ | ||
| (if (stdenv.isLinux && (stdenv.isx86_32 || stdenv.isx86_64)) then |
There was a problem hiding this comment.
Nit: We only have a limited set of hostPlatforms for which this applies; why not simply list them?
Suggested change
| (if (stdenv.isLinux && (stdenv.isx86_32 || stdenv.isx86_64)) then | |
| (if lib.elem stdenv.hostPlatform.system [ "x86_64-linux" "i686-linux" ] then |
There was a problem hiding this comment.
Nit: You could do the same for the aarch64-linux check but I won't block on this.
tnias
force-pushed
the
update/vectorscan-5.4.11
branch
from
24af5ca
to
111f1da
Compare
The darwin (or libcxxStdenv) builds use C++17, which only seems to work
with boost >= 1.83.
The errors look like this:
include/boost/functional.hpp:454:62: error: no template named 'binary_function' in namespace 'boost::functional::detail'; did you mean '__binary_function'?
class mem_fun1_ref_t : public boost::functional::detail::binary_function<T&, A, S>
~~~~~~~~~~~~~~~~~~~~~~~~~~~^
include/boost/functional.hpp:46:24: note: '__binary_function' declared here
using std::binary_function;
tnias
force-pushed
the
update/vectorscan-5.4.11
branch
from
111f1da
to
dbaa125
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of changes
Release: https://github.com/VectorCamp/vectorscan/releases/tag/vectorscan%2F5.4.11
Changelog: https://github.com/VectorCamp/vectorscan/blob/develop/CHANGELOG-vectorscan.md#5411-2023-11-19
tldr: This PR bumps the version and explicitly enables all available CPU extensions the
FAT_RUNTIMEbinaries.With the version bump the
FAT_RUNTIMEwon't build without AVX2 support explicitly enabled.And upsteam now enabled all extensions for fat binaries (not released yet).
And Debian has almost all extensions enabled. (I assume they did not enable BUILD_AVX512VBMI, because they want it to be buildable by an older toolchain?)
This let me to belive it would be sensible enable all
FAT_RUNTIMEextensions.FAT_RUNTIMEbundles optimized implementations for different CPU extensions and uses CPUID to transparently select the fastest for the current hardware.This feature is only available on linux for x86, x86_64, and aarch64.
If
FAT_RUNTIMEis not available, we fall back to building for a single extension based on stdenv.hostPlatform.For generic builds (e.g. x86_64) this can mean using an implementation not optimized for the potentially available more modern hardware extensions (e.g. x86_64 with AVX512).
Effectively this adds avx512 and avx512vbmi support for linux x86_64 binaries and follows upstreams direction.
"nm -D ..." showing what symbols are available from what library builds
linux x86_64:linux aarch64:
Result of
nixpkgs-review pr 303207run on x86_64-linux 15 packages built:
Result of
nixpkgs-review pr 303207run on aarch64-linux 15 packages built:
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.