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]>
---
libgomp/config/gcn/bar.c | 4 ++--
libgomp/config/gcn/bar.h | 13 ++-----------
libgomp/config/linux/bar.c | 4 ++--
libgomp/config/linux/bar.h | 13 ++-----------
libgomp/config/nvptx/bar.h | 13 ++-----------
libgomp/config/posix/bar.c | 4 ++--
libgomp/config/posix/bar.h | 13 ++-----------
libgomp/config/rtems/bar.h | 13 ++-----------
8 files changed, 16 insertions(+), 61 deletions(-)
diff --git a/libgomp/config/gcn/bar.c b/libgomp/config/gcn/bar.c
index 3045587f0f3..b4ea8e68a92 100644
--- a/libgomp/config/gcn/bar.c
+++ b/libgomp/config/gcn/bar.c
@@ -130,7 +130,7 @@ gomp_team_barrier_wait_end (gomp_barrier_t *bar,
gomp_barrier_state_t state)
gen = __atomic_load_n (&bar->generation, MEMMODEL_ACQUIRE);
}
}
- while (!gomp_barrier_state_is_incremented (gen, state));
+ while ((gen & -BAR_INCR) != state + BAR_INCR);
}
void
@@ -211,7 +211,7 @@ gomp_team_barrier_wait_cancel_end (gomp_barrier_t *bar,
gen = __atomic_load_n (&bar->generation, MEMMODEL_RELAXED);
}
}
- while (!gomp_barrier_state_is_incremented (gen, state));
+ while ((gen & -BAR_INCR) != state + BAR_INCR);
return false;
}
diff --git a/libgomp/config/gcn/bar.h b/libgomp/config/gcn/bar.h
index 6e838ff54a8..943bafc3551 100644
--- a/libgomp/config/gcn/bar.h
+++ b/libgomp/config/gcn/bar.h
@@ -168,20 +168,11 @@ gomp_team_barrier_done (gomp_barrier_t *bar,
gomp_barrier_state_t state)
MEMMODEL_RELEASE);
}
-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;
- return next_state > state ? gen >= next_state : gen < state;
-}
-
static inline bool
gomp_barrier_has_completed (gomp_barrier_state_t state, gomp_barrier_t *bar)
{
- /* Handling overflow in the generation. The "next" state could be less than
- or greater than the current one. */
- return gomp_barrier_state_is_incremented (bar->generation, state);
+ unsigned int gen = bar->generation;
+ return (gen & -BAR_INCR) == (state & -BAR_INCR) + BAR_INCR;
}
#endif /* GOMP_BARRIER_H */
diff --git a/libgomp/config/linux/bar.c b/libgomp/config/linux/bar.c
index bbdfc896391..330b37a9692 100644
--- a/libgomp/config/linux/bar.c
+++ b/libgomp/config/linux/bar.c
@@ -120,7 +120,7 @@ gomp_team_barrier_wait_end (gomp_barrier_t *bar,
gomp_barrier_state_t state)
}
generation |= gen & BAR_WAITING_FOR_TASK;
}
- while (!gomp_barrier_state_is_incremented (gen, state));
+ while ((gen & -BAR_INCR) != state + BAR_INCR);
}
void
@@ -189,7 +189,7 @@ gomp_team_barrier_wait_cancel_end (gomp_barrier_t *bar,
}
generation |= gen & BAR_WAITING_FOR_TASK;
}
- while (!gomp_barrier_state_is_incremented (gen, state));
+ while ((gen & -BAR_INCR) != state + BAR_INCR);
return false;
}
diff --git a/libgomp/config/linux/bar.h b/libgomp/config/linux/bar.h
index 4dc0d3cca99..b7399f88dc0 100644
--- a/libgomp/config/linux/bar.h
+++ b/libgomp/config/linux/bar.h
@@ -168,20 +168,11 @@ gomp_team_barrier_done (gomp_barrier_t *bar,
gomp_barrier_state_t state)
MEMMODEL_RELEASE);
}
-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;
- return next_state > state ? gen >= next_state : gen < state;
-}
-
static inline bool
gomp_barrier_has_completed (gomp_barrier_state_t state, gomp_barrier_t *bar)
{
- /* Handling overflow in the generation. The "next" state could be less than
- or greater than the current one. */
- return gomp_barrier_state_is_incremented (bar->generation, state);
+ unsigned int gen = bar->generation;
+ return (gen & -BAR_INCR) == (state & -BAR_INCR) + BAR_INCR;
}
#endif /* GOMP_BARRIER_H */
diff --git a/libgomp/config/nvptx/bar.h b/libgomp/config/nvptx/bar.h
index aa2592ba5b3..fd375696580 100644
--- a/libgomp/config/nvptx/bar.h
+++ b/libgomp/config/nvptx/bar.h
@@ -169,20 +169,11 @@ gomp_team_barrier_done (gomp_barrier_t *bar,
gomp_barrier_state_t state)
bar->generation = (state & -BAR_INCR) + BAR_INCR;
}
-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;
- return next_state > state ? gen >= next_state : gen < state;
-}
-
static inline bool
gomp_barrier_has_completed (gomp_barrier_state_t state, gomp_barrier_t *bar)
{
- /* Handling overflow in the generation. The "next" state could be less than
- or greater than the current one. */
- return gomp_barrier_state_is_incremented (bar->generation, state);
+ unsigned int gen = bar->generation;
+ return (gen & -BAR_INCR) == (state & -BAR_INCR) + BAR_INCR;
}
#endif /* GOMP_BARRIER_H */
diff --git a/libgomp/config/posix/bar.c b/libgomp/config/posix/bar.c
index c46659bd264..8dbf7be08ea 100644
--- a/libgomp/config/posix/bar.c
+++ b/libgomp/config/posix/bar.c
@@ -158,7 +158,7 @@ gomp_team_barrier_wait_end (gomp_barrier_t *bar,
gomp_barrier_state_t state)
gen = __atomic_load_n (&bar->generation, MEMMODEL_ACQUIRE);
}
}
- while (!gomp_barrier_state_is_incremented (gen, state));
+ while ((gen & -BAR_INCR) != state + BAR_INCR);
#ifdef HAVE_SYNC_BUILTINS
n = __sync_add_and_fetch (&bar->arrived, -1);
@@ -232,7 +232,7 @@ gomp_team_barrier_wait_cancel_end (gomp_barrier_t *bar,
break;
}
}
- while (!gomp_barrier_state_is_incremented (gen, state));
+ while ((gen & -BAR_INCR) != state + BAR_INCR);
#ifdef HAVE_SYNC_BUILTINS
n = __sync_add_and_fetch (&bar->arrived, -1);
diff --git a/libgomp/config/posix/bar.h b/libgomp/config/posix/bar.h
index 026daca793d..8aab3733cc7 100644
--- a/libgomp/config/posix/bar.h
+++ b/libgomp/config/posix/bar.h
@@ -158,20 +158,11 @@ gomp_team_barrier_done (gomp_barrier_t *bar,
gomp_barrier_state_t state)
MEMMODEL_RELEASE);
}
-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;
- return next_state > state ? gen >= next_state : gen < state;
-}
-
static inline bool
gomp_barrier_has_completed (gomp_barrier_state_t state, gomp_barrier_t *bar)
{
- /* Handling overflow in the generation. The "next" state could be less than
- or greater than the current one. */
- return gomp_barrier_state_is_incremented (bar->generation, state);
+ unsigned int gen = bar->generation;
+ return (gen & -BAR_INCR) == (state & -BAR_INCR) + BAR_INCR;
}
#endif /* GOMP_BARRIER_H */
diff --git a/libgomp/config/rtems/bar.h b/libgomp/config/rtems/bar.h
index 80fb1cd3be8..47b961c2104 100644
--- a/libgomp/config/rtems/bar.h
+++ b/libgomp/config/rtems/bar.h
@@ -170,20 +170,11 @@ gomp_team_barrier_done (gomp_barrier_t *bar,
gomp_barrier_state_t state)
MEMMODEL_RELEASE);
}
-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;
- return next_state > state ? gen >= next_state : gen < state;
-}
-
static inline bool
gomp_barrier_has_completed (gomp_barrier_state_t state, gomp_barrier_t *bar)
{
- /* Handling overflow in the generation. The "next" state could be less than
- or greater than the current one. */
- return gomp_barrier_state_is_incremented (bar->generation, state);
+ unsigned int gen = bar->generation;
+ return (gen & -BAR_INCR) == (state & -BAR_INCR) + BAR_INCR;
}
#endif /* GOMP_BARRIER_H */
--
2.43.0
