[patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory

[patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]

I intent to commit this tomorrow, unless there are comments.

It does as it says (see commit log): It initializes default-device-var
to the value using the algorithm described in OpenMP 5.2, which
depends on whether OMP_TARGET_OFFLOAD=mandatory was set.

NOTE: With -foffload=disable there is no binary code but still
devices get found - such that default-device-var == 0 (= first
nonhost device). Thus, in that case, libgomp runs the code on that
device but as no binary data is available, host fallback is used.
(Even if there would be executable code for another device on
the system.)
With mandatory, this unintended host fallback is detected and an
error is diagnosed. One can argue whether keeping the devices
makes sense (e.g. because in a dynamic library device code will
be loaded later) or not (don't list if no code is available).

Note that TR11 (future OpenMP 6.0) extends OMP_DEFAULT_DEVICE and
adds OMP_AVAILABLE_DEVICES which permit a finer-grained control about
the device, including OMP_DEFAULT_DEVICE=initial (and 'invalid') which
the current scheme does not permit. (Well, there is
OMP_TARGET_OFFLOAD=disabled, but that's a too big hammer.)

Tobias

PS:  DejaGNU testing was done without offloading configured
and with remote testing on a system having an offload device,
which which does not support setting environment variables.
Manual testing was done with offloading enabled and depending
on the testcase, running on a system with and/or without offloading
hardware.
-----------------
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

[-- Attachment #2: omp-mandatory.diff --]
[-- Type: text/x-patch, Size: 16166 bytes --]

OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory

OMP_TARGET_OFFLOAD=mandatory handling was before inconsistent. Hence, in
OpenMP 5.2 it was clarified/extended by having implications on the
default-device-var; additionally, omp_initial_device and omp_invalid_device
enum values/PARAMETERs were added; support for it was added
in r13-1066-g1158fe43407568 including aborting for omp_invalid_device and
non-conforming device numbers. Only the mandatory handling was missing.

Namely, while the default-device-var is usually initialized to value 0,
with 'mandatory' it must have the value 'omp_invalid_device' if and only if
zero non-host devices are available. (The OMP_DEFAULT_DEVICE env var
overrides this as it comes semantically after the initialization.)

To achieve this, default-device-var is now initialized to MIN_INT. If
there is no 'mandatory', it is set to 0 directly after env var parsing.
Otherwise, it is updated in gomp_target_init to either 0 or
omp_invalid_device. To ensure INT_MIN is never seen by the user, both
the omp_get_default_device API routine and omp_display_env (user call
and OMP_DISPLAY_ENV env var) call gomp_init_targets_once() in that case.

libgomp/ChangeLog:

	* env.c (gomp_default_icv_values): Init default_device_var to
	an nonconforming value - INT_MIN.
	(initialize_env): After env-var parsing, set default_device_var to
	device 0 unless OMP_TARGET_OFFLOAD=mandatory.
	(omp_display_env): If default_device_var is INT_MIN, call
	gomp_init_targets_once.
	* icv-device.c (omp_get_default_device): Likewise.
	* libgomp.texi (OMP_DEFAULT_DEVICE): Update init description.
	(OpenMP 5.2 Impl. Status): Mark OMP_TARGET_OFFLOAD=mandatory as 'Y'.
	* target.c (resolve_device): Improve error message device-num < 0
	with 'mandatory' and no no-host devices available.
	(gomp_target_init): Set default-device-var if INT_MIN.
	* testsuite/libgomp.c/target-48.c: New test.
	* testsuite/libgomp.c/target-49.c: New test.
	* testsuite/libgomp.c/target-50.c: New test.
	* testsuite/libgomp.c/target-51.c: New test.
	* testsuite/libgomp.c/target-52.c: New test.
	* testsuite/libgomp.c/target-53.c: New test.
	* testsuite/libgomp.c/target-54.c: New test.

 libgomp/env.c                            | 13 ++++++++--
 libgomp/icv-device.c                     |  4 +++
 libgomp/libgomp.texi                     |  4 ++-
 libgomp/target.c                         | 15 ++++++++++-
 libgomp/testsuite/libgomp.c/target-48.c  | 31 +++++++++++++++++++++++
 libgomp/testsuite/libgomp.c/target-49.c  | 18 +++++++++++++
 libgomp/testsuite/libgomp.c/target-50.c  | 27 ++++++++++++++++++++
 libgomp/testsuite/libgomp.c/target-50a.c | 43 ++++++++++++++++++++++++++++++++
 libgomp/testsuite/libgomp.c/target-51.c  | 24 ++++++++++++++++++
 libgomp/testsuite/libgomp.c/target-52.c  | 25 +++++++++++++++++++
 libgomp/testsuite/libgomp.c/target-53.c  | 22 ++++++++++++++++
 libgomp/testsuite/libgomp.c/target-54.c  | 20 +++++++++++++++
 12 files changed, 242 insertions(+), 4 deletions(-)

diff --git a/libgomp/env.c b/libgomp/env.c
index e7a035b593c..25c0211dda1 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -62,13 +62,14 @@
 #include "secure_getenv.h"
 #include "environ.h"
 
-/* Default values of ICVs according to the OpenMP standard.  */
+/* Default values of ICVs according to the OpenMP standard,
+   except for default-device-var.  */
 const struct gomp_default_icv gomp_default_icv_values = {
   .nthreads_var = 1,
   .thread_limit_var = UINT_MAX,
   .run_sched_var = GFS_DYNAMIC,
   .run_sched_chunk_size = 1,
-  .default_device_var = 0,
+  .default_device_var = INT_MIN,
   .max_active_levels_var = 1,
   .bind_var = omp_proc_bind_false,
   .nteams_var = 0,
@@ -1614,6 +1615,10 @@ omp_display_env (int verbose)
   struct gomp_icv_list *none
     = gomp_get_initial_icv_item (GOMP_DEVICE_NUM_FOR_NO_SUFFIX);
 
+  if (none->icvs.default_device_var == INT_MIN)
+    /* This implies OMP_TARGET_OFFLOAD=mandatory.  */
+    gomp_init_targets_once ();
+
   fputs ("\nOPENMP DISPLAY ENVIRONMENT BEGIN\n", stderr);
 
   fputs ("  _OPENMP = '201511'\n", stderr);
@@ -2213,6 +2218,10 @@ initialize_env (void)
 	gomp_global_icv.max_active_levels_var = gomp_supported_active_levels;
     }
 
+  if (gomp_global_icv.default_device_var == INT_MIN
+      && gomp_target_offload_var != GOMP_TARGET_OFFLOAD_MANDATORY)
+    none->icvs.default_device_var = gomp_global_icv.default_device_var = 0;
+
   /* Process GOMP_* variables and dependencies between parsed ICVs.  */
   parse_int_secure ("GOMP_DEBUG", &gomp_debug_var, true);
 
diff --git a/libgomp/icv-device.c b/libgomp/icv-device.c
index a2bbedc672a..b48ea3b096c 100644
--- a/libgomp/icv-device.c
+++ b/libgomp/icv-device.c
@@ -27,6 +27,7 @@
    expected to replace.  */
 
 #include "libgomp.h"
+#include <limits.h>
 
 void
 omp_set_default_device (int device_num)
@@ -41,6 +42,9 @@ int
 omp_get_default_device (void)
 {
   struct gomp_task_icv *icv = gomp_icv (false);
+  if (icv->default_device_var == INT_MIN)
+    /* This implies OMP_TARGET_OFFLOAD=mandatory.  */
+    gomp_init_targets_once ();
   return icv->default_device_var;
 }
 
diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index a3d370a0fb3..21d3582a665 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -423,7 +423,7 @@ to address of matching mapped list item per 5.1, Sect. 2.21.7.2 @tab N @tab
 @item Conforming device numbers and @code{omp_initial_device} and
       @code{omp_invalid_device} enum/PARAMETER @tab Y @tab
 @item Initial value of @emph{default-device-var} ICV with
-      @code{OMP_TARGET_OFFLOAD=mandatory} @tab N @tab
+      @code{OMP_TARGET_OFFLOAD=mandatory} @tab Y @tab
 @item @emph{interop_types} in any position of the modifier list for the @code{init} clause
       of the @code{interop} construct @tab N @tab
 @end multitable
@@ -2006,6 +2006,8 @@ Set to choose the device which is used in a @code{target} region, unless the
 value is overridden by @code{omp_set_default_device} or by a @code{device}
 clause.  The value shall be the nonnegative device number. If no device with
 the given device number exists, the code is executed on the host.  If unset,
+@env{OMP_TARGET_OFFLOAD} is @code{mandatory} and no non-host devices are
+available, it is set to @code{omp_invalid_device}.  Otherwise, if unset,
 device number 0 will be used.
 
 
diff --git a/libgomp/target.c b/libgomp/target.c
index e3c4121a09f..f1020fad601 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -150,7 +150,11 @@ resolve_device (int device_id, bool remapped)
       if (device_id == (remapped ? GOMP_DEVICE_HOST_FALLBACK
 				 : omp_initial_device))
 	return NULL;
-      if (device_id == omp_invalid_device)
+      if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
+	  && gomp_get_num_devices () == 0)
+	gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY but only the host "
+		    "device is available");
+      else if (device_id == omp_invalid_device)
 	gomp_fatal ("omp_invalid_device encountered");
       else if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY)
 	gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
@@ -5184,6 +5188,15 @@ gomp_target_init (void)
       if (devs[i].capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
 	goacc_register (&devs[i]);
     }
+  if (gomp_global_icv.default_device_var == INT_MIN)
+    {
+       /* This implies OMP_TARGET_OFFLOAD=mandatory.  */
+       struct gomp_icv_list *none;
+       none = gomp_get_initial_icv_item (GOMP_DEVICE_NUM_FOR_NO_SUFFIX);
+       gomp_global_icv.default_device_var = (num_devs_openmp
+					     ? 0 : omp_invalid_device);
+       none->icvs.default_device_var = gomp_global_icv.default_device_var;
+    }
 
   num_devices = num_devs;
   num_devices_openmp = num_devs_openmp;
diff --git a/libgomp/testsuite/libgomp.c/target-48.c b/libgomp/testsuite/libgomp.c/target-48.c
new file mode 100644
index 00000000000..8e95c1c3ac3
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-48.c
@@ -0,0 +1,31 @@
+/* Check OMP_TARGET_OFFLOAD on systems with no available non-host devices;
+   omp_invalid_device == -4 with GCC.  */
+
+/* { dg-do run { target { ! offload_device } } } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '-4'.*OMP_TARGET_OFFLOAD = 'MANDATORY'.*" } */
+
+#include <omp.h>
+
+int
+main ()
+{
+  if (omp_get_default_device () != omp_invalid_device)
+    __builtin_abort ();
+
+  omp_set_default_device (omp_initial_device);
+
+  /* The spec is a bit unclear whether the line above sets the device number
+     (a) to -1 (= omp_initial_device) or
+     (b) to omp_get_initial_device() == omp_get_num_devices(). Therefore,
+     we accept either value.   */
+
+  if (omp_get_default_device() != omp_get_initial_device()
+      && omp_get_default_device() != omp_initial_device)
+    __builtin_abort ();
+
+  omp_display_env (0);
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-49.c b/libgomp/testsuite/libgomp.c/target-49.c
new file mode 100644
index 00000000000..970cb91d512
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-49.c
@@ -0,0 +1,18 @@
+/* Check OMP_TARGET_OFFLOAD on systems with no available non-host devices,
+   which is enforced by using -foffload=disable.  */
+
+/* { dg-do run } */
+/* { dg-additional-options "-foffload=disable" } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+/* { dg-set-target-env-var OMP_DISPLAY_ENV "true" } */
+
+/* See comment in target-50.c/target-50.c for why default-device-var can be '0'.  */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '-4'.*OMP_TARGET_OFFLOAD = 'MANDATORY'.*" { target { ! offload_device } } } */
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '0'.*OMP_TARGET_OFFLOAD = 'MANDATORY'.*" { target offload_device  } } */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-50.c b/libgomp/testsuite/libgomp.c/target-50.c
new file mode 100644
index 00000000000..6f15569ee21
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-50.c
@@ -0,0 +1,27 @@
+/* Check OMP_TARGET_OFFLOAD on systems with no available non-host devices;
+   here with using -foffload=disable.
+   As default-device-var is set to 0 (= host in this case), it should not fail.  */
+
+/* Note that -foffload=disable will still find devices on the system and only
+   when trying to use them, it will fail as no binary data has been produced.
+   The "target offload_device" case is checked for in 'target-50a.c'.  */
+
+/* { dg-do run { target { ! offload_device } } } */
+
+/* { dg-additional-options "-foffload=disable" } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+/* { dg-set-target-env-var OMP_DEFAULT_DEVICE "0" } */
+/* { dg-set-target-env-var OMP_DISPLAY_ENV "true" } */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '0'.*OMP_TARGET_OFFLOAD = 'MANDATORY'.*" } */
+
+int
+main ()
+{
+  int x;
+  #pragma omp target map(tofrom:x)
+    x = 5;
+  if (x != 5)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-50a.c b/libgomp/testsuite/libgomp.c/target-50a.c
new file mode 100644
index 00000000000..0835cb5bae3
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-50a.c
@@ -0,0 +1,43 @@
+/* Check OMP_TARGET_OFFLOAD on systems with non-host devices but no executable
+   code due to -foffload=disable.
+
+   Note: While one might expect that -foffload=disable implies no non-host
+   devices, libgomp actually detects the devices and only fails when trying to
+   run as no executable code is availale for that device.
+   (Without MANDATORY it simply uses host fallback, which should usually be fine
+   but might have issues in corner cases.)
+
+   We have default-device-var = 0 (default but also explicitly set), which will
+   fail at runtime. For -foffload=disable without non-host devices, see
+   target-50.c testcase.  */
+
+/* { dg-do run { target offload_device } } */
+
+/* { dg-additional-options "-foffload=disable" } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+/* { dg-set-target-env-var OMP_DEFAULT_DEVICE "0" } */
+/* { dg-set-target-env-var OMP_DISPLAY_ENV "true" } */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '0'.*OMP_TARGET_OFFLOAD = 'MANDATORY'.*" } */
+
+#include <omp.h>
+
+int
+main ()
+{
+  int x;
+  /* We know that there are non-host devices. With GCC, we still find them as
+     available devices, hence, check for it.  */
+  if (omp_get_num_devices() <= 0)
+    __builtin_abort ();
+
+  /* But due to -foffload=disable, there are no binary code for (default) device '0'  */
+
+  /* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading.*" } */
+  /* { dg-shouldfail "OMP_TARGET_OFFLOAD=mandatory and no binary code for a non-host device" } */
+  #pragma omp target map(tofrom:x)
+    x = 5;
+  if (x != 5)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-51.c b/libgomp/testsuite/libgomp.c/target-51.c
new file mode 100644
index 00000000000..7d09bceacd5
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-51.c
@@ -0,0 +1,24 @@
+/* Check OMP_TARGET_OFFLOAD on systems with no available non-host devices,
+   which is enforced by using -foffload=disable.  */
+
+/* { dg-do run } */
+/* { dg-additional-options "-foffload=disable" } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+
+/* { dg-shouldfail "OMP_TARGET_OFFLOAD=mandatory and no available device" } */
+
+/* See comment in target-50.c/target-50.c for why the output differs.  */
+
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but only the host device is available.*" { target { ! offload_device } } } */
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but device not found.*" { target offload_device } } */
+
+int
+main ()
+{
+  int x;
+  #pragma omp target map(tofrom:x)
+    x = 5;
+  if (x != 5)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-52.c b/libgomp/testsuite/libgomp.c/target-52.c
new file mode 100644
index 00000000000..809380c6928
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-52.c
@@ -0,0 +1,25 @@
+/* Only run this with available non-host devices; in that case, GCC sets
+   the default-device-var to 0.  */
+
+/* { dg-do run { target { offload_device } } } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+/* { dg-set-target-env-var OMP_DISPLAY_ENV "true" } */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '0'.*OMP_TARGET_OFFLOAD = 'MANDATORY'.*" } */
+
+#include <omp.h>
+
+int
+main ()
+{
+  int x;
+  #pragma omp target map(tofrom:x)
+    x = 5 + omp_is_initial_device ();
+
+  if (x != 5)
+    __builtin_abort ();
+
+  if (0 != omp_get_default_device())
+    __builtin_abort ();
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-53.c b/libgomp/testsuite/libgomp.c/target-53.c
new file mode 100644
index 00000000000..866e8961af1
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-53.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "disabled" } */
+/* { dg-set-target-env-var OMP_DISPLAY_ENV "true" } */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '\[0-9\]+'.*OMP_TARGET_OFFLOAD = 'DISABLED'.*" } */
+
+#include <omp.h>
+
+int
+main ()
+{
+  int x;
+  #pragma omp target map(tofrom:x)
+    x = 5 + omp_is_initial_device ();
+
+  if (x != 5+1)
+    __builtin_abort ();
+
+  if (omp_get_default_device() != omp_get_initial_device())
+    __builtin_abort ();
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-54.c b/libgomp/testsuite/libgomp.c/target-54.c
new file mode 100644
index 00000000000..bc4e69b5278
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-54.c
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "default" } */
+/* { dg-set-target-env-var OMP_DISPLAY_ENV "true" } */
+
+/* { dg-output ".*OMP_DEFAULT_DEVICE = '0'.*OMP_TARGET_OFFLOAD = 'DEFAULT'.*" } */
+
+#include <omp.h>
+
+int
+main ()
+{
+  int x;
+  #pragma omp target map(tofrom:x)
+    x = 5 + omp_is_initial_device ();
+
+  if (x != 5 + (omp_get_default_device() == omp_get_initial_device()))
+    __builtin_abort ();
+
+  return 0;
+}

Fix typo in 'libgomp.c/target-51.c' (was: [patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory)

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 5278 bytes --]

Hi!

On 2023-06-13T20:44:39+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> I intent to commit this tomorrow, unless there are comments.

I'm sorry I'm late.  ;-P

> It does as it says (see commit log): It initializes default-device-var
> to the value using the algorithm described in OpenMP 5.2, which
> depends on whether OMP_TARGET_OFFLOAD=mandatory was set.
>
> NOTE: With -foffload=disable there is no binary code but still
> devices get found - such that default-device-var == 0 (= first
> nonhost device). Thus, in that case, libgomp runs the code on that
> device but as no binary data is available, host fallback is used.
> (Even if there would be executable code for another device on
> the system.)
> With mandatory, this unintended host fallback is detected and an
> error is diagnosed. One can argue whether keeping the devices
> makes sense (e.g. because in a dynamic library device code will
> be loaded later) or not (don't list if no code is available).

This reminds me of the (unresolved) <https://gcc.gnu.org/PR81886>
"Means to determine at runtime foffload targets specified at compile time".

> Note that TR11 (future OpenMP 6.0) extends OMP_DEFAULT_DEVICE and
> adds OMP_AVAILABLE_DEVICES which permit a finer-grained control about
> the device, including OMP_DEFAULT_DEVICE=initial (and 'invalid') which
> the current scheme does not permit. (Well, there is
> OMP_TARGET_OFFLOAD=disabled, but that's a too big hammer.)

> PS:  DejaGNU testing was done without offloading configured
> and with remote testing on a system having an offload device,
> which which does not support setting environment variables.
> Manual testing was done with offloading enabled and depending
> on the testcase, running on a system with and/or without offloading
> hardware.

> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -150,7 +150,11 @@ resolve_device (int device_id, bool remapped)
>        if (device_id == (remapped ? GOMP_DEVICE_HOST_FALLBACK
>                                : omp_initial_device))
>       return NULL;
> -      if (device_id == omp_invalid_device)
> +      if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
> +       && gomp_get_num_devices () == 0)
> +     gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY but only the host "
> +                 "device is available");
> +      else if (device_id == omp_invalid_device)
>       gomp_fatal ("omp_invalid_device encountered");
>        else if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY)
>       gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
|                   "but device not found");
|
|        return NULL;
|      }
|    else if (device_id >= gomp_get_num_devices ())
|      {
|        if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
|         && device_id != num_devices_openmp)
|       gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
|                   "but device not found");
|
|        return NULL;
|      }
|
|    gomp_mutex_lock (&devices[device_id].lock);
|    if (devices[device_id].state == GOMP_DEVICE_UNINITIALIZED)
|      gomp_init_device (&devices[device_id]);
|    else if (devices[device_id].state == GOMP_DEVICE_FINALIZED)
|      {
|        gomp_mutex_unlock (&devices[device_id].lock);
|
|        if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY)
|       gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
|                   "but device is finalized");
|
|        return NULL;
|      }
|    gomp_mutex_unlock (&devices[device_id].lock);
|
|    return &devices[device_id];
|  }

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.c/target-51.c
> @@ -0,0 +1,24 @@
> +/* Check OMP_TARGET_OFFLOAD on systems with no available non-host devices,
> +   which is enforced by using -foffload=disable.  */
> +
> +/* { dg-do run } */
> +/* { dg-additional-options "-foffload=disable" } */
> +/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
> +
> +/* { dg-shouldfail "OMP_TARGET_OFFLOAD=mandatory and no available device" } */
> +
> +/* See comment in target-50.c/target-50.c for why the output differs.  */
> +
> +/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but only the host device is available.*" { target { ! offload_device } } } */
> +/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but device not found.*" { target offload_device } } */

