Updated patch – taking the comments below into account – and the remark
by Harald, second by Jakub. Namely:

I have now split the pre-existing nowait-2.f90 into nowait-2.f90 (with
only valid usage) and nowait-4.f90 (with the dg-error tests). In the
previous version of the patch, nowait-4.f90 was a variant of
nowait-2.f90 that used 'nowait' on the directive line. - And Harald
suggested to split the latter, which I now did – into nowait-{5,6}.f90.

Cf. Harald's email at
https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600539.html and
two emails by Jakub ("Otherwise LGTM"), first at
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601304.html +
the next email in the thread.

I intent to commit the attached patch tomorrow, unless there are further
comments.

Thanks for the reviews (and I know that the follow up is very belated)!

Tobias


On 08.09.22 17:21, Jakub Jelinek via Fortran wrote:
> On Fri, Aug 26, 2022 at 08:21:26PM +0200, Tobias Burnus wrote:
>> I did run into some issues related to this; those turned out to be
>> unrelated, but I end ended up implementing this feature.
>>
>> Side remark: 'omp parallel workshare' seems to actually permit 'nowait'
>> now, but I guess that's an unintended change due to the
>> syntax-representation change. Hence, it is now tracked as Spec Issue
>> 3338 and I do not permit it.
>>
>> OK for mainline?
>>
>> 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
>> OpenMP/Fortran: Permit end-clause on directive
>>
>> gcc/fortran/ChangeLog:
>>
>>      * openmp.cc (OMP_DO_CLAUSES, OMP_SCOPE_CLAUSES,
>>      OMP_SECTIONS_CLAUSES, OMP_SINGLE_CLAUSES): Add 'nowait'.
> This doesn't describe what the patch actually does, Add 'nowait'.
> is only true for the first 3, for OMP_SINGLE_CLAUSES IMHO you
> want a separate
>       (OMP_SINGLE_CLAUSES): Add 'nowait' and 'copyprivate'.
> entry.
>
>> @@ -3855,7 +3857,7 @@ cleanup:
>>      | OMP_CLAUSE_ORDER | OMP_CLAUSE_ALLOCATE)
>>   #define OMP_SINGLE_CLAUSES \
>>     (omp_mask (OMP_CLAUSE_PRIVATE) | OMP_CLAUSE_FIRSTPRIVATE         \
>> -   | OMP_CLAUSE_ALLOCATE)
>> +   | OMP_CLAUSE_ALLOCATE | OMP_CLAUSE_NOWAIT | OMP_CLAUSE_COPYPRIVATE)
>>   #define OMP_ORDERED_CLAUSES \
>>     (omp_mask (OMP_CLAUSE_THREADS) | OMP_CLAUSE_SIMD)
>>   #define OMP_DECLARE_TARGET_CLAUSES \
>> @@ -5909,13 +5915,11 @@ gfc_match_omp_teams_distribute_simd (void)
>>   match
>>   gfc_match_omp_workshare (void)
>>   {
>> -  if (gfc_match_omp_eos () != MATCH_YES)
>> -    {
>> -      gfc_error ("Unexpected junk after $OMP WORKSHARE statement at %C");
>> -      return MATCH_ERROR;
>> -    }
>> +  gfc_omp_clauses *c;
>> +  if (gfc_match_omp_clauses (&c, omp_mask (OMP_CLAUSE_NOWAIT)) != MATCH_YES)
>> +    return MATCH_ERROR;
>>     new_st.op = EXEC_OMP_WORKSHARE;
>> -  new_st.ext.omp_clauses = gfc_get_omp_clauses ();
>> +  new_st.ext.omp_clauses = c;
>>     return MATCH_YES;
>>   }
> I think it would be better to introduce OMP_WORKSHARE_CLAUSES and use
> it in both gfc_match_omp_workshare and just use
>    return match_omp (EXEC_OMP_WORKSHARE, OMP_WORKSHARE_CLAUSES);
> ?
>
>> @@ -6954,6 +6952,9 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
>>            }
>>          break;
>>        case OMP_LIST_COPYPRIVATE:
>> +        if (omp_clauses->nowait)
>> +          gfc_error ("NOWAIT clause must not be be used with COPYPRIVATE "
> s/be be/be/
>> +                     "clause at %L", &n->where);
>>          for (; n != NULL; n = n->next)
>>            {
>>              if (n->sym->as && n->sym->as->type == AS_ASSUMED_SIZE)
>> @@ -5284,7 +5285,13 @@ parse_omp_do (gfc_statement omp_st)
>>     if (st == omp_end_st)
>>       {
>>         if (new_st.op == EXEC_OMP_END_NOWAIT)
>> -    cp->ext.omp_clauses->nowait |= new_st.ext.omp_bool;
>> +    {
>> +      if (cp->ext.omp_clauses->nowait && new_st.ext.omp_bool)
>> +        gfc_error_now ("Duplicated NOWAIT clause on %s and %s at %C",
>> +                       gfc_ascii_statement (omp_st),
>> +                       gfc_ascii_statement (omp_end_st));
>> +      cp->ext.omp_clauses->nowait |= new_st.ext.omp_bool;
>> +    }
>>         else
>>      gcc_assert (new_st.op == EXEC_NOP);
>>         gfc_clear_new_st ();
> Not sure if the standard is clear enough that unique clauses can't be
> repeated on both directive and corresponding end directive.  But let's
> assume that is the case.
>
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/gomp/copyprivate-2.f90
>> @@ -0,0 +1,69 @@
>> +  FUNCTION t()
>> +    INTEGER :: a, b, t
>> +    a = 0
>> +    t = b
>> +    b = 0
>> +    !$OMP PARALLEL REDUCTION(+:b)
>> +      !$OMP SINGLE COPYPRIVATE (b) NOWAIT  ! { dg-error "NOWAIT clause must not be be used with COPYPRIVATE clause" }
> Here too (several times).
>
>> +        !$OMP ATOMIC WRITE
>> +        b = 6
>> +      !$OMP END SINGLE
>> +    !$OMP END PARALLEL
>> +    t = t + b
>> +  END FUNCTION
>> +
>> +  FUNCTION t2()
>> +    INTEGER :: a, b, t2
>> +    a = 0
>> +    t2 = b
>> +    b = 0
>> +    !$OMP PARALLEL REDUCTION(+:b)
>> +      !$OMP SINGLE NOWAIT COPYPRIVATE (b)  ! { dg-error "NOWAIT clause must not be be used with COPYPRIVATE clause" }
>> +        !$OMP ATOMIC WRITE
>> +        b = 6
>> +      !$OMP END SINGLE
>> +    !$OMP END PARALLEL
>> +    t2 = t2 + b
>> +  END FUNCTION
>> +
>> +  FUNCTION t3()
>> +    INTEGER :: a, b, t3
>> +    a = 0
>> +    t3 = b
>> +    b = 0
>> +    !$OMP PARALLEL REDUCTION(+:b)
>> +      !$OMP SINGLE COPYPRIVATE (b)  ! { dg-error "NOWAIT clause must not be be used with COPYPRIVATE clause" }
>> +        !$OMP ATOMIC WRITE
>> +        b = 6
>> +      !$OMP END SINGLE NOWAIT
>> +    !$OMP END PARALLEL
>> +    t3 = t3 + b
>> +  END FUNCTION
>> +
>> +  FUNCTION t4()
>> +    INTEGER :: a, b, t4
>> +    a = 0
>> +    t4 = b
>> +    b = 0
>> +    !$OMP PARALLEL REDUCTION(+:b)
>> +      !$OMP SINGLE
>> +        !$OMP ATOMIC WRITE
>> +        b = 6
>> +      !$OMP END SINGLE NOWAIT COPYPRIVATE (b)  ! { dg-error "NOWAIT clause must not be be used with COPYPRIVATE clause" }
>> +    !$OMP END PARALLEL
>> +    t4 = t4 + b
>> +  END FUNCTION
>> +
>> +  FUNCTION t5()
>> +    INTEGER :: a, b, t5
>> +    a = 0
>> +    t5 = b
>> +    b = 0
>> +    !$OMP PARALLEL REDUCTION(+:b)
>> +      !$OMP SINGLE
>> +        !$OMP ATOMIC WRITE
>> +        b = 6
>> +      !$OMP END SINGLE COPYPRIVATE (b) NOWAIT  ! { dg-error "NOWAIT clause must not be be used with COPYPRIVATE clause" }
>> +    !$OMP END PARALLEL
>> +    t5 = t5 + b
>> +  END FUNCTION
> I think this lacks a test for !$OMP SINGLE NOWAIT and !$OMP END SINGLE COPYPRIVATE (b).
>
> Also, shouldn't we have test coverage for !$OMP SINGLE COPYPRIVATE (b) with !$OMP END SINGLE COPYPRIVATE (b)
> (that we detect multiple copyprivate clauses for the same variable even that
> way)?
>
> Otherwise LGTM.
>
> Note, for combined constructs with target seems we were already implementing
> this, because the 5.1 wording allows nowait only on !$omp target and not on
> !$omp end target, right?
>
>       Jakub
>
-----------------
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