Skip to content

Commit 4524235

Browse files
committed
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
1 parent 371623b commit 4524235

File tree

2 files changed

+15
-2
lines changed

2 files changed

+15
-2
lines changed

src/libexpr/eval-gc.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,25 @@
1616
# endif
1717

1818
# include <gc/gc_allocator.h>
19+
# include <gc/gc_tiny_fl.h> // For GC_GRANULE_BYTES
1920

2021
# include <boost/coroutine2/coroutine.hpp>
2122
# include <boost/coroutine2/protected_fixedsize_stack.hpp>
2223
# include <boost/context/stack_context.hpp>
2324

2425
#endif
2526

27+
/*
28+
* Ensure that Boehm satisfies our alignment requirements. This is the default configuration [^]
29+
* and this assertion should never break for any platform. Let's assert it just in case.
30+
*
31+
* This alignment is particularly useful to be able to use aligned
32+
* load/store instructions for loading/writing Values.
33+
*
34+
* [^]: https://github.com/bdwgc/bdwgc/blob/54ac18ccbc5a833dd7edaff94a10ab9b65044d61/include/gc/gc_tiny_fl.h#L31-L33
35+
*/
36+
static_assert(sizeof(void *) * 2 == GC_GRANULE_BYTES, "Boehm GC must use GC_GRANULE_WORDS = 2");
37+
2638
namespace nix {
2739

2840
#if NIX_USE_BOEHMGC

src/libexpr/include/nix/expr/value.hh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ namespace detail {
369369
/* Whether to use a specialization of ValueStorage that does bitpacking into
370370
alignment niches. */
371371
template<std::size_t ptrSize>
372-
inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 8);
372+
inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 16);
373373

374374
} // namespace detail
375375

@@ -378,7 +378,8 @@ inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEF
378378
* Packs discriminator bits into the pointer alignment niches.
379379
*/
380380
template<std::size_t ptrSize>
381-
class ValueStorage<ptrSize, std::enable_if_t<detail::useBitPackedValueStorage<ptrSize>>> : public detail::ValueBase
381+
class alignas(16) ValueStorage<ptrSize, std::enable_if_t<detail::useBitPackedValueStorage<ptrSize>>>
382+
: public detail::ValueBase
382383
{
383384
/* Needs a dependent type name in order for member functions (and
384385
* potentially ill-formed bit casts) to be SFINAE'd out.

0 commit comments

Comments
 (0)