I intend to push the attached "Fix typo in 'libgomp.c/target-51.c'" after
testing.

Let me know if I should also adjust the new 'target { ! offload_device }'
diagnostic "[...] MANDATORY but only the host device is available" to
include a comma before 'but', for consistency with the other existing
diagnostics (cited above)?


Grüße
 Thomas


> +
> +int
> +main ()
> +{
> +  int x;
> +  #pragma omp target map(tofrom:x)
> +    x = 5;
> +  if (x != 5)
> +    __builtin_abort ();
> +  return 0;
> +}


-----------------
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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-typo-in-libgomp.c-target-51.c.patch --]
[-- Type: text/x-diff, Size: 1464 bytes --]

From 2464d87db542b87a1d276637f334e9c6eb35be64 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 14 Jun 2023 09:25:15 +0200
Subject: [PATCH] Fix typo in 'libgomp.c/target-51.c'

..., and therefore, given 'target offload_device':

    PASS: libgomp.c/target-51.c (test for excess errors)
    PASS: libgomp.c/target-51.c execution test
    [-FAIL:-]{+PASS:+} libgomp.c/target-51.c output pattern test

Fix-up for recent commit 18c8b56c7d67a9e37acf28822587786f0fc0efbc
"OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory".

	libgomp/
	* testsuite/libgomp.c/target-51.c: Fix typo.
---
 libgomp/testsuite/libgomp.c/target-51.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgomp/testsuite/libgomp.c/target-51.c b/libgomp/testsuite/libgomp.c/target-51.c
index 7d09bceacd58..cf9e690263e9 100644
--- a/libgomp/testsuite/libgomp.c/target-51.c
+++ b/libgomp/testsuite/libgomp.c/target-51.c
@@ -10,7 +10,7 @@
 /* See comment in target-50.c/target-50.c for why the output differs.  */
 
 /* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but only the host device is available.*" { target { ! offload_device } } } */
-/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but device not found.*" { target offload_device } } */
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found.*" { target offload_device } } */
 
 int
 main ()
-- 
2.39.2

Re: Fix typo in 'libgomp.c/target-51.c' (was: [patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory)

[Tobias Burnus]

On 14.06.23 10:09, Thomas Schwinge wrote:
> This reminds me of the (unresolved)https://gcc.gnu.org/PR81886
> "Means to determine at runtime foffload targets specified at compile time".

I think there is the problem that we also support offloading in
libraries. Thus, if you compile the main program without offloading and
then link in a shared offloading-providing library (possibly with
dlopen), it comes (too) late. Thus, we either exclude devices which
could be later used – or we have to live with providing devices
(existing in hardware and with libgomp support) for which no executable
code is available.

As long as the number of devices is not a dynamic property, I guess we
can only handle one or the other.

> I intend to push the attached "Fix typo in 'libgomp.c/target-51.c'"
> after testing.
> Let me know if I should also adjust the new 'target { ! offload_device }'
> diagnostic "[...] MANDATORY but only the host device is available" to
> include a comma before 'but', for consistency with the other existing
> diagnostics (cited above)?

I think it makes sense to be consistent. Thus: Yes, please add the commas.

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

Align a 'OMP_TARGET_OFFLOAD=mandatory' diagnostic with others (was: Fix typo in 'libgomp.c/target-51.c' (was: [patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory))

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

Hi!

On 2023-06-14T11:42:22+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> On 14.06.23 10:09, Thomas Schwinge wrote:
>> Let me know if I should also adjust the new 'target { ! offload_device }'
>> diagnostic "[...] MANDATORY but only the host device is available" to
>> include a comma before 'but', for consistency with the other existing
>> diagnostics (cited above)?
>
> I think it makes sense to be consistent. Thus: Yes, please add the commas.

I've pushed commit f2ef1dabbc18eb6efc0eb47bbb0eebbc6d72e09e
"Align a 'OMP_TARGET_OFFLOAD=mandatory' diagnostic with others", see
attached.


Grüße
 Thomas


-----------------
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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Align-a-OMP_TARGET_OFFLOAD-mandatory-diagnostic-with.patch --]
[-- Type: text/x-diff, Size: 2585 bytes --]

From f2ef1dabbc18eb6efc0eb47bbb0eebbc6d72e09e Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 14 Jun 2023 12:44:05 +0200
Subject: [PATCH] Align a 'OMP_TARGET_OFFLOAD=mandatory' diagnostic with others

On 2023-06-14T11:42:22+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> On 14.06.23 10:09, Thomas Schwinge wrote:
>> Let me know if I should also adjust the new 'target { ! offload_device }'
>> diagnostic "[...] MANDATORY but only the host device is available" to
>> include a comma before 'but', for consistency with the other existing
>> diagnostics (cited above)?
>
> I think it makes sense to be consistent. Thus: Yes, please add the commas.

Fix-up for recent commit 18c8b56c7d67a9e37acf28822587786f0fc0efbc
"OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory".

	libgomp/
	* target.c (resolve_device): Align a
	'OMP_TARGET_OFFLOAD=mandatory' diagnostic with others.
	* testsuite/libgomp.c/target-51.c: Adjust.
---
 libgomp/target.c                        | 4 ++--
 libgomp/testsuite/libgomp.c/target-51.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libgomp/target.c b/libgomp/target.c
index f1020fad601b..e39ef8f6e82a 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -152,8 +152,8 @@ resolve_device (int device_id, bool remapped)
 	return NULL;
       if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
 	  && gomp_get_num_devices () == 0)
-	gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY but only the host "
-		    "device is available");
+	gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
+		    "but only the host device is available");
       else if (device_id == omp_invalid_device)
 	gomp_fatal ("omp_invalid_device encountered");
       else if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY)
diff --git a/libgomp/testsuite/libgomp.c/target-51.c b/libgomp/testsuite/libgomp.c/target-51.c
index cf9e690263e9..bbe9ade6e24b 100644
--- a/libgomp/testsuite/libgomp.c/target-51.c
+++ b/libgomp/testsuite/libgomp.c/target-51.c
@@ -9,7 +9,7 @@
 
 /* See comment in target-50.c/target-50.c for why the output differs.  */
 
-/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY but only the host device is available.*" { target { ! offload_device } } } */
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available.*" { target { ! offload_device } } } */
 /* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found.*" { target offload_device } } */
 
 int
-- 
2.39.2

[committed] libgomp: Fix OMP_TARGET_OFFLOAD=mandatory

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

Found an order problem caused by my r14-1801-g18c8b56c7d67a9 due to
ordering issues related to the offloading initialization
(gomp_init_targets_once).

The testsuite did test various ways but only code such paths that
initialized the library before ...

Committed as Rev. r14-1893-g8216ca85037be9.

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

[-- Attachment #2: committed.diff --]
[-- Type: text/x-patch, Size: 4194 bytes --]

commit 8216ca85037be9f4d5c20540522a22a4a93b660e
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Fri Jun 16 17:21:59 2023 +0200

    libgomp: Fix OMP_TARGET_OFFLOAD=mandatory
    
    It turned out that gomp_init_targets_once() was not run when directly
    calling 'omp target' or 'omp target (enter/exit) data' causing an
    abort with OMP_TARGET_OFFLOAD=mandatory wrongly claiming that no
    device is available. It was called a tiny bit later but few lines too
    late for updating the default-device-var.
    
    libgomp/ChangeLog:
    
            * target.c (resolve_device): Call gomp_get_num_devices early to ensure
            gomp_init_targets_once was called before using default-device-var.
            * testsuite/libgomp.c/target-55.c: New test.
            * testsuite/libgomp.c/target-55a.c: New test.
---
 libgomp/target.c                         | 10 +++++++---
 libgomp/testsuite/libgomp.c/target-55.c  | 20 ++++++++++++++++++++
 libgomp/testsuite/libgomp.c/target-55a.c | 23 +++++++++++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/libgomp/target.c b/libgomp/target.c
index e39ef8f6e82..b6a7214ab4f 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -138,6 +138,10 @@ gomp_get_num_devices (void)
 static struct gomp_device_descr *
 resolve_device (int device_id, bool remapped)
 {
+  /* Get number of devices and thus ensure that 'gomp_init_targets_once' was
+     called, which must be done before using default_device_var.  */
+  int num_devices = gomp_get_num_devices ();
+
   if (remapped && device_id == GOMP_DEVICE_ICV)
     {
       struct gomp_task_icv *icv = gomp_icv (false);
@@ -151,7 +155,7 @@ resolve_device (int device_id, bool remapped)
 				 : omp_initial_device))
 	return NULL;
       if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
-	  && gomp_get_num_devices () == 0)
+	  && num_devices == 0)
 	gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
 		    "but only the host device is available");
       else if (device_id == omp_invalid_device)
@@ -162,10 +166,10 @@ resolve_device (int device_id, bool remapped)
 
       return NULL;
     }
-  else if (device_id >= gomp_get_num_devices ())
+  else if (device_id >= num_devices)
     {
       if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
-	  && device_id != num_devices_openmp)
+	  && device_id != num_devices)
 	gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
 		    "but device not found");
 
diff --git a/libgomp/testsuite/libgomp.c/target-55.c b/libgomp/testsuite/libgomp.c/target-55.c
new file mode 100644
index 00000000000..1314b3c6963
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-55.c
@@ -0,0 +1,20 @@
+/* { dg-do run { target { offload_device } } } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+
+/* Should pass - see target-55a.c for !offload_device */
+
+/* Check OMP_TARGET_OFFLOAD - it shall run on systems with offloading
+   devices available and fail otherwise.  Note that this did always
+   fail - as the device handling wasn't initialized before doing the
+   mandatory checking.  */
+
+int
+main ()
+{
+  int x = 1;
+  #pragma omp target map(tofrom: x)
+    x = 5;
+  if (x != 5)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/target-55a.c b/libgomp/testsuite/libgomp.c/target-55a.c
new file mode 100644
index 00000000000..53978c3f405
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/target-55a.c
@@ -0,0 +1,23 @@
+/* { dg-do run { target { ! offload_device } } } */
+/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
+
+/* Should fail - see target-55a.c for offload_device */
+
+/* { dg-shouldfail "omp_invalid_device" } */
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available.*" } */
+
+/* Check OMP_TARGET_OFFLOAD - it shall run on systems with offloading
+   devices available and fail otherwise.  Note that this did always
+   fail - as the device handling wasn't initialized before doing the
+   mandatory checking.  */
+
+int
+main ()
+{
+  int x = 1;
+  #pragma omp target map(tofrom: x)
+    x = 5;
+  if (x != 5)
+    __builtin_abort ();
+  return 0;
+}

Re: [committed] libgomp: Fix OMP_TARGET_OFFLOAD=mandatory

[Thomas Schwinge]

Hi Tobias!

On 2023-06-16T17:57:10+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> Found an order problem caused by my r14-1801-g18c8b56c7d67a9 due to
> ordering issues related to the offloading initialization
> (gomp_init_targets_once).
>
> The testsuite did test various ways but only code such paths that
> initialized the library before ...
>
> Committed as Rev. r14-1893-g8216ca85037be9.

> commit 8216ca85037be9f4d5c20540522a22a4a93b660e
> Author: Tobias Burnus <tobias@codesourcery.com>
> Date:   Fri Jun 16 17:21:59 2023 +0200
>
>     libgomp: Fix OMP_TARGET_OFFLOAD=mandatory
>
>     It turned out that gomp_init_targets_once() was not run when directly
>     calling 'omp target' or 'omp target (enter/exit) data' causing an
>     abort with OMP_TARGET_OFFLOAD=mandatory wrongly claiming that no
>     device is available. It was called a tiny bit later but few lines too
>     late for updating the default-device-var.
>
>     libgomp/ChangeLog:
>
>             * target.c (resolve_device): Call gomp_get_num_devices early to ensure
>             gomp_init_targets_once was called before using default-device-var.

> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -138,6 +138,10 @@ gomp_get_num_devices (void)
>  static struct gomp_device_descr *
>  resolve_device (int device_id, bool remapped)
>  {
> +  /* Get number of devices and thus ensure that 'gomp_init_targets_once' was
> +     called, which must be done before using default_device_var.  */
> +  int num_devices = gomp_get_num_devices ();
> +
>    if (remapped && device_id == GOMP_DEVICE_ICV)
>      {
>        struct gomp_task_icv *icv = gomp_icv (false);
> @@ -151,7 +155,7 @@ resolve_device (int device_id, bool remapped)
>                                : omp_initial_device))
>       return NULL;
>        if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
> -       && gomp_get_num_devices () == 0)
> +       && num_devices == 0)
>       gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
>                   "but only the host device is available");
>        else if (device_id == omp_invalid_device)
> @@ -162,10 +166,10 @@ resolve_device (int device_id, bool remapped)
>
>        return NULL;
>      }
> -  else if (device_id >= gomp_get_num_devices ())
> +  else if (device_id >= num_devices)
>      {
>        if (gomp_target_offload_var == GOMP_TARGET_OFFLOAD_MANDATORY
> -       && device_id != num_devices_openmp)
> +       && device_id != num_devices)
>       gomp_fatal ("OMP_TARGET_OFFLOAD is set to MANDATORY, "
>                   "but device not found");

