From 4524235af46b7c57008101afdbe00fd2d3cbfbbd Mon Sep 17 00:00:00 2001
From: Sergei Zimmerman <sergei@zimmerman.foo>
Date: Tue, 9 Sep 2025 22:18:52 +0300
Subject: [PATCH] libexpr: Overalign Value to 16 bytes

This is necessary to make use of 128 bit atomics on x86_64 [1],
since MOVAPD, MOVAPS, and MOVDQA need memory operands to be 16-byte
aligned. We are not losing anything here, because Value is already 16-byte
wide and Boehm allocates memory in granules that are 16 bytes by default
on 64 bit systems [2].

[1]: https://patchwork.sourceware.org/project/gcc/patch/YhxkfzGEEQ9KHbBC@tucnak/
[2]: https://github.com/bdwgc/bdwgc/blob/54ac18ccbc5a833dd7edaff94a10ab9b65044d61/include/gc/gc_tiny_fl.h#L31-L33
---
 src/libexpr/eval-gc.cc                | 12 ++++++++++++
 src/libexpr/include/nix/expr/value.hh |  5 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/libexpr/eval-gc.cc b/src/libexpr/eval-gc.cc
index b17336a90..28aed7c37 100644
--- a/src/libexpr/eval-gc.cc
+++ b/src/libexpr/eval-gc.cc
@@ -16,6 +16,7 @@
 #  endif
 
 #  include <gc/gc_allocator.h>
+#  include <gc/gc_tiny_fl.h> // For GC_GRANULE_BYTES
 
 #  include <boost/coroutine2/coroutine.hpp>
 #  include <boost/coroutine2/protected_fixedsize_stack.hpp>
@@ -23,6 +24,17 @@
 
 #endif
 
+/*
+ * Ensure that Boehm satisfies our alignment requirements. This is the default configuration [^]
+ * and this assertion should never break for any platform. Let's assert it just in case.
+ *
+ * This alignment is particularly useful to be able to use aligned
+ * load/store instructions for loading/writing Values.
+ *
+ * [^]: https://github.com/bdwgc/bdwgc/blob/54ac18ccbc5a833dd7edaff94a10ab9b65044d61/include/gc/gc_tiny_fl.h#L31-L33
+ */
+static_assert(sizeof(void *) * 2 == GC_GRANULE_BYTES, "Boehm GC must use GC_GRANULE_WORDS = 2");
+
 namespace nix {
 
 #if NIX_USE_BOEHMGC
diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh
index 55ab797c7..228b23a7a 100644
--- a/src/libexpr/include/nix/expr/value.hh
+++ b/src/libexpr/include/nix/expr/value.hh
@@ -369,7 +369,7 @@ namespace detail {
 /* Whether to use a specialization of ValueStorage that does bitpacking into
    alignment niches. */
 template<std::size_t ptrSize>
-inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 8);
+inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 16);
 
 } // namespace detail
 
@@ -378,7 +378,8 @@ inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEF
  * Packs discriminator bits into the pointer alignment niches.
  */
 template<std::size_t ptrSize>
-class ValueStorage<ptrSize, std::enable_if_t<detail::useBitPackedValueStorage<ptrSize>>> : public detail::ValueBase
+class alignas(16) ValueStorage<ptrSize, std::enable_if_t<detail::useBitPackedValueStorage<ptrSize>>>
+    : public detail::ValueBase
 {
     /* Needs a dependent type name in order for member functions (and
      * potentially ill-formed bit casts) to be SFINAE'd out.