Skip to content

Conversation

trofi
Copy link
Contributor

@trofi trofi commented Jun 17, 2024

Changes:

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot requested a review from zhaofengli June 17, 2024 21:57
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Jun 17, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4305

@SuperSandro2000
Copy link
Member

@ofborg build libhwy

@SuperSandro2000
Copy link
Member

aarch64-linux fails to build

FAILED: CMakeFiles/math_test.dir/hwy/contrib/math/math_test.cc.o 
/nix/store/s6g4y3da8f87wwshs3l86wfcycmjgdqi-gcc-wrapper-13.3.0/bin/g++ -DHWY_STATIC_DEFINE -I/build/source -isystem /nix/store/s0ncp3pvk2vzsiv5pi8hb0p35dm1d8ch-gtest-1.14.0-dev/include -O3 -DNDEBUG -std=c++17 -fPIE -fvisibility=hidden -fvisibility-inlines-hidden -Wno-builtin-macro-redefined -D__DATE__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -D__TIME__=\"redacted\" -fmerge-all-constants -Wall -Wextra -Wconversion -Wsign-conversion -Wvla -Wnon-virtual-dtor -Wcast-align -fmath-errno -fno-exceptions -Wno-psabi -DHWY_IS_TEST=1 -MD -MT CMakeFiles/math_test.dir/hwy/contrib/math/math_test.cc.o -MF CMakeFiles/math_test.dir/hwy/contrib/math/math_test.cc.o.d -o CMakeFiles/math_test.dir/hwy/contrib/math/math_test.cc.o -c /build/source/hwy/contrib/math/math_test.cc
In file included from /build/source/hwy/tests/test_util-inl.h:28,
                 from /build/source/hwy/contrib/math/math_test.cc:31,
                 from /build/source/hwy/foreach_target.h:163,
                 from /build/source/hwy/contrib/math/math_test.cc:28:
/build/source/hwy/tests/test_util.h: In function 'std::string hwy::TypeName(T, size_t) [with T = double]':
/build/source/hwy/tests/test_util.h:152:19: error: this operation requires the SVE ISA extension
  152 |   detail::TypeName(detail::MakeTypeInfo<T>(), N, string100);
      |   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/source/hwy/tests/test_util.h:152:19: note: you can enable SVE using the command-line option '-march', or by using the 'target' attribute or pragma

Copy link
Member

@NickCao NickCao left a comment

Choose a reason for hiding this comment

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

The riscv patch need an update:

diff --git a/pkgs/development/libraries/libhwy/default.nix b/pkgs/development/libraries/libhwy/default.nix
index c82d3738efe9..c5a777be8691 100644
--- a/pkgs/development/libraries/libhwy/default.nix
+++ b/pkgs/development/libraries/libhwy/default.nix
@@ -18,14 +18,13 @@ stdenv.mkDerivation rec {
     hash = "sha256-yJQH5ZkpEdJ6lsTAt6yJSN3TQnVoxNpkbChENaxhcHo=";
   };

-  patches = lib.optional stdenv.hostPlatform.isRiscV
-    # Adds CMake option HWY_CMAKE_RVV
-    # https://github.com/google/highway/pull/1743
+  patches = [
     (fetchpatch {
-      name = "libhwy-add-rvv-optout.patch";
-      url = "https://github.com/google/highway/commit/5d58d233fbcec0c6a39df8186a877329147324b3.patch";
-      hash = "sha256-ileSNYddOt1F5rooRB0fXT20WkVlnG+gP5w7qJdBuww=";
-    });
+      name = "disable-RVV-runtime-dispatch.patch";
+      url = "https://github.com/google/highway/commit/c95cc0237d2f7a0f5ca5dc3fb4b5961b2b1dcdfc.patch";
+      hash = "sha256-oQfyZrjZ9MGcSrFInbbj+0iOLjPng7tgTzli1QTITSg=";
+    })
+  ];

   hardeningDisable = lib.optionals stdenv.hostPlatform.isAarch64 [
     # aarch64-specific code gets:

@trofi
Copy link
Contributor Author

trofi commented Jul 27, 2024

The riscv patch need an update:

Applied. Thank you!

@SuperSandro2000
Copy link
Member

aarch64-linux fails to build

that still doesn't work...

@trofi trofi marked this pull request as draft July 28, 2024 21:00
@trofi
Copy link
Contributor Author

trofi commented Jul 28, 2024

Yeah, I did not get to look at it. Let's convert to draft.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 9, 2024
@philiptaron
Copy link
Contributor

7ea1332 landed in the interim. Maybe aarch64 works now?

@trofi trofi marked this pull request as ready for review December 23, 2024 22:41
@trofi
Copy link
Contributor Author

trofi commented Dec 23, 2024

Sounds good. Let's try again.

@philiptaron philiptaron removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 23, 2024
@github-actions github-actions bot removed the 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. label Dec 23, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. and removed 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. labels Dec 24, 2024
@trofi trofi marked this pull request as draft December 30, 2024 21:42
@trofi
Copy link
Contributor Author

trofi commented Dec 30, 2024

Ofborg says it still fails. Looks like a -march= / target attribute mismatch:

In file included from /build/source/hwy/tests/test_util-inl.h:28,
                 from /build/source/hwy/contrib/math/math_test.cc:31,
                 from /build/source/hwy/foreach_target.h:163,
                 from /build/source/hwy/contrib/math/math_test.cc:28:
/build/source/hwy/tests/test_util.h: In function 'std::string hwy::TypeName(T, size_t) [with T = float]':
/build/source/hwy/tests/test_util.h:152:19: error: this operation requires the SVE ISA extension
  152 |   detail::TypeName(detail::MakeTypeInfo<T>(), N, string100);
      |   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/source/hwy/tests/test_util.h:152:19: note: you can enable SVE using the command-line option '-march', or by using the 'target' attribute or pragma

@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 29, 2025
@MisileLab
Copy link
Contributor

I know it is stale, but new version released: https://github.com/google/highway/releases/tag/1.3.0

@nixpkgs-ci nixpkgs-ci bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 15, 2025
@trofi trofi changed the title libhwy: 1.0.7 -> 1.2.0 libhwy: 1.0.7 -> 1.3.0 Aug 16, 2025
@trofi
Copy link
Contributor Author

trofi commented Aug 16, 2025

I know it is stale, but new version released: https://github.com/google/highway/releases/tag/1.3.0

Good catch! Updated to 1.3.0. Did not test on aarch64-linux as I don't have it.

UPDATE: ofborg still fails:

/build/source/hwy/tests/test_util.h:182:19: error: this operation requires the SVE ISA extension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants