Re: [PATCH] OpenMP: Duplicate checking for map clauses in Fortran (PR107214)

[Tobias Burnus <tobias@codesourcery.com>]

Hi Julian,

I had a first quick lock at this patch, I should have a closer look
later. However, I stumbled over the following:

On 20.10.22 18:14, Julian Brown wrote:
> typedef struct gfc_symbol
> {
> ...
>    struct gfc_symbol *old_symbol;
>
>    unsigned mark:1, comp_mark:1, data_mark:1, dev_mark:1, gen_mark:1;
>    unsigned reduc_mark:1, gfc_new:1;
>
>    struct gfc_symbol *tlink;
>
>    unsigned equiv_built:1;
>    ...
I know that this was the case before, but can you move the mark:1 etc.
after 'tlink'? In that case all bitfields are grouped together. If I
have not miscounted, we have currently 7 bits before and 9 bits after
'tlink' and grouping them together reduced pointless padding.

* * *
> +      else if (n->sym->mark)
> +     gfc_error ("Symbol %qs present on both data and map clauses "
> +                "at %L", n->sym->name, &n->where);

I wonder whether that also rejects the following – which seems to be
valid. The 'map' goes to 'target' and the 'firstprivate' to 'parallel',
cf. OpenMP 5.2, "17.2 Clauses on Combined and Composite Constructs",
[340:3-4 & 12-14]. (BTW: While some fixes went into 5.1 regarding this section,
a likewise wording is already in 5.0.)

(Testing showed: it give an ICE without the patch and an error with.)

module m
   integer :: a = 1
end module m

module m2
contains
subroutine bar()
   use m
   !$omp declare target
   a = a + 5
end subroutine bar
end

program p
   use m
   !$omp target parallel do map(a) firstprivate(a)
     do i = 1, 1
        a = 7
       call bar()
        if (a /= 7) error stop 1
        a = a + 8

    end do
   if (a /= 6) error stop
end

  * * *

The ICE seems to be because gcc/fortran/trans-openmp.cc's gfc_split_omp_clauses
mishandles this as the dump shows the following:

   #pragma omp target firstprivate(a) map(tofrom:a)
         #pragma omp parallel firstprivate(a)

  * * *

In contrast, for the C testcase:

void foo(int x) {
#pragma omp target parallel for simd map(x) firstprivate(x)
for (int k = 0; k < 1; ++k)
   x = 1;
}

the dump is as follows, which seems to be sensible:

   #pragma omp target map(tofrom:x)
         #pragma omp parallel firstprivate(x)
               #pragma omp for nowait
                     #pragma omp simd

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