From: Thomas Schwinge <thomas@codesourcery.com>
To: Tobias Burnus <tobias@codesourcery.com>
Cc: <gcc-patches@gcc.gnu.org>
Subject: Re: [committed] libgomp: Fix OMP_TARGET_OFFLOAD=mandatory
Date: Fri, 16 Jun 2023 22:42:35 +0200 [thread overview]
Message-ID: <87cz1vf8as.fsf@euler.schwinge.homeip.net> (raw)
In-Reply-To: <91bb9136-f8a4-e516-3f42-ed6d66dc8ce0@codesourcery.com>
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
next prev parent reply other threads:[~2023-06-16 20:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-13 18:44 [patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory Tobias Burnus
2023-06-14 8:09 ` Fix typo in 'libgomp.c/target-51.c' (was: [patch] OpenMP: Set default-device-var with OMP_TARGET_OFFLOAD=mandatory) Thomas Schwinge
2023-06-14 9:42 ` Tobias Burnus
2023-06-14 10:58 ` 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
2023-06-16 15:57 ` [committed] libgomp: Fix OMP_TARGET_OFFLOAD=mandatory Tobias Burnus
2023-06-16 20:42 ` Thomas Schwinge [this message]
2023-06-19 8:02 ` [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
2023-06-19 10:24 ` 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87cz1vf8as.fsf@euler.schwinge.homeip.net \
--to=thomas@codesourcery.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=tobias@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).