diff mbox series

GCC11 - Fortran: combined directives - order(concurrent) not on distribute (was: Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier)

Message ID d909388d-94e6-6804-d7ad-24bf7086b9e6@codesourcery.com
State New
Headers show
Series GCC11 - Fortran: combined directives - order(concurrent) not on distribute (was: Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier) | expand

Commit Message

Tobias Burnus Sept. 20, 2021, 3:01 p.m. UTC
On 20.09.21 11:55, Jakub Jelinek via Fortran wrote:
> So the FE was splitting the order clause to distribute already before,
> perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
> support.
> The difference is e.g. the distribute parallel do order(concurrent) copyin(thr)
> case which used to be ok in 5.0 and is not in 5.1.

Well, if I try with GCC 11:

void f(int *a)
{
int  i;
static int thr;
#pragma omp threadprivate (thr)
#pragma omp distribute parallel for order(concurrent) copyin(thr)
   for (i = 0; i < 10; ++i)
    {
     thr = 5;
     a[i] = thr;
    }
}

I get with gcc (+ gfortran):
   error: threadprivate variable ‘thr’ used in a region with ‘order(concurrent)’ clause
I might have misunderstood the example.

  * * *

In any case, for GCC 11, I have now fixed the splitting and added a testcase which
relies on -fdump-tree-original scanning and does not use threadprivate.

OK for GCC 11, only?

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

Comments

Jakub Jelinek Sept. 20, 2021, 3:13 p.m. UTC | #1
On Mon, Sep 20, 2021 at 05:01:32PM +0200, Tobias Burnus wrote:
> On 20.09.21 11:55, Jakub Jelinek via Fortran wrote:
> > So the FE was splitting the order clause to distribute already before,
> > perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
> > support.
> > The difference is e.g. the distribute parallel do order(concurrent) copyin(thr)
> > case which used to be ok in 5.0 and is not in 5.1.
> 
> Well, if I try with GCC 11:
> 
> void f(int *a)
> {
> int  i;
> static int thr;
> #pragma omp threadprivate (thr)
> #pragma omp distribute parallel for order(concurrent) copyin(thr)
>   for (i = 0; i < 10; ++i)
>    {
>     thr = 5;
>     a[i] = thr;
>    }
> }
> 
> I get with gcc (+ gfortran):
>   error: threadprivate variable ‘thr’ used in a region with ‘order(concurrent)’ clause
> I might have misunderstood the example.

Sure, even before you couldn't use it in the region body, because
order(concurrent) was split to worksharing-loop.
The testcase that used to be accepted and is now rejected is e.g.

int thr;
#pragma omp threadprivate (thr)

void
foo (void)
{
  int i;
#pragma omp distribute parallel for order(concurrent) copyin(thr)
  for (i = 0; i < 10; ++i)
   ;
}

While copyin without actually using the threadprivate var in the body
might look weird, in some cases it might be useful if the threadprivate
variable is used in some following parallel region.

> OK for GCC 11, only?
> 
> 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

> GCC11 - Fortran: combined directives - order(concurrent) not on distribute
> 
> While OpenMP 5.1 and GCC 12 permits 'order(concurrent)' on distribute,
> OpenMP 5.0 and GCC 11 don't. This patch for GCC 11 ensures the clause also
> does not end up on 'distribute' when splitting combined directives.
> 
> gcc/fortran/ChangeLog:
> 
> 	* trans-openmp.c (gfc_split_omp_clauses): Don't put 'order(concurrent)'
> 	on 'distribute' for combined directives, matching OpenMP 5.0
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gfortran.dg/gomp/distribute-order-concurrent.f90: New test.

Ok, thanks.

	Jakub
diff mbox series

Patch

GCC11 - Fortran: combined directives - order(concurrent) not on distribute

While OpenMP 5.1 and GCC 12 permits 'order(concurrent)' on distribute,
OpenMP 5.0 and GCC 11 don't. This patch for GCC 11 ensures the clause also
does not end up on 'distribute' when splitting combined directives.

gcc/fortran/ChangeLog:

	* trans-openmp.c (gfc_split_omp_clauses): Don't put 'order(concurrent)'
	on 'distribute' for combined directives, matching OpenMP 5.0

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/distribute-order-concurrent.f90: New test.

 gcc/fortran/trans-openmp.c                         |  2 --
 .../gomp/distribute-order-concurrent.f90           | 25 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 7e931bf4bc7..973d916b4a2 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -5176,8 +5176,6 @@  gfc_split_omp_clauses (gfc_code *code,
 	  /* Duplicate collapse.  */
 	  clausesa[GFC_OMP_SPLIT_DISTRIBUTE].collapse
 	    = code->ext.omp_clauses->collapse;
-	  clausesa[GFC_OMP_SPLIT_DISTRIBUTE].order_concurrent
-	    = code->ext.omp_clauses->order_concurrent;
 	}
       if (mask & GFC_OMP_MASK_PARALLEL)
 	{
diff --git a/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90 b/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90
new file mode 100644
index 00000000000..9597d913684
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90
@@ -0,0 +1,25 @@ 
+! { dg-additional-options "-fdump-tree-original" }
+!
+! In OpenMP 5.0, 'order(concurrent)' does not apply to distribute
+! Ensure that it is rejected in GCC 11.
+! 
+! Note: OpenMP 5.1 allows it; the GCC 12 testcase for it is gfortran.dg/gomp/order-5.f90
+
+subroutine f(a)
+implicit none
+integer :: i, thr
+!save :: thr
+integer :: a(:)
+
+!$omp distribute parallel do order(concurrent) private(thr)
+  do i = 1, 10
+    thr = 5
+    a(i) = thr
+  end do
+!$omp end distribute parallel do
+end
+
+! { dg-final { scan-tree-dump-not "omp distribute\[^\n\r]*order" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp distribute\[\n\r\]" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp parallel private\\(thr\\)" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp for nowait order\\(concurrent\\)" "original" } }