Alex wrote:
Here is a follow up patch for documentation of the omp.h allocators,
I'm not super happy with it but I wanted to get eyes on it before I go
to sleep tonight.
I want the table in there somewhere but I'm not confident that where I
put it was the right place.
I think having the C++ template classes listed under the OMP_ALLOCATOR
environment variable feels odd.
I think it is best to move the two tables, the existing one under
"OMP_ALLOCATOR – Set the default allocator",
https://https://gcc.gnu.org/onlinedocs/libgomp/OMP_005fALLOCATOR.html
and the one you added to "11.3 Memory allocation",
https://gcc.gnu.org/onlinedocs/libgomp/Memory-allocation.html
In OMP_ALLOCATOR, I think a sentence such as:
"For the list of available predefined allocators and memory spaces, see
@ref{Memory allocation}."
And then in "Memory Allocator" changing:
"For the available predefined allocators and, as applicable, their
associated predefined memory spaces and for the available traits and
their default values, see "OMP_ALLOCATOR – Set the default allocator."
Predefined allocators without an associated memory space use the
omp_default_mem_space memory space. See additionally Offload-Target
Specifics."
to something like:
"For the available traits and their default values, see ..."
Or we move the traits as well - and then refer to the available traits
and their default in OMP_ALLOCATOR, which seems to be even a bit
cleaner.
And I would add a first bullet point (before the "OpenMP API routines") to
@item The environment variable @ref{OMP_ALLOCATOR}.
(which expands to "OMP_ALLOCATOR" or "OMP_ALLOCATOR – Set the default
allocator") - which seems to be missing from the list.
These are located in the omp::allocator namespace, […] extensions.
std::vector<int, omp::allocator::cgroup_mem<int>> vec;
I think an intro/lead to the example is missing - like: "For instance"
or "The allocator templates can be used with allocator-aware containers
like".
Otherwise, the example comes a bit sudden and without context.
Except for some additional @code and the other comments by Sandra,
changes LGTM.
Thanks,
Tobias
