Ping On 5/27/26 17:14, Matthew Malcomson wrote:
The function `gomp_barrier_state_is_incremented` is there in order to handle edge cases for when a generation gets incremented. It handles flags that might be on the generation number by checking `>` instead of `==` and it handles wraparound on the generation number.Unfortunately it had a problem where it did not handle the combination of flags on the state provided and wraparound on the generation number. I.e. if there were flags on the `state` input and the next generation would be *smaller* than the current generation due to unsigned integral overflow and hence wraparound then it could report that the generation has been incremented when it has not. This patch fixes that problem. I found a testcase to take too long for it to be sensible to have in the testsuite. My impression is that it's so long that it doesn't even make sense to run it when the `run_expensive_tests` effective target is set but would appreciate confirmation or refutation of that. This is because it takes a long time to go through a barrier enough times to trigger wraparound. My manual testing on a shared machine showed ~4 min when on the linux/ target and drastically longer built without futex support to test the posix/ target. Testing done as part of my patch-series: - Bootstrap & regtest on aarch64 and x86_64. - Testsuite with & without OMP_WAIT_POLICY=passive - With and without configure `--enable-linux-futex=no` for posix target. - Cross compilation & regtest on arm. - TSAN done on before and after total patch-series. libgomp/ChangeLog: * config/gcn/bar.h (gomp_barrier_state_is_incremented): Mask flags on previously observed state before comparison against current state. * config/linux/bar.h (gomp_barrier_state_is_incremented): Likewise * config/nvptx/bar.h (gomp_barrier_state_is_incremented): Likewise * config/posix/bar.h (gomp_barrier_state_is_incremented): Likewise * config/rtems/bar.h (gomp_barrier_state_is_incremented): Likewise --- libgomp/config/gcn/bar.h | 3 ++- libgomp/config/linux/bar.h | 3 ++- libgomp/config/nvptx/bar.h | 3 ++- libgomp/config/posix/bar.h | 3 ++- libgomp/config/rtems/bar.h | 3 ++- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/libgomp/config/gcn/bar.h b/libgomp/config/gcn/bar.h index 6e838ff54a8..f7d5a6f8756 100644 --- a/libgomp/config/gcn/bar.h +++ b/libgomp/config/gcn/bar.h @@ -172,7 +172,8 @@ static inline bool gomp_barrier_state_is_incremented (gomp_barrier_state_t gen, gomp_barrier_state_t state) { - unsigned next_state = (state & -BAR_INCR) + BAR_INCR; + state &= -BAR_INCR; + unsigned next_state = state + BAR_INCR; return next_state > state ? gen >= next_state : gen < state; }diff --git a/libgomp/config/linux/bar.h b/libgomp/config/linux/bar.hindex 623c90f5bfc..8ae6edf9fb7 100644 --- a/libgomp/config/linux/bar.h +++ b/libgomp/config/linux/bar.h @@ -172,7 +172,8 @@ static inline bool gomp_barrier_state_is_incremented (gomp_barrier_state_t gen, gomp_barrier_state_t state) { - unsigned next_state = (state & -BAR_INCR) + BAR_INCR; + state &= -BAR_INCR; + unsigned next_state = state + BAR_INCR; return next_state > state ? gen >= next_state : gen < state; }diff --git a/libgomp/config/nvptx/bar.h b/libgomp/config/nvptx/bar.hindex aa2592ba5b3..84796c1ffa7 100644 --- a/libgomp/config/nvptx/bar.h +++ b/libgomp/config/nvptx/bar.h @@ -173,7 +173,8 @@ static inline bool gomp_barrier_state_is_incremented (gomp_barrier_state_t gen, gomp_barrier_state_t state) { - unsigned next_state = (state & -BAR_INCR) + BAR_INCR; + state &= -BAR_INCR; + unsigned next_state = state + BAR_INCR; return next_state > state ? gen >= next_state : gen < state; }diff --git a/libgomp/config/posix/bar.h b/libgomp/config/posix/bar.hindex 026daca793d..b24c0d34433 100644 --- a/libgomp/config/posix/bar.h +++ b/libgomp/config/posix/bar.h @@ -162,7 +162,8 @@ static inline bool gomp_barrier_state_is_incremented (gomp_barrier_state_t gen, gomp_barrier_state_t state) { - unsigned next_state = (state & -BAR_INCR) + BAR_INCR; + state &= -BAR_INCR; + unsigned next_state = state + BAR_INCR; return next_state > state ? gen >= next_state : gen < state; }diff --git a/libgomp/config/rtems/bar.h b/libgomp/config/rtems/bar.hindex 80fb1cd3be8..fa38fccd2c8 100644 --- a/libgomp/config/rtems/bar.h +++ b/libgomp/config/rtems/bar.h @@ -174,7 +174,8 @@ static inline bool gomp_barrier_state_is_incremented (gomp_barrier_state_t gen, gomp_barrier_state_t state) { - unsigned next_state = (state & -BAR_INCR) + BAR_INCR; + state &= -BAR_INCR; + unsigned next_state = state + BAR_INCR; return next_state > state ? gen >= next_state : gen < state; }
