On 15/07/2024 16:36, Thomas Schwinge wrote:
Hi!
On 2024-07-15T12:16:30+0100, Andrew Stubbs <a...@baylibre.com> wrote:
On 15/07/2024 10:29, Thomas Schwinge wrote:
On 2021-11-12T18:58:04+0100, Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
And finally here is a third version, [...]
... which became commit 9fa72756d90e0d9edadf6e6f5f56476029925788
"libgomp, nvptx: Honor OpenMP 5.1 num_teams lower bound".
Attached here is "GCN: Honor OpenMP 5.1 'num_teams' lower bound", which
are exactly the corresponding changes for GCN (see below Jakub's nvptx
changes for reference); OK to push?
That's a lot of convoluted logic to drop in without a single comment!
Well, I'll pass that compliment over to Jakub ;-) -- my code changes just
intend to be a faithful "'s%nvptx%GCN'" of his code changes from back
then.
The GCN bits look fine, and I assume you've probably thought about the
logic here a lot, but I've no idea what you're trying to achieve, or why
you're trying to achieve it (from the patch alone).
Can we have some comments on motivation and goals, please?
Here's the original context:
- <https://inbox.sourceware.org/20211111190313.GV2710@tucnak> "[PATCH] openmp:
Honor OpenMP 5.1 num_teams lower bound"
- <https://inbox.sourceware.org/20211112132023.GC2710@tucnak> "[PATCH] libgomp,
nvptx: Honor OpenMP 5.1 num_teams lower bound"
Is that sufficient, and/or would you like to see some commentary to the
relevant libgomp generic/nvptx/GCN code added?
Yes, sorry if it wasn't clear; I meant *code* comments.
/* The team number is usually the same as the gcn_dim_pos(0), except
when num_teams(N) is ..... */
The FIXME actually tells me something useful about one of the
conditional cases, but that's being removed here.
Also, why are we returning "false" in other cases, and what effect does
that have? Is that for "spare" teams when we launch more than we need?
Andrew