I see the new tests PASS, but with offloading enabled (nvptx) also see:

    PASS: libgomp.c/target-51.c (test for excess errors)
    PASS: libgomp.c/target-51.c execution test
    [-PASS:-]{+FAIL:+} libgomp.c/target-51.c output pattern test

... due to:

    Output was:

    libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading

    Should match:
    .*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found.*


Grüße
 Thomas


> diff --git a/libgomp/testsuite/libgomp.c/target-55.c b/libgomp/testsuite/libgomp.c/target-55.c
> new file mode 100644
> index 00000000000..1314b3c6963
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.c/target-55.c
> @@ -0,0 +1,20 @@
> +/* { dg-do run { target { offload_device } } } */
> +/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
> +
> +/* Should pass - see target-55a.c for !offload_device */
> +
> +/* Check OMP_TARGET_OFFLOAD - it shall run on systems with offloading
> +   devices available and fail otherwise.  Note that this did always
> +   fail - as the device handling wasn't initialized before doing the
> +   mandatory checking.  */
> +
> +int
> +main ()
> +{
> +  int x = 1;
> +  #pragma omp target map(tofrom: x)
> +    x = 5;
> +  if (x != 5)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git a/libgomp/testsuite/libgomp.c/target-55a.c b/libgomp/testsuite/libgomp.c/target-55a.c
> new file mode 100644
> index 00000000000..53978c3f405
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.c/target-55a.c
> @@ -0,0 +1,23 @@
> +/* { dg-do run { target { ! offload_device } } } */
> +/* { dg-set-target-env-var OMP_TARGET_OFFLOAD "mandatory" } */
> +
> +/* Should fail - see target-55a.c for offload_device */
> +
> +/* { dg-shouldfail "omp_invalid_device" } */
> +/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available.*" } */
> +
> +/* Check OMP_TARGET_OFFLOAD - it shall run on systems with offloading
> +   devices available and fail otherwise.  Note that this did always
> +   fail - as the device handling wasn't initialized before doing the
> +   mandatory checking.  */
> +
> +int
> +main ()
> +{
> +  int x = 1;
> +  #pragma omp target map(tofrom: x)
> +    x = 5;
> +  if (x != 5)
> +    __builtin_abort ();
> +  return 0;
> +}
-----------------
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

[committed] libgomp.c/target-51.c: Accept more error-msg variants in dg-output (was: Re: [committed] libgomp: Fix OMP_TARGET_OFFLOAD=mandatory)

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 2167 bytes --]

