Hi Sandra,
Sandra Loosemore wrote:
+ /* omp_initial_device is -1, omp_invalid_device is -4; choose
+ a value that isn't otherwise defined to indicate the default
+ device. */
+ device_num = build_int_cst (integer_type_node, -2);
Don't do this - we do it differently for 'target' and it should do the
same. Some value usage history:
Without caring for backward compatibility, I think we had somewhere
#define OMP_DEFAULT_DEVICE -2
and would simply use it everywhere when doing API calls.
But to handle old code, we have to handle both:
-1 → default device
and
-1 → initial device (= host).
Before coming back to your code, let's try to explain the history
and reason again. Maybe I manage to explain it better this time:
* * *
The problem is that -1 on the user side and -1 on the internal-use
side mean different things. Namely:
In the old days OpenMP had on the user side:
device numbers 0 ... omp_get_num_devices()
where the upper bound was the initial device (= host),
omp_get_initial_device().
For
omp target num_device(n)
the device number has to be passed to the run time – and GCC just passes
"n" here.
But GCC also needs to handle:
omp target
i.e. not specifying a device number (= using the default device). It has
been implemented in the obvious way, i.e. passing '-1'.
Later, OpenMP added:
omp_initial_device == -1
omp_invalid_device (negative, implementation defined, !=
omp_initial_device)
GCC set the latter rather arbitrary to -4.
RESULT: Everything works fine, except for -1 as
omp target device_num(omp_initial_device)
and
omp target
are now the same, but semantically one uses the host and the other the
default device.
Therefore, GCC uses:
(A) API routines - use omp_initial_device == -1 as value.
(B) Directives - use -1 for no clause (= backward compatible), using the
default device.
Using -2 for omp_initial_device.
Hence, the following defines exist:
#define GOMP_DEVICE_ICV -1
#define GOMP_DEVICE_HOST_FALLBACK -2
#define GOMP_DEVICE_INVALID -4
If you call an OpenMP runtime API routine, you need to use -1 for the
initial device and for GOMP_* functions related to directives -2 using
GOMP_DEVICE_HOST_FALLBACK, when constructing it manually.
Code wise, GCC handles num_device(n) by generating code like:
if !num_device
devnum = GOMP_DEVICE_ICV;
else
devnum = (n == -1) ? GOMP_DEVICE_HOST_FALLBACK : n;
That's not ideal but one solution to handle backward compatibility.
Inside libgomp/target.c, there is:
resolve_device (int device_id, bool remapped)
and 'remapped' is
- 'false' for OpenMP API routines and
- 'true' for GOMP_* calls.
The following code in resolve_device does then undo the '-1':
if (remapped && device_id == GOMP_DEVICE_ICV)
device_id = icv->default_device_var;
remapped = false;
if (device_id < 0)
if (device_id == (remapped ? GOMP_DEVICE_HOST_FALLBACK
: omp_initial_device))
return NULL;
* * *
Now coming back to your code:
If you call
resolve_device
directly, using the GOMP_* variant makes sense, i.e. passing
the device number as is with 'remap = true'. This also makes
sense for consistency with the remaining code.
Downside: This requires to add
(n == -1) ? -2 : n
for user-specified 'n'.
If you handle the device_num resolution yourself in libgomp, you have
two variants to chose from:
(a) using a different value to denote the default-device (e.g. '-2' or
'-3') and pass it as is
(b) call resolve_device with remapping in libgomp, but handling -1 for
the default device as '(n == -1) ? -2 : n' during code gen
I think either works - and either variant is confusing in one way or the
other.
* * *
Jumping to:
[PATCH v2 03/12] libgomp: runtime support for target_device selector
libgomp/target.c:
+bool
+GOMP_evaluate_target_device (int device_num, const char *kind,
+ const char *arch, const char *isa)
+{
If you do the remapping, you could just use:
struct gomp_device_descr *devicep = resolve_device (device, true);
if (kind && strcmp (kind, "any") == 0)
kind = NULL;
if (devicep == NULL)
result = GOMP_evaluate_current_device (kind, arch, isa);
else
result = device->evaluate_device_func (device_num, kind, arch, isa);
which seems to be simpler than the code you have.
If you don't do the remapping:
+ bool result = true;
+
+ /* -2 is a magic number to indicate the device number was not specified;
+ in that case it's supposed to use the default device. */
+ if (device_num == -2)
+ device_num = omp_get_default_device ();
… then you need to handle -2 yourself.
+ if (kind && strcmp (kind, "any") == 0)
+ kind = NULL;
+
+ gomp_debug (1, "%s: device_num = %u, kind=%s, arch=%s, isa=%s",
+ __FUNCTION__, device_num, kind, arch, isa);
+
+ if (omp_get_device_num () == device_num)
+ result = GOMP_evaluate_current_device (kind, arch, isa);
Here, you additionally need to take care of omp_initial_device, e.g.
if (device_num == -1 /* omp_initial_device */
|| device_num == gomp_get_num_devices ())
(note the 'gomp_' to avoid two indirections)
+ else
+ {
+ if (!omp_is_initial_device ())
+ /* Accelerators are not expected to know about other devices. */
+ result = false;
This is always false. At least for GCN and NVPTX as those have their own
file and functions under libgomp → config/gcn/target.c and
config/nvptx/target.c
In turn, if GOMP_evaluate_target_device can be called on the device
side, you need to add it to those files. I think this is compile-time
resolvable, but I have not checked.
(And if it is callable but protected by a run-time 'if(false)' you have
to add a stub with 'gcc_unreachable()' to avoid linking errors. But I
think the TARGET_OMP_DEVICE_KIND_ARCH_ISA function already takes care
of resolving this at compile time. [Not checked.])
* * *
+ else
+ {
+ struct gomp_device_descr *device = resolve_device (device_num, true);
Semantically, this should use 'false' here as there is no remapping
done. It does not really matter as '-2'/default-device has been resolved
above and the inital device (-1 and num_devices) is handled above.
BTW: This code handles omp_invalid_device via error termination. (As it
should.)
It also handles the OMP_TARGET_OFFLOAD=mandatory diagnostic.
+ if (device == NULL)
+ result = false;
+ else if (device->evaluate_device_func)
+ result = device->evaluate_device_func (device_num, kind, arch,
+ isa);
+ }
+ }
+
+ gomp_debug (1, " -> %s\n", result ? "true" : "false");
+ return result;
+}
+
* * *
Due to backward compatibility issues, there is no real clean solution
for the magic value, whether '-1' is the default device or the initial
device.
I think both variants are acceptable, but either way, libgomp/target.c
needs some tweaking, cf. above.
* * *
Question: Is now everything clear - or are you still confused by my writing?
Tobias
