Re: [Patch][v5] OpenMP: Move omp requires checks to libgomp

[Jakub Jelinek via Gcc-patches]

On Fri, Jul 01, 2022 at 11:08:16PM +0200, Tobias Burnus wrote:
>    gomp_mutex_lock (&register_lock);
>  
> +  if (omp_requires_mask && omp_requires_mask != omp_req)

I'd use if (omp_req && omp_requires_mask && omp_requires_mask != omp_req)
e.g. for the case of mixing GCC <= 12 compiled code with GCC 13,
treat omp_req 0 as "don't know" while GOMP_REQUIRES_TARGET_USED
as "known and no requires uni*/rev* specified".

> +    {
> +      char buf1[sizeof ("unified_address, unified_shared_memory, "
> +                     "reverse_offload")];
> +      char buf2[sizeof ("unified_address, unified_shared_memory, "
> +                     "reverse_offload")];
> +      gomp_requires_to_name (buf2, sizeof (buf2),
> +                          omp_req != GOMP_REQUIRES_TARGET_USED
> +                          ? omp_req : omp_requires_mask);
> +      if (omp_req != GOMP_REQUIRES_TARGET_USED
> +       && omp_requires_mask != GOMP_REQUIRES_TARGET_USED)
> +     {
> +       gomp_requires_to_name (buf1, sizeof (buf1), omp_requires_mask);
> +       gomp_fatal ("OpenMP 'requires' directive with non-identical clauses "
> +                   "in multiple compilation units: '%s' vs. '%s'",
> +                   buf1, buf2);
> +     }
> +      else
> +     gomp_fatal ("OpenMP 'requires' directive with '%s' specified only in "
> +                 "some compilation units", buf2);
> +    }
> +  omp_requires_mask = omp_req;
> +
>    /* Load image to all initialized devices.  */
>    for (i = 0; i < num_devices; i++)
>      {
> @@ -4125,8 +4173,30 @@ gomp_target_init (void)
>  
>       if (gomp_load_plugin_for_device (&current_device, plugin_name))
>         {
> -         new_num_devs = current_device.get_num_devices_func ();
> -         if (new_num_devs >= 1)
> +         int omp_req = omp_requires_mask & ~GOMP_REQUIRES_TARGET_USED;
> +         new_num_devs = current_device.get_num_devices_func (omp_req);
> +         if (new_num_devs < 0)

Can this be if (gomp_debug && new_num_devs < 0) - i.e. be verbose only
when the user asks for it?

> +           {
> +             bool found = false;
> +             int type = current_device.get_type_func ();
> +             for (int img = 0; img < num_offload_images; img++)
> +               if (type == offload_images[img].type)
> +                 found = true;
> +             if (found)
> +               {
> +                 char buf[sizeof ("unified_address, unified_shared_memory, "
> +                                  "reverse_offload")];
> +                 gomp_requires_to_name (buf, sizeof (buf), omp_req);
> +                 char *name = (char *) malloc (cur_len + 1);
> +                 memcpy (name, cur, cur_len);
> +                 name[cur_len] = '\0';
> +                 GOMP_PLUGIN_error ("note: %s devices present but 'omp "
> +                                    "requires %s' cannot be fulfilled",
> +                                    name, buf);
> +                 free (name);
> +               }
> +           }
> +         else if (new_num_devs >= 1)
>             {
>               /* Augment DEVICES and NUM_DEVICES.  */
>  

Otherwise LGTM.

        Jakub