On 16.06.23 22:42, Thomas Schwinge wrote:
> I see the new tests PASS, but with offloading enabled (nvptx) also see:
>
>      PASS: libgomp.c/target-51.c (test for excess errors)
>      PASS: libgomp.c/target-51.c execution test
>      [-PASS:-]{+FAIL:+} libgomp.c/target-51.c output pattern test
>
> ... due to:
>
>      Output was:
>
>      libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading
>
>      Should match:
>      .*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found.*

Thanks for the report. I can offer yet another wording for the same program – and also
with nvptx enabled:

libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading

And I can also offer (which is already in the testcase with "! offload_device"):

libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available

I think I will just match "..., but .*" without distinguishing check_effective_target_* ...

... which I now did in commit r14-1926-g01fe115ba7eafe (see also attached patch).

* * *

With offloading, there are simply too many possibilities:

* Not compiled with offloading support - vs. with (ENABLE_OFFLOADING)
* Support compiled in but either compiler or library support not installed
   (requires configuring with --enable-offload-defaulted)
* Offloading libgomp plugins there but no CUDA or hsa runtime libraries
* The latter being installed but no device available

Plus -foffload=disable or only enabling an (at runtime) unavailable or
unsupported device type or other issues like CUDA and device present but
an issue with the kernel driver (or similar half-broken states) or ...

