From 766c96233aca1a6307f83e99cc5ef7650e6d3a90 Mon Sep 17 00:00:00 2001 From: Robert Shi Date: Sun, 20 Sep 2026 12:09:35 -0700 Subject: [PATCH] Replace GM_CACHE_LINE_PAD with C11 alignas --- docs/style_guide.md | 9 +--- src/core/analysis/analysis.h | 5 +- src/core/gamesman_memory.h | 13 ----- src/core/solvers/tier_solver/tier_analyzer.c | 4 +- .../tier_worker/backward_induction/frontier.h | 6 +-- tests/unit/core/gamesman_memory_test.cpp | 50 ------------------- 6 files changed, 8 insertions(+), 79 deletions(-) diff --git a/docs/style_guide.md b/docs/style_guide.md index 45fcb36e..3931456d 100644 --- a/docs/style_guide.md +++ b/docs/style_guide.md @@ -184,7 +184,6 @@ typedef enum Value { * All macros (whether representing constant expressions, sentinels, or function-like macros) must use `SCREAMING_SNAKE_CASE`: ```c #define INT64_HASH_SET_EMPTY_KEY INT64_MIN -#define GM_CACHE_LINE_PAD(n) ((((n) + (GM_CACHE_LINE_SIZE) - 1) / (GM_CACHE_LINE_SIZE) * (GM_CACHE_LINE_SIZE)) - (n)) #define DECLARE_INT64_STATIC_HASH_SET(name, cap) ... ``` @@ -282,13 +281,7 @@ static_assert((GM_CACHE_LINE_SIZE & (GM_CACHE_LINE_SIZE - 1)) == 0, ``` ### 5.4 Macro Safety -* When defining macros with parameters, enclose all parameter usages and the entire macro expression in parentheses to avoid operator precedence issues: -```c -#define GM_CACHE_LINE_PAD(n) \ - ((((n) + (GM_CACHE_LINE_SIZE) - 1) / (GM_CACHE_LINE_SIZE) * \ - (GM_CACHE_LINE_SIZE)) - \ - (n)) -``` +* When defining macros with parameters, enclose all parameter usages and the entire macro expression in parentheses to avoid operator precedence issues。 * Prefer `static inline` functions over function-like macros whenever feasible. ### 5.5 Array Bounds and Restrict Qualifiers diff --git a/src/core/analysis/analysis.h b/src/core/analysis/analysis.h index eba7ea41..31fe0832 100644 --- a/src/core/analysis/analysis.h +++ b/src/core/analysis/analysis.h @@ -27,6 +27,7 @@ #ifndef GAMESMANONE_CORE_ANALYSIS_ANALYSIS_H_ #define GAMESMANONE_CORE_ANALYSIS_ANALYSIS_H_ +#include #include #include #include @@ -112,9 +113,7 @@ typedef struct Analysis { * in an array. */ typedef struct CacheAlignedAnalysis { - Analysis data; /**< Unpadded data. */ - /** Padding CacheAlignedAnalysis to GM_CACHE_LINE_SIZE bytes. */ - char padding[GM_CACHE_LINE_PAD(sizeof(Analysis))]; + alignas(GM_CACHE_LINE_SIZE) Analysis data; /**< Cache-line-aligned data. */ } CacheAlignedAnalysis; /** diff --git a/src/core/gamesman_memory.h b/src/core/gamesman_memory.h index 17d0bc69..67f489d2 100644 --- a/src/core/gamesman_memory.h +++ b/src/core/gamesman_memory.h @@ -41,19 +41,6 @@ static_assert( static_assert((GM_CACHE_LINE_SIZE & (GM_CACHE_LINE_SIZE - 1)) == 0, "GM_CACHE_LINE_SIZE is not defined as a power of 2"); -/** - * @brief Returns the number of bytes to be padded to an object of size `n` so - * that its size becomes a multiple of `GM_CACHE_LINE_SIZE`. - * - * @param[in] n Size of the object in bytes. - * - * @return The number of bytes to be padded. - */ -#define GM_CACHE_LINE_PAD(n) \ - ((((n) + (GM_CACHE_LINE_SIZE) - 1) / (GM_CACHE_LINE_SIZE) * \ - (GM_CACHE_LINE_SIZE)) - \ - (n)) - // ================================= Allocator ================================= /** diff --git a/src/core/solvers/tier_solver/tier_analyzer.c b/src/core/solvers/tier_solver/tier_analyzer.c index 269be200..0f69b23e 100644 --- a/src/core/solvers/tier_solver/tier_analyzer.c +++ b/src/core/solvers/tier_solver/tier_analyzer.c @@ -27,6 +27,7 @@ #include "core/solvers/tier_solver/tier_analyzer.h" #include +#include #include #include #include @@ -84,8 +85,7 @@ static ConcurrentBitset **child_tier_maps; static int num_threads; // Number of threads available. typedef struct { - PositionArray a; - char padding[GM_CACHE_LINE_PAD(sizeof(PositionArray))]; + alignas(GM_CACHE_LINE_SIZE) PositionArray a; } PaddedPositionArray; static PaddedPositionArray *fringe; // Discovered but unprocessed positions. static PaddedPositionArray *discovered; // Newly discovered positions. diff --git a/src/core/solvers/tier_solver/tier_worker/backward_induction/frontier.h b/src/core/solvers/tier_solver/tier_worker/backward_induction/frontier.h index 933d6c85..e115618f 100644 --- a/src/core/solvers/tier_solver/tier_worker/backward_induction/frontier.h +++ b/src/core/solvers/tier_solver/tier_worker/backward_induction/frontier.h @@ -33,6 +33,7 @@ #ifndef GAMESMANONE_CORE_SOLVERS_TIER_SOLVER_TIER_WORKER_BACKWARD_INDUCTION_FRONTIER_H_ #define GAMESMANONE_CORE_SOLVERS_TIER_SOLVER_TIER_WORKER_BACKWARD_INDUCTION_FRONTIER_H_ +#include #include #include @@ -96,9 +97,8 @@ typedef struct FrontierInternal { * with remoteness i. */ typedef struct Frontier { - FrontierInternal f; /**< Unpadded frontier object. */ - /** Padding Frontier to GM_CACHE_LINE_SIZE bytes. */ - char padding[GM_CACHE_LINE_PAD(sizeof(FrontierInternal))]; + /** Cache-line-aligned frontier. */ + alignas(GM_CACHE_LINE_SIZE) FrontierInternal f; } Frontier; /** diff --git a/tests/unit/core/gamesman_memory_test.cpp b/tests/unit/core/gamesman_memory_test.cpp index d97d14a2..716c31aa 100644 --- a/tests/unit/core/gamesman_memory_test.cpp +++ b/tests/unit/core/gamesman_memory_test.cpp @@ -36,56 +36,6 @@ extern "C" const char *__asan_default_options() { } #endif -// =================================== Macro =================================== - -// Verifies that a size of exactly zero correctly evaluates to needing zero -// bytes of padding. -TEST(CacheLinePadTest, ZeroSize) { EXPECT_EQ(GM_CACHE_LINE_PAD(0), 0); } - -// Verifies that sizes that are exact multiples of the cache line size require -// no additional padding. -TEST(CacheLinePadTest, ExactMultiple) { - EXPECT_EQ(GM_CACHE_LINE_PAD(GM_CACHE_LINE_SIZE), 0); - EXPECT_EQ(GM_CACHE_LINE_PAD(GM_CACHE_LINE_SIZE * 2), 0); - EXPECT_EQ(GM_CACHE_LINE_PAD(GM_CACHE_LINE_SIZE * 10), 0); -} - -// Verifies that a single byte allocation forces almost an entire cache line of -// padding to reach the next boundary. -TEST(CacheLinePadTest, SmallValue) { - EXPECT_EQ(GM_CACHE_LINE_PAD(1), GM_CACHE_LINE_SIZE - 1); -} - -// Verifies the upper boundary condition where the size falls exactly one byte -// short of a full cache line multiple. -TEST(CacheLinePadTest, OneByteUnder) { - EXPECT_EQ(GM_CACHE_LINE_PAD(GM_CACHE_LINE_SIZE - 1), 1); - EXPECT_EQ(GM_CACHE_LINE_PAD((GM_CACHE_LINE_SIZE * 3) - 1), 1); -} - -// Verifies the lower boundary condition where the size spills exactly one byte -// over a full cache line multiple. -TEST(CacheLinePadTest, OneByteOver) { - EXPECT_EQ(GM_CACHE_LINE_PAD(GM_CACHE_LINE_SIZE + 1), - GM_CACHE_LINE_SIZE - 1); - EXPECT_EQ(GM_CACHE_LINE_PAD((GM_CACHE_LINE_SIZE * 4) + 1), - GM_CACHE_LINE_SIZE - 1); -} - -// Verifies that the macro safely handles larger memory block padding math -// without truncating or evaluating poorly. -TEST(CacheLinePadTest, LargeMemoryBlock) { - // 1MB memory block plus half a cache line offset to verify boundary - // arithmetic at higher scales - const size_t one_mb = 1 << 20; - ASSERT_TRUE(one_mb % GM_CACHE_LINE_SIZE == 0) - << "GM_CACHE_LINE_SIZE == " << GM_CACHE_LINE_SIZE - << " does not divide 1 MiB. This is very likely the result of a bug " - "in the cache line size detection program."; - const size_t large_size = one_mb + (GM_CACHE_LINE_SIZE / 2); - EXPECT_EQ(GM_CACHE_LINE_PAD(large_size), GM_CACHE_LINE_SIZE / 2); -} - // ================================= Allocator ================================= // Verifies that populating default options behaves as specified.