From d45e1595ec1cffeab83ac6602b748250b66bea03 Mon Sep 17 00:00:00 2001 From: Sebastiano Tronto Date: Mon, 24 Mar 2025 23:09:26 +0100 Subject: Big cleanup for appendsolution() With this PR the appendsolution routine is extracted from the h48 solver and the new coordinate solver and made generic. This has many advantages: - less repetition (even if the two versions are different enough that *for now* it was not a big deal) - smaller h48/solve.h file, which is already a big beast - easier to test the appendsolution() routine separately --- src/core/io_moves.h | 41 +++++++++++++++++++++++++++++--- src/core/moves.h | 68 ++++++++++++++--------------------------------------- 2 files changed, 55 insertions(+), 54 deletions(-) (limited to 'src/core') diff --git a/src/core/io_moves.h b/src/core/io_moves.h index fa91e8b..8c56a9f 100644 --- a/src/core/io_moves.h +++ b/src/core/io_moves.h @@ -1,6 +1,26 @@ STATIC uint8_t readmove(char); +STATIC int64_t readmoves(const char *, size_t n, uint8_t [n]); STATIC uint8_t readmodifier(char); -STATIC int64_t writemoves(size_t n, uint8_t [n], size_t m, char [m]); +STATIC int64_t writemoves(size_t n, const uint8_t [n], size_t m, char [m]); + +#define FOREACH_READMOVE(ARG_BUF, ARG_MOVE, ARG_C, ARG_MAX, \ + RET_ERROR, ARG_ACTION) \ + const char *VAR_B; \ + uint8_t VAR_MOVE_NOMOD, VAR_MOD; \ + for (VAR_B = ARG_BUF, ARG_C = 0; *VAR_B != '\0'; VAR_B++, ARG_C++) { \ + while (*VAR_B == ' ' || *VAR_B == '\t' || *VAR_B == '\n') \ + VAR_B++; \ + if (*VAR_B == '\0' || ARG_C == ARG_MAX) \ + break; \ + if ((VAR_MOVE_NOMOD = readmove(*VAR_B)) == UINT8_ERROR) { \ + LOG("Error: unknown move '%c'\n", *VAR_B); \ + return RET_ERROR; \ + } \ + if ((VAR_MOD = readmodifier(*(VAR_B+1))) != 0) \ + VAR_B++; \ + ARG_MOVE = VAR_MOVE_NOMOD + VAR_MOD; \ + ARG_ACTION \ + } STATIC uint8_t readmove(char c) @@ -38,10 +58,23 @@ readmodifier(char c) } } +STATIC int64_t +readmoves(const char *buf, size_t n, uint8_t ret[n]) +{ + uint8_t m; + uint64_t c; + + FOREACH_READMOVE(buf, m, c, n, NISSY_ERROR_INVALID_MOVES, + ret[c] = m; + ) + + return (int64_t)c; +} + STATIC int64_t writemoves( size_t nmoves, - uint8_t m[nmoves], + const uint8_t m[nmoves], size_t buf_size, char buf[buf_size] ) @@ -69,7 +102,9 @@ writemoves( *b = ' '; } - if (b != buf) + if (b == buf) + written = 1; /* Nothing written, only NULL-terminator */ + else b--; /* Remove last space */ *b = '\0'; diff --git a/src/core/moves.h b/src/core/moves.h index 820406b..e56d5c8 100644 --- a/src/core/moves.h +++ b/src/core/moves.h @@ -3,6 +3,7 @@ STATIC_INLINE bool allowednextmove(size_t n, const uint8_t [n]); STATIC_INLINE uint32_t allowednextmove_mask(size_t n, const uint8_t [n]); +STATIC bool allowedmoves(size_t n, const uint8_t [n]); STATIC_INLINE uint8_t movebase(uint8_t); STATIC_INLINE uint8_t moveaxis(uint8_t); @@ -13,11 +14,9 @@ STATIC_INLINE uint32_t disable_moves(uint32_t, uint8_t); STATIC cube_t move(cube_t, uint8_t); STATIC cube_t premove(cube_t, uint8_t); STATIC uint8_t inverse_move(uint8_t); -STATIC void invertmoves(size_t n, const uint8_t [n], uint8_t [n]); -STATIC void sortparallel(size_t n, uint8_t [n]); +STATIC void sortparallel_moves(size_t n, uint8_t [n]); STATIC bool are_lastmoves_singlecw(size_t n, uint8_t [n]); -STATIC int readmoves(const char *, int, uint8_t *); STATIC cube_t applymoves(cube_t, const char *); #define FOREACH_READMOVE(ARG_BUF, ARG_MOVE, ARG_C, ARG_MAX, \ @@ -75,6 +74,18 @@ allowednextmove_mask(size_t n, const uint8_t moves[n]) return result; } +STATIC bool +allowedmoves(size_t n, const uint8_t moves[n]) +{ + uint8_t j; + + for (j = 2; j < n; j++) + if (!allowednextmove(j, moves)) + return false; + + return true; +} + STATIC_INLINE uint32_t disable_moves(uint32_t current_result, uint8_t base_index) { @@ -210,44 +221,13 @@ inverse_move(uint8_t m) return m - 2 * (m % 3) + 2; } -/* -GCC has issues when -Wstringop-overflow is used together with O3. It produces -warnings like the following: - -In function 'invertmoves', - inlined from 'solve_h48_appendsolution' at src/solvers/h48/solve.h:81:3, - inlined from 'solve_h48_dfs.isra' at src/solvers/h48/solve.h:139:3: -warning: writing 32 bytes into a region of size 0 [-Wstringop-overflow=] - 197 | ret[i] = inverse_move(moves[nmoves - i - 1]); - | ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -In function 'solve_h48_dfs.isra': -note: at offset 192 into destination object 'invertedpremoves' of size 20 - 71 | uint8_t invertedpremoves[MAXLEN]; - -Clang does not give any warning. -Someone else complained here: https://access.redhat.com/solutions/6755371 - -To solve this issue temporarily, we use a lower optimization setting for -this function only. - -TODO check if the issue is resolved -*/ -#pragma GCC push_options -#pragma GCC optimize ("O2") STATIC void -invertmoves(size_t n, const uint8_t moves[n], uint8_t ret[n]) +sortparallel_moves(size_t n, uint8_t moves[n]) { uint8_t i; - for (i = 0; i < n; i++) - ret[i] = inverse_move(moves[n - i - 1]); -} -#pragma GCC pop_options - -STATIC void -sortparallel(size_t n, uint8_t moves[n]) -{ - uint8_t i; + if (n < 2) + return; for (i = 0; i < n-1; i++) if (moveaxis(moves[i]) == moveaxis(moves[i+1]) && @@ -268,20 +248,6 @@ are_lastmoves_singlecw(size_t n, uint8_t moves[n]) return isbase(moves[n-1]) && (!two || isbase(moves[n-2])); } -STATIC int -readmoves(const char *buf, int max, uint8_t *ret) -{ - uint8_t m; - int c; - - FOREACH_READMOVE(buf, m, c, max, NISSY_ERROR_INVALID_MOVES, - if (ret != NULL) - ret[c] = m; - ) - - return c; -} - STATIC cube_t applymoves(cube_t cube, const char *buf) { -- cgit v1.3