[And with remote testing issues related to dg-set-target-env-var and only
few systems supporting offloading, a full test coverage is even harder.]

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

[-- Attachment #2: committed.diff --]
[-- Type: text/x-patch, Size: 1993 bytes --]

commit 01fe115ba7eafebcf97bbac9e157038a003d0c85
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Mon Jun 19 09:52:10 2023 +0200

    libgomp.c/target-51.c: Accept more error-msg variants in dg-output
    
    Depending on the details, the testcase can fail with different but
    related messages; all of the following all could be observed for this
    testcase:
    
      libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading
      libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found
      libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available
    
    Before, the last two were tested for with 'target offload_device' and
    '! offload_device', respectively. Now, all three are accepted by matching
    '.*' already after 'but' and without distinguishing whether the effective
    target is an offload_device or not.
    
    (For completeness, there is a fourth error that follows this pattern:
    'OMP_TARGET_OFFLOAD is set to MANDATORY, but device is finalized'.)
    
    libgomp/
    
            * testsuite/libgomp.c/target-51.c: Accept more error msg variants
            as expected dg-output.
---
 libgomp/testsuite/libgomp.c/target-51.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libgomp/testsuite/libgomp.c/target-51.c b/libgomp/testsuite/libgomp.c/target-51.c
index bbe9ade6e24..db0363bfc14 100644
--- a/libgomp/testsuite/libgomp.c/target-51.c
+++ b/libgomp/testsuite/libgomp.c/target-51.c
@@ -9,8 +9,7 @@
 
 /* See comment in target-50.c/target-50.c for why the output differs.  */
 
-/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available.*" { target { ! offload_device } } } */
-/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found.*" { target offload_device } } */
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but .*" } } */
 
 int
 main ()

Fix DejaGnu directive syntax error in 'libgomp.c/target-51.c' (was: [committed] libgomp.c/target-51.c: Accept more error-msg variants in dg-output (was: Re: [committed] libgomp: Fix OMP_TARGET_OFFLOAD=mandatory))

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2485 bytes --]

Hi!

On 2023-06-19T10:02:58+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> On 16.06.23 22:42, Thomas Schwinge wrote:
>> I see the new tests PASS, but with offloading enabled (nvptx) also see:
>>
>>      PASS: libgomp.c/target-51.c (test for excess errors)
>>      PASS: libgomp.c/target-51.c execution test
>>      [-PASS:-]{+FAIL:+} libgomp.c/target-51.c output pattern test
>>
>> ... due to:
>>
>>      Output was:
>>
>>      libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading
>>
>>      Should match:
>>      .*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device not found.*
>
> Thanks for the report. I can offer yet another wording for the same program – and also
> with nvptx enabled:
>
> libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but device cannot be used for offloading
>
> And I can also offer (which is already in the testcase with "! offload_device"):
>
> libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but only the host device is available
>
> I think I will just match "..., but .*" without distinguishing check_effective_target_* ...
>
> ... which I now did in commit r14-1926-g01fe115ba7eafe (see also attached patch).

Pushed commit de2d3b69eefde005759279d6739d9a0dbd2a05cc
"Fix DejaGnu directive syntax error in 'libgomp.c/target-51.c'",
see attached.


Grüße
 Thomas


> * * *
>
> With offloading, there are simply too many possibilities:
>
> * Not compiled with offloading support - vs. with (ENABLE_OFFLOADING)
> * Support compiled in but either compiler or library support not installed
>    (requires configuring with --enable-offload-defaulted)
> * Offloading libgomp plugins there but no CUDA or hsa runtime libraries
> * The latter being installed but no device available
>
> Plus -foffload=disable or only enabling an (at runtime) unavailable or
> unsupported device type or other issues like CUDA and device present but
> an issue with the kernel driver (or similar half-broken states) or ...
>
> [And with remote testing issues related to dg-set-target-env-var and only
> few systems supporting offloading, a full test coverage is even harder.]
>
> 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-DejaGnu-directive-syntax-error-in-libgomp.c-targ.patch --]
[-- Type: text/x-diff, Size: 1134 bytes --]

From de2d3b69eefde005759279d6739d9a0dbd2a05cc Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 19 Jun 2023 12:20:15 +0200
Subject: [PATCH] Fix DejaGnu directive syntax error in 'libgomp.c/target-51.c'

    ERROR: libgomp.c/target-51.c: unknown dg option: \} for "}"

Fix-up for recent commit 01fe115ba7eafebcf97bbac9e157038a003d0c85
"libgomp.c/target-51.c: Accept more error-msg variants in dg-output".

	libgomp/
	* testsuite/libgomp.c/target-51.c: Fix DejaGnu directive syntax
	error.
---
 libgomp/testsuite/libgomp.c/target-51.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgomp/testsuite/libgomp.c/target-51.c b/libgomp/testsuite/libgomp.c/target-51.c
index db0363bfc14..7ff8122861f 100644
--- a/libgomp/testsuite/libgomp.c/target-51.c
+++ b/libgomp/testsuite/libgomp.c/target-51.c
@@ -9,7 +9,7 @@
 
 /* See comment in target-50.c/target-50.c for why the output differs.  */
 
-/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but .*" } } */
+/* { dg-output ".*libgomp: OMP_TARGET_OFFLOAD is set to MANDATORY, but .*" } */
 
 int
 main ()
-- 
2.34.1