On Thu, Jul 23, 2026 at 04:36:06PM +0100, [email protected] wrote:
> From: Matthew Malcomson <[email protected]>
>
> N.b. sending out before testing is finished to check that it's in the
> right direction.
>
> As mentioned earlier I like the helper, but after I inlined each of the
> wait_end variants having the helper seemed silly. I plan to add it back
> in the more complex patches where I found it useful.
>
> ------------- 8< ---------------------------- >8 -------------------
>
> 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.
>
> While discussing that problem it was pointed out that the helper
> performs unnecessary checks because the barrier can only by incremented
> by one when this is used. In order to avoid this unnecessary work we
> explicitly check against one increment ahead (which naturally handles
> the overflow case). Moreover in the busy-wait loops of `*wait_*end` in
> bar.c we explicitly inline the helper to take advantage of our knowledge
> that `state` has no flags set on it.
>
> 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:
> - 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.
> - nvptx, gcn & rtems targets built.
> - Cross compilation & regtest on arm.
>
> libgomp/ChangeLog:
>
> * config/gcn/bar.c (gomp_team_barrier_wait_end): Inline
> increment check.
> (gomp_team_barrier_wait_cancel_end): Inline increment check.
> * config/gcn/bar.h (gomp_barrier_state_is_incremented): Remove
> function.
> (gomp_barrier_has_completed): Perform via equality check.
> * config/linux/bar.c (gomp_team_barrier_wait_end): Inline
> increment check.
> (gomp_team_barrier_wait_cancel_end): Inline increment check.
> * config/linux/bar.h (gomp_barrier_state_is_incremented): Remove
> function.
> (gomp_barrier_has_completed): Perform via equality check.
> * config/nvptx/bar.h (gomp_barrier_state_is_incremented): Remove
> function.
> (gomp_barrier_has_completed): Perform via equality check.
> * config/posix/bar.c (gomp_team_barrier_wait_end): Inline
> increment check.
> (gomp_team_barrier_wait_cancel_end): Inline increment check.
> * config/posix/bar.h (gomp_barrier_state_is_incremented): Remove
> function.
> (gomp_barrier_has_completed): Perform via equality check.
> * config/rtems/bar.h (gomp_barrier_state_is_incremented): Remove
> function.
> (gomp_barrier_has_completed): Perform via equality check.
>
> Signed-off-by: Matthew Malcomson <[email protected]>
LGTM if testing passes.
Jakub
