On 8/24/24 09:13, Tobias Burnus wrote:
+@node Foreign-runtime support for AMD GPUs
+@subsection OpenMP @code{interop} -- Foreign-Runtime Support for AMD GPUs
Em-dash in Texinfo is three dashes with no surrounding spaces. I
believe that the libgomp manual uses the incorrect markup you have here
in its usual template for documenting specific runtime routines (and I'm
not telling you to change that everywhere), but that's not how you're
using it here.
+An interoperability object of OpenMP @code{interop} type can be obtained using
+the @code{interop} directive; supported as foreign runtimes are HIP
+(C++ Heterogeneous-Compute Interface for Portability) and HSA (Heterogeneous
+System Architecture). If no @code{prefer_type} argument has been specified,
+HIP is used.
+
+The following properties can then be extracted using the @ref{Interoperability
+Routines}. Each listed property name has an associated named constant,
It would be much better to rewrite this in the active voice, phrasing as
"you can do X" instead of "X can be done", and "GCC supports the foreign
runtimes ..." instead of the awkward "supported as foreign runtimes are
...".
Also, "if no @code{prefer_type} argument has been specified"....
argument to what? Looking it up in the spec, it appears that is
actually a modifier to the @code{init} clause of the @code{interop}
directive? I think this needs to be said explicitly.
+consisting of @code{omp_ipr_} followed by the property name. The following
s/@code/@samp/ (as already noted in my comments on your previous patch)
+table uses ``@emph{int}'', ``@emph{str}'' and ``@emph{ptr}'' to denote the
+routine to be used to obtain the property value.
Ugh, we should never be using literal quotes for markup purposes. :-(
I'd say just use @samp instead of @emph here.
+Available properties for an HIP interop object:
+@multitable @columnfractions .30 .30 .30
+@headitem Property @tab data type @tab
value (if constant)
+@item @code{fr_id} @tab @samp{omp_interop_fr_t} @emph{(int)} @tab
@samp{omp_fr_hip}
+@item @code{fr_name} @tab @samp{const char *} @emph{(str)} @tab
@samp{hip}
+@item @code{vendor} @tab @samp{int} @emph{(int)} @tab
@samp{1}
+@item @code{vendor_name} @tab @samp{const char *} @emph{(str)} @tab
@samp{amd}
+@item @code{device_num} @tab @samp{int} @emph{(int)} @tab
+@item @code{platform} @tab N/A @tab
+@item @code{device} @tab @samp{hipDevice_t} @emph{(int)} @tab
+@item @code{device_context} @tab @samp{hipCtx_t} @emph{(ptr)} @tab
+@item @code{targetsync} @tab @samp{hipStream_t} @emph{(ptr)} @tab
+@end multitable
Ugh, again. :-( You have 4 columns in the table that you are trying to
format into 3, and I think you really want to have 5 columns (property
name, constant name, data type, accessor function, and value). And I
think you are going to overflow the right margin for PDF output of the
manual if you try making the table that wide. Can you instead format it
as a vertical table for each property, like
@table @code
@item fr_id
@table @asis
@item Named constant:
@code{omp_ipr_fr_id}
@item Data type:
@code{omp_interop_fr_t}
@item Accessor:
@code{omp_get_interop_int}
@item Property value:
@code{omp_fr_hip}
@end table
...
@end table
I don't think the @samp and @emph markup you have is helpful, just use
@code for all the fields.
Similarly for all the other tables in your patch.
Also, here:
+@node Foreign-runtime support for Nvidia GPUs
+@subsection OpenMP @code{interop} -- Foreign-Runtime Support for Nvidia GPUs
+
+An interoperability object of OpenMP @code{interop} type can be obtained using
+the @code{interop} directive; supported as foreign runtimes are the CUDA
+runtime API, the CUDA driver API, and the C++ Heterogeneous-Compute Interface
+for Portability (HIP), which is -- for CUDA-based systems -- a very thin layer
+on top of the CUDA APIs. If no @code{prefer_type} argument has been specified,
+the CUDA runtime API is used.
You have more problems with incorrect em-dash markup (the ones in the
body of the paragraph are certainly incorrect Texinfo usage even if you
want to argue about the one in the @subsection name), use of the passive
voice, and @code{prefer_type} confusion, as previously noted.
-Sandra
