[Patch] OpenMP/Fortran: Permit end-clause on directive
Tobias Burnus
tobias@codesourcery.com
Sun Nov 27 17:38:41 GMT 2022
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: end-decl-v3.diff
Type: text/x-patch
Size: 38550 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/fortran/attachments/20221127/3adc58da/attachment-0001.bin>
More information about the Fortran
mailing list