On 09.06.22 13:40, Jakub Jelinek via Gcc-patches wrote:
On Wed, Jun 08, 2022 at 05:56:02AM +0200, Tobias Burnus wrote:
+ && lookup_attribute ("omp declare target",
+ DECL_ATTRIBUTES (current_function_decl)))
+ omp_requires_mask
+ = (enum omp_requires) (omp_requires_mask | OMP_REQUIRES_TARGET_USED);
I must admit it is unclear what the
"must appear lexically before any device constructs or device routines."
restriction actually means for device routines.
Is that lexically before definition of such device routines, or even their
declarations?
I have similar issues – also for Fortran (and C++) module use. Hence, I
had filled https://github.com/OpenMP/spec/issues/3240 (not publicly
accessible); I added your issues to the list.
The above patch snippet is I believe for function definitions that were
arked as declare target before the definition somehow (another decl for
it merged with the new one or in between the begin/end). And is true
even for device_type (host), to rule that out you'd need to check for
"omp declare target host" attribute not being present.
I'm not against the above snippet perhaps adjusted for device_type(host),
but IMHO we want clarifications from omp-lang
How to proceed for now? And does 'omp_is_initial_device()' on the host a
device function or not? It can be hard-coded to 'true' ...
[...]
target update is also a device construct and the above snippet hasn't been
added for it, ditto for interop which we don't implement yet.
But, my preference would be instead of adding these snippets to
c_parser_omp_target_{data,enter_data,exit_data,update} etc. move it from
c_parser_omp_target to c_parser_omp_all_clauses:
if (flag_openmp
&& (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_DEVICE)) != 0)
omp_requires_mask
= (enum omp_requires) (omp_requires_mask | OMP_REQUIRES_TARGET_USED);
(somewhere at the start of the function), because the definition of device
constructs is exactly like that:
"device construct An OpenMP construct that accepts the device clause."
Makes sense.
[C++ cases]
Ditto.
For Fortran, is the above mostly not needed because requires need to be in
the specification part and device constructs are executable and appear in
the part after it? Do we allow requires in BLOCK's specification part?
We don't allow it in BLOCK – but there are issues related to USE-ing
modules, cf. OpenMP issue.
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -3644,6 +3644,9 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool
want_value)
+ if (fndecl && flag_openmp && omp_runtime_api_call (fndecl, true))
+ omp_requires_mask
+ = (enum omp_requires) (omp_requires_mask | OMP_REQUIRES_TARGET_USED);
I'm sure device APIs were discussed, but I must be blind and I can't find it
in either 5.0, 5.1 or 5.2. All I see is device constructs or device routines
in those places where I'd also look for device related OpenMP runtime
library APIs. Though, if some routine calls omp_get_num_devices (),
certainly the library at that point needs to know
reverse_offload/unified_shared_memory/etc. requires because that determines
how many devices it has. So, what have I missed (aka on which place in the
standard the above snippet is based on)?
It is based on your review comments from last year ("Something I miss in
the patch is that for the device API calls") plus what requires some
device initialization. But otherwise, I also did not see it.
In terms of parsing, it makes no difference – contrary to
'unified_shared_memory', where the parser could decide not to add
implicit mapping, the compiler part is not affected by API calls.
I cannot really make up my mind whether it should be required in this
case or not. Maybe, it is not needed.
+ const char *requires_section = ".gnu.gomp_requires";
+ tree maskvar = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+ get_identifier (".gomp_requires_mask"),
+ unsigned_type_node);
+ SET_DECL_ALIGN (maskvar, TYPE_ALIGN (unsigned_type_node));
Don't we want also DECL_USER_ALIGN (maskvar) = 1; so that
we never try to increase its alignment?
Probably yes.
Is it an allocated section, or should it be better non-allocated and then
dealt with by mkoffload?
Shouldn't the vars in that section be const, so that it is a read-only
section?
Is unsigned_type_node what we want (say wouldn't be just unsigned_char_node
be enough, currently we just need 3 bits).
Probably -that would be 8 bits, leaving 5 spare. I have not checked what
Andrew et al. do with the pinned-memory support by -f<some-flag>, but
that will likely use only 1 to 3 bits, if any.
Also, wonder if for HAVE_GAS_SHF_MERGE && flag_merge_constants
we shouldn't try to make that section mergeable. If it goes away during
linking and is replaced by something, then it doesn't matter, but otherwise,
as we don't record which TU had what flags, all we care about is that
there were some TUs which used device construct/routines (and device APIs?)
and used bitmask 7, other TUs that used bitmask 3 and others that used
bitmask 4.
(maybe – I am not sure about this, either.)
@@ -442,6 +463,14 @@ omp_finish_file (void)
}
else
{
+#ifndef ACCEL_COMPILER
+ if (flag_openmp
+ && (omp_requires_mask & OMP_REQUIRES_TARGET_USED)
+ && (omp_requires_mask & (OMP_REQUIRES_UNIFIED_ADDRESS
+ | OMP_REQUIRES_UNIFIED_SHARED_MEMORY
+ | OMP_REQUIRES_REVERSE_OFFLOAD)))
+ sorry ("OpenMP device offloading is not supported for this target");
+#endif
I don't understand this snippet. Without named sections on the host,
I bet we simply don't support offloading at all,
the record_offload_symbol target hook is only non-trivially defined
for nvptx and nvptx isn't typical host for OpenMP offloading,
because we don't remember it anywhere.
I thought that would address your: "This probably needs to sorry if the
target doesn't support named sections. We probably don't support LTO in
that case either though."
I wonder if we shouldn't rename it when we change the arguments,
so that if one mixes an older plugin with newer libgomp or vice versa
this is easily caught.
Ok.
+/* Start/end of .gnu.gomp.requires section of program, defined in
Isn't it .gnu.gomp_requires ?
Yes.
+ crtoffloadbegin/end.o. */
+__attribute__((weak))
+extern const unsigned int __requires_mask_table[];
+__attribute__((weak))
+extern const unsigned int __requires_mask_table_end[];
I must say it is unclear to me how this works.
It will only find one such array, say in the executable,
or if the executable doesn't have it, in one of the shared libraries.
I think we want some solution that will work with OpenMP code
at least in the executable and some shared libraries it is linked against
(obviously another case is when a library with certain #pragma omp requires
is dlopened after we've finalized the number of devices, bet the options
in that case are either warn or fatal error).
There is the general problem that GCC does not work well with having
device routines in a shared library; it works fine if the device part
is only in the library – but calling from an .exe program's target
region a declare-target function in a library won't work.
Thus, we may need to find a more general solution for this.
The choices could be e.g. make __requires_mask_table{,_end} .hidden
and in the crtoffloadbegin.o (or end) unconditionally call some new libgomp
routine to register the table, but the disadvantage of that is that we could
have many of those register calls even when there is nothing to register
(sure, the .ctor in crtoffloadbegin.o (or end) could compare the 2 addresses
and not call anything if the table is empty but there would be still code
executed during initialization of the library).
Yet another possibility is linker plugin case. We already use it for the
case where we actually have some offloading LTO bytecode, transform it into
a data section and register with GOMP_offload_register*.
So, if we could e.g. at the same time also process those requires arrays,
diagnose at link time if multiple TUs with that set disagree on the mask
value and in the target data provide that mask to the library, that would
be much nicer.
And the masks either could be gathered from .gnu.gomp_requires or it can be
somehow encoded in the offloading LTO or its associated data.
What is important though is that it will work even if we actually don't have
any "omp declare target" functions or variables in the TU or the whole
executable or library, just the requires mask. But that can be dealt with
e.g. by forcing the LTO sections even for that case or so.
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
