diff mbox series

libgomp.texi: Update OMP_PLACES

Message ID 5baf5009-23e5-0f56-cf94-cf200caed701@codesourcery.com
State Changes Requested
Headers show
Series libgomp.texi: Update OMP_PLACES | expand

Commit Message

Tobias Burnus Oct. 18, 2021, 7:22 a.m. UTC
This patch updates the OMP_PLACES description for the recent
OpenMP 5.1 changes.

OK?

I actually wonder when/whether the spec reference
should be updated to OpenMP 5.1 or an additional
reference to it should be added.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Jakub Jelinek Oct. 18, 2021, 7:52 a.m. UTC | #1
On Mon, Oct 18, 2021 at 09:22:51AM +0200, Tobias Burnus wrote:
> This patch updates the OMP_PLACES description for the recent
> OpenMP 5.1 changes.
> 
> OK?
> 
> I actually wonder when/whether the spec reference
> should be updated to OpenMP 5.1 or an additional
> reference to it should be added.
> 
> Tobias
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

> libgomp.texi: Update OMP_PLACES
> 
> libgomp/ChangeLog:
> 
> 	* libgomp.texi (OMP_PLACES): Extend description for OMP 5.1 changes.
> 
> diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
> index e9fa8ba0bf7..58d63c50935 100644
> --- a/libgomp/libgomp.texi
> +++ b/libgomp/libgomp.texi
> @@ -2031,18 +2031,22 @@ When undefined, @env{OMP_PROC_BIND} defaults to @code{TRUE} when
>  @table @asis
>  @item @emph{Description}:
>  The thread placement can be either specified using an abstract name or by an
> -explicit list of the places.  The abstract names @code{threads}, @code{cores}
> -and @code{sockets} can be optionally followed by a positive number in
> -parentheses, which denotes the how many places shall be created.  With
> -@code{threads} each place corresponds to a single hardware thread; @code{cores}
> -to a single core with the corresponding number of hardware threads; and with
> -@code{sockets} the place corresponds to a single socket.  The resulting
> -placement can be shown by setting the @env{OMP_DISPLAY_ENV} environment
> -variable.
> +explicit list of the places.  The abstract names @code{threads}, @code{cores},
> +@code{sockets}, @code{ll_caches} and @code{numa_domains} can be optionally
> +followed by a positive number in parentheses, which denotes the how many places
> +shall be created.  With @code{threads} each place corresponds to a single
> +hardware thread; @code{cores} to a single core with the corresponding number of
> +hardware threads; with @code{sockets} the place corresponds to a single
> +socket; with @code{ll_caches} to a set of cores that shares the last level
> +cache on the device; and @code{numa_domains} to a set of cores for which their
> +closest memory on the device is the same meory and at a similar distance from
> +the cores.  The resulting placement can be shown by setting the
> +@env{OMP_DISPLAY_ENV} environment variable.
>  
>  Alternatively, the placement can be specified explicitly as comma-separated
> -list of places.  A place is specified by set of nonnegative numbers in curly
> -braces, denoting the denoting the hardware threads.  The hardware threads
> +list of places.  A place is specified by a single nonnegative number or
> +by a set of nonnegative numbers in curly braces, denoting the denoting
> +the hardware threads.  The hardware threads
>  belonging to a place can either be specified as comma-separated list of
>  nonnegative thread numbers or using an interval.  Multiple places can also be
>  either specified by a comma-separated list of places or by an interval.  To

The first paragraph looks good, but I think the latter change only adds to
confusion that the following text already has.
At least I'd say by a single nonnegative number which is treated as { number }
or ...
The confusion I see is that for both the placement list and place level the
wording first talks about comma-separated list of ..., and then later says
it can be specified either using that or by using an interval.
While actually it is always comma-separated list of ... or ... intervals,
one can mix the intervals with just mere places/numbers etc.
And there is no mention of ! and that the ! form can't use intervals.

Do you think you could try to reword it so that already in the first
sentence it shows all the 3 options ..., ... interval, !...
(where ... would be place or non-negative number)
or should I?
There is also "after after" in the paragraph.

	Jakub
Tobias Burnus Oct. 20, 2021, 10:54 a.m. UTC | #2
On 18.10.21 09:52, Jakub Jelinek wrote:

> On Mon, Oct 18, 2021 at 09:22:51AM +0200, Tobias Burnus wrote:
>> This patch updates the OMP_PLACES description for the recent
>> OpenMP 5.1 changes.
>> I actually wonder when/whether the spec reference
>> should be updated to OpenMP 5.1 or an additional
>> reference to it should be added.

(That question is still open. I think we have the problem that only 4.5
is fully supported while 5.0+5.1 features are supported and documented
for some items.)

> The first paragraph looks good, but I think the latter change only adds to
> confusion that the following text already has.
> Do you think you could try to reword it ... or should I?

I attached an updated version, but I am also not completely happy with
it. – Actually, the wording in the OpenMP spec is also not clear and
having an incomplete description in words plus a complex and fully
syntax as grammar (but without stating some details or only via an
example) is also not helpful.

As I am not completely happy with the attached patch, I like to leave
the rewording to you; that's unless you only have some minor suggestions.

