On Thu, Oct 07, 2021 at 09:59:00PM +0800, Chung-Lin Tang wrote: > this patch add support for "strictly-structured blocks" introduced in OpenMP > 5.1, > basically allowing BLOCK constructs to serve as the body for directives: > > !$omp target > block > ... > end block > [!$omp end target] !! end directive is optional > > !$omp parallel > block > ... > end block > ... > !$omp end parallel !! error, considered as not match to above parallel > directive > > The parsing loop in parse_omp_structured_block() has been modified to allow > a BLOCK construct after the first statement has been detected to be ST_BLOCK. > This is done by a hard modification of the state into (the new) > COMP_OMP_STRICTLY_STRUCTURED_BLOCK > after the statement is known (I'm not sure if there's a way to 'peek' the next > statement/token in the Fortran FE, open to suggestions on how to better write > this)
Thanks for working on this. The workshare/parallel workshare case is unclear, I've filed https://github.com/OpenMP/spec/issues/3153 for it. Either don't allow block if workshare_stmts_only for now until that is clarified, or if we do, we need to make sure that it does the expected thing, does that gfc_trans_block_construct call ensure it? Then we have the https://github.com/OpenMP/spec/issues/3154 issue Tobias discovered, if that issue is resolved to end always applying to the directive before the block statement, I think your patch handles it that way but we want testsuite coverage for some of those cases. For the testcases, I think best would be to split it into two, one that contains only what we want to accept and another one with dg-errors in it. I don't think the patch does the right thing for sections/parallel sections. That is (at least in 5.1) defined as: !$omp sections clauses... [!$omp section] structured-block-sequence [!$omp section structured-block-sequence] ... !$omp end sections (and similarly for parallel sections). I believe your patch properly disallows: !$omp sections block ... !$omp section ... end block !$omp end sections - block itself is allowed, e.g. !$omp sections block a=1 b=2 end block !$omp end sections with the meaning that the block is after the first implied !$omp section and there is nothing else. But does the patch actually check that !$omp sections block ... end block c=1 !$omp end sections or !$omp sections !$omp section block ... end block c=1 !$omp section d=1 !$omp end sections is invalid? Though, not sure if that was the intended effect, in OpenMP 5.0 that used to be fine. But then the other changes are backwards incompatible too, !$omp parallel block ... end block c=1 !$omp end parallel used to be valid but no longer is. structured-block-sequence is fortran defined as structured-block and structured-block is defined as either loosely-structured-block or strictly-structured-block, so for sections in between each !$omp section should be either anything not starting with block, or, if it starts with block, after end block there should be immediately !$omp section or !$omp end {,parallel }sections. Another thing is scan, the wording is similar and newly !$omp do reduction(+,inscan:a) do i=1,10 block ... end block x=1 !$omp scan block ... end block x=2 end do is invalid. > @@ -5538,6 +5539,32 @@ parse_omp_structured_block (gfc_statement omp_st, bool > workshare_stmts_only) > gcc_unreachable (); > } > > + bool block_construct = false; > + gfc_namespace* my_ns = NULL; > + gfc_namespace* my_parent = NULL; The usual coding conventions put * before variable name instead of after it (except for libstdc++). > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-1.f90 > @@ -0,0 +1,295 @@ > +! { dg-do compile } > +! { dg-options "-fopenmp" } > + > +program main > + integer :: x > + > + !$omp parallel > + block > + x = x + 1 > + end block > + > + !$omp parallel > + block > + x = x + 1 > + end block > + !$omp end parallel > + > + !$omp parallel > + block > + x = x + 1 > + end block > + x = x + 1 > + !$omp end parallel ! { dg-error "Unexpected !.OMP END PARALLEL statement" } Other than the splitting into non-dg-error stuff in one testcase and dg-error in another one, I think we want (probably in yet another pair of testcases) test what we are not required to do but with your patch we actually implement, in particular that !$omp master behaves the same way. > + !$omp ordered > + block > + x = x + 1 > + end block > + > + !$omp ordered > + block > + x = x + 1 > + end block > + !$omp end ordered > + > + !$omp ordered > + block > + x = x + 1 > + end block > + x = x + 1 > + !$omp end ordered ! { dg-error "Unexpected !.OMP END ORDERED statement" } I believe these 3 can't be done in the program, would either need to be wrapped in some !$omp do with ordered clause or should go each ordered into its own orphaned subroutine, because ordered region must bind to a worksharing-loop with ordered clause. I think wrapping it inside of !$omp do ordered is easier. Jakub