Thanks,

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Jakub Jelinek Dec. 3, 2021, 2:57 p.m. UTC | #3
On Wed, Oct 20, 2021 at 12:54:05PM +0200, Tobias Burnus wrote:
> libgomp/ChangeLog:
> 
> 	* libgomp.texi (OMP_PLACES): Extend description for OMP 5.1 changes.
> 
> diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
> index e9fa8ba0bf7..aee82ef2ba2 100644
> --- a/libgomp/libgomp.texi
> +++ b/libgomp/libgomp.texi
> @@ -2031,25 +2031,33 @@ When undefined, @env{OMP_PROC_BIND} defaults to @code{TRUE} when
>  @table @asis
>  @item @emph{Description}:
>  The thread placement can be either specified using an abstract name or by an
> -explicit list of the places.  The abstract names @code{threads}, @code{cores}
> -and @code{sockets} can be optionally followed by a positive number in
> -parentheses, which denotes the how many places shall be created.  With
> -@code{threads} each place corresponds to a single hardware thread; @code{cores}
> -to a single core with the corresponding number of hardware threads; and with
> -@code{sockets} the place corresponds to a single socket.  The resulting
> -placement can be shown by setting the @env{OMP_DISPLAY_ENV} environment
> -variable.
> +explicit list of the places.  The abstract names @code{threads}, @code{cores},
> +@code{sockets}, @code{ll_caches} and @code{numa_domains} can be optionally
> +followed by a positive number in parentheses, which denotes the how many places
> +shall be created.  With @code{threads} each place corresponds to a single
> +hardware thread; @code{cores} to a single core with the corresponding number of
> +hardware threads; with @code{sockets} the place corresponds to a single
> +socket; with @code{ll_caches} to a set of cores that shares the last level
> +cache on the device; and @code{numa_domains} to a set of cores for which their
> +closest memory on the device is the same meory and at a similar distance from

s/meory/memory/

> +the cores.  The resulting placement can be shown by setting the
> +@env{OMP_DISPLAY_ENV} environment variable.
>  
>  Alternatively, the placement can be specified explicitly as comma-separated
>  list of places.  A place is specified by set of nonnegative numbers in curly
> -braces, denoting the denoting the hardware threads.  The hardware threads
> +braces, denoting the denoting the hardware threads.  (The curly braces can be

Preexisting issue, "denoting the " is repeated twice, can you please fix
that?

> +omitted when only a single number has been specified.)  The hardware threads

Also, I wouldn't add ()s around the above sentence.

>  belonging to a place can either be specified as comma-separated list of
>  nonnegative thread numbers or using an interval.  Multiple places can also be
>  either specified by a comma-separated list of places or by an interval.  To
> -specify an interval, a colon followed by the count is placed after after
> +specify an interval, a colon followed by the count is placed after
>  the hardware thread number or the place.  Optionally, the length can be
>  followed by a colon and the stride number -- otherwise a unit stride is
> -assumed.  For instance, the following specifies the same places list:
> +assumed. Placing an exclamation mark (@code{!}) directly before a curly

Two spaces after .

> +brace or numbers inside the curley braces (excluding intervals) will

s/curley/curly/

> +exclude those hardware threads.
> +
> +For instance, the following specifies the same places list:
>  @code{"@{0,1,2@}, @{3,4,6@}, @{7,8,9@}, @{10,11,12@}"};
>  @code{"@{0:3@}, @{3:3@}, @{7:3@}, @{10:3@}"}; and @code{"@{0:2@}:4:3"}.
>  

Otherwise LGTM.

	Jakub
diff mbox series

Patch

libgomp.texi: Update OMP_PLACES

libgomp/ChangeLog:

	* libgomp.texi (OMP_PLACES): Extend description for OMP 5.1 changes.

diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index e9fa8ba0bf7..58d63c50935 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -2031,18 +2031,22 @@  When undefined, @env{OMP_PROC_BIND} defaults to @code{TRUE} when
 @table @asis
 @item @emph{Description}:
 The thread placement can be either specified using an abstract name or by an
-explicit list of the places.  The abstract names @code{threads}, @code{cores}
-and @code{sockets} can be optionally followed by a positive number in
-parentheses, which denotes the how many places shall be created.  With
-@code{threads} each place corresponds to a single hardware thread; @code{cores}
-to a single core with the corresponding number of hardware threads; and with
-@code{sockets} the place corresponds to a single socket.  The resulting
-placement can be shown by setting the @env{OMP_DISPLAY_ENV} environment
-variable.
+explicit list of the places.  The abstract names @code{threads}, @code{cores},
+@code{sockets}, @code{ll_caches} and @code{numa_domains} can be optionally
+followed by a positive number in parentheses, which denotes the how many places
+shall be created.  With @code{threads} each place corresponds to a single
+hardware thread; @code{cores} to a single core with the corresponding number of
+hardware threads; with @code{sockets} the place corresponds to a single
+socket; with @code{ll_caches} to a set of cores that shares the last level
+cache on the device; and @code{numa_domains} to a set of cores for which their
+closest memory on the device is the same meory and at a similar distance from
+the cores.  The resulting placement can be shown by setting the
+@env{OMP_DISPLAY_ENV} environment variable.
 
 Alternatively, the placement can be specified explicitly as comma-separated
-list of places.  A place is specified by set of nonnegative numbers in curly
-braces, denoting the denoting the hardware threads.  The hardware threads
+list of places.  A place is specified by a single nonnegative number or
+by a set of nonnegative numbers in curly braces, denoting the denoting
+the hardware threads.  The hardware threads
 belonging to a place can either be specified as comma-separated list of
 nonnegative thread numbers or using an interval.  Multiple places can also be
 either specified by a comma-separated list of places or by an interval.  To