diff mbox series

Fortran: Fixes for OpenMP loop-iter privatization (PRs 95109 + 94690)

Message ID 9dc63789-fd86-d645-bc04-43c746ae6c98@codesourcery.com
State New
Headers show
Series Fortran: Fixes for OpenMP loop-iter privatization (PRs 95109 + 94690) | expand

Commit Message

Tobias Burnus Sept. 8, 2020, 10:22 a.m. UTC
This patch fixes all known issues related to loop-iter privatization.

This patch removes the code added to gfc_resolve_do_iterator in commit
r11-349-gf884bef21cccc05d748fd7869cd641cbb4f6b6bb.

I added it to fix some issues in testsuite/libgomp.fortran/pr66199-*.f90
but it turned out that removing no longer causes fails; on the other hand,
while r11-349 caused target1.f90 to fail, just removing that changes did
not fix it.

Digging deeper, I found that for target1.f90's
   !$omp target teams ...
   !$omp distribute parallel do simd ...
the do-loop's i and j where added to '(target) teams'
instead of the 'distribute parallel do simd', causing
a middle-end ICE → resolve.c and openmp.c part of the patch.

Some testing and looking at dumps additionally showed
that for 'target parallel do simd', all loop variables
where 'shared' as the wrong function was called in trans-openmp.c.

The latter is also the reason that 'combined-if.f90' now has
a higher 'omp simd.*if' count.

Tested on x86-64-gnu-linux.

OK for the trunk and GCC 10?
(As r11-349 was not applied to GCC 10, the
gfc_resolve_do_iterator change is trunk only.)

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Comments

Jakub Jelinek Sept. 9, 2020, 6:54 a.m. UTC | #1
On Tue, Sep 08, 2020 at 12:22:57PM +0200, Tobias Burnus wrote:
> gcc/fortran/ChangeLog:
> 
> 	PR fortran/95109
> 	PR fortran/94690
> 	* resolve.c (gfc_resolve_code): Also call
> 	gfc_resolve_omp_parallel_blocks for 'distribute parallel do (simd)'.
> 	* openmp.c (gfc_resolve_omp_parallel_blocks): Handle it.
> 	(gfc_resolve_do_iterator): Remove special code for SIMD, which is
> 	not needed.
> 	* trans-openmp.c (gfc_trans_omp_target): For TARGET_PARALLEL_DO_SIMD,
> 	call simd not do processing function.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR fortran/95109
> 	PR fortran/94690
> 	* gfortran.dg/gomp/combined-if.f90: Update scan-tree-dump-times for
> 	'omp simd.*if'.
> 	* gfortran.dg/gomp/openmp-simd-5.f90: New test.

LGTM, thanks.
Note for OpenMP 5.0 we'll need also
EXEC_OMP_{,PARALLEL_}MASTER_TASKLOOP{,_SIMD},
EXEC_OMP_PARALLEL_{LOOP,MASTER}
EXEC_OMP_TARGET_{PARALLEL,TEAMS}_LOOP
EXEC_OMP_TEAMS_LOOP
handling for the combined constructs in these.

	Jakub
Tobias Burnus Sept. 9, 2020, 10:19 a.m. UTC | #2
On 9/9/20 8:54 AM, Jakub Jelinek wrote:

> On Tue, Sep 08, 2020 at 12:22:57PM +0200, Tobias Burnus wrote:
>> gcc/testsuite/ChangeLog:
>>
>>      PR fortran/95109
>>      PR fortran/94690
>>      * gfortran.dg/gomp/combined-if.f90: Update scan-tree-dump-times for
>>      'omp simd.*if'.
>>      * gfortran.dg/gomp/openmp-simd-5.f90: New test.

I have applied a follow-up commit for nvptx as the scan times increased
– and are/were different (with -O1 and higher; the testsuite uses "-O".)

> LGTM, thanks.
> Note for OpenMP 5.0 we'll need also
> EXEC_OMP_{,PARALLEL_}MASTER_TASKLOOP{,_SIMD},
> EXEC_OMP_PARALLEL_{LOOP,MASTER}
> EXEC_OMP_TARGET_{PARALLEL,TEAMS}_LOOP
> EXEC_OMP_TEAMS_LOOP
> handling for the combined constructs in these.

Indeed. – 'omp loop' that's one of the larger features missing
from gfortran, which are in already supported in C/C++.

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
diff mbox series

Patch

Fortran: Fixes for OpenMP loop-iter privatization (PRs 95109 + 94690)

This commit also fixes a gfortran.dg/gomp/target1.f90 regression;
target1.f90 tests the resolve.c and openmp.c changes.

gcc/fortran/ChangeLog:

	PR fortran/95109
	PR fortran/94690
	* resolve.c (gfc_resolve_code): Also call
	gfc_resolve_omp_parallel_blocks for 'distribute parallel do (simd)'.
	* openmp.c (gfc_resolve_omp_parallel_blocks): Handle it.
	(gfc_resolve_do_iterator): Remove special code for SIMD, which is
	not needed.
	* trans-openmp.c (gfc_trans_omp_target): For TARGET_PARALLEL_DO_SIMD,
	call simd not do processing function.

gcc/testsuite/ChangeLog:

	PR fortran/95109
	PR fortran/94690
	* gfortran.dg/gomp/combined-if.f90: Update scan-tree-dump-times for
	'omp simd.*if'.
	* gfortran.dg/gomp/openmp-simd-5.f90: New test.

 gcc/fortran/openmp.c                             | 27 ++----------------------
 gcc/fortran/resolve.c                            |  2 ++
 gcc/fortran/trans-openmp.c                       |  8 ++++++-
 gcc/testsuite/gfortran.dg/gomp/combined-if.f90   |  2 +-
 gcc/testsuite/gfortran.dg/gomp/openmp-simd-5.f90 | 24 +++++++++++++++++++++
 5 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index d0e516c472d..1efce33e519 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -5962,6 +5962,8 @@  gfc_resolve_omp_parallel_blocks (gfc_code *code, gfc_namespace *ns)
 
   switch (code->op)
     {
+    case EXEC_OMP_DISTRIBUTE_PARALLEL_DO:
+    case EXEC_OMP_DISTRIBUTE_PARALLEL_DO_SIMD:
     case EXEC_OMP_PARALLEL_DO:
     case EXEC_OMP_PARALLEL_DO_SIMD:
     case EXEC_OMP_TARGET_PARALLEL_DO:
@@ -6047,31 +6049,6 @@  gfc_resolve_do_iterator (gfc_code *code, gfc_symbol *sym, bool add_clause)
   if (omp_current_ctx->sharing_clauses->contains (sym))
     return;
 
-  if (omp_current_ctx->is_openmp && omp_current_ctx->code->block)
-    {
-      /* SIMD is handled differently and, hence, ignored here.  */
-      gfc_code *omp_code = omp_current_ctx->code->block;
-      for ( ; omp_code->next; omp_code = omp_code->next)
-	switch (omp_code->op)
-	  {
-	  case EXEC_OMP_SIMD:
-	  case EXEC_OMP_DO_SIMD:
-	  case EXEC_OMP_PARALLEL_DO_SIMD:
-	  case EXEC_OMP_DISTRIBUTE_SIMD:
-	  case EXEC_OMP_DISTRIBUTE_PARALLEL_DO_SIMD:
-	  case EXEC_OMP_TEAMS_DISTRIBUTE_SIMD:
-	  case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE_SIMD:
-	  case EXEC_OMP_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD:
-	  case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD:
-	  case EXEC_OMP_TARGET_PARALLEL_DO_SIMD:
-	  case EXEC_OMP_TARGET_SIMD:
-	  case EXEC_OMP_TASKLOOP_SIMD:
-	    return;
-	  default:
-	    break;
-	  }
-    }
-
   if (! omp_current_ctx->private_iterators->add (sym) && add_clause)
     {
       gfc_omp_clauses *omp_clauses = omp_current_ctx->code->ext.omp_clauses;
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index ebf89a9b1f5..f4ce49f8432 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -11722,6 +11722,8 @@  gfc_resolve_code (gfc_code *code, gfc_namespace *ns)
 	      omp_workshare_flag = 1;
 	      gfc_resolve_omp_parallel_blocks (code, ns);
 	      break;
+	    case EXEC_OMP_DISTRIBUTE_PARALLEL_DO:
+	    case EXEC_OMP_DISTRIBUTE_PARALLEL_DO_SIMD:
 	    case EXEC_OMP_PARALLEL:
 	    case EXEC_OMP_PARALLEL_DO:
 	    case EXEC_OMP_PARALLEL_DO_SIMD:
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 7d3365fe7e0..0e1da0426b4 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -5591,13 +5591,19 @@  gfc_trans_omp_target (gfc_code *code)
       }
       break;
     case EXEC_OMP_TARGET_PARALLEL_DO:
-    case EXEC_OMP_TARGET_PARALLEL_DO_SIMD:
       stmt = gfc_trans_omp_parallel_do (code, &block, clausesa);
       if (TREE_CODE (stmt) != BIND_EXPR)
 	stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
       else
 	poplevel (0, 0);
       break;
+    case EXEC_OMP_TARGET_PARALLEL_DO_SIMD:
+      stmt = gfc_trans_omp_parallel_do_simd (code, &block, clausesa);
+      if (TREE_CODE (stmt) != BIND_EXPR)
+	stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
+      else
+	poplevel (0, 0);
+      break;
     case EXEC_OMP_TARGET_SIMD:
       stmt = gfc_trans_omp_do (code, EXEC_OMP_SIMD, &block,
 			       &clausesa[GFC_OMP_SPLIT_SIMD], NULL_TREE);
diff --git a/gcc/testsuite/gfortran.dg/gomp/combined-if.f90 b/gcc/testsuite/gfortran.dg/gomp/combined-if.f90
index 0bb6c28b286..d9e4a26ca0c 100644
--- a/gcc/testsuite/gfortran.dg/gomp/combined-if.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/combined-if.f90
@@ -104,6 +104,6 @@  contains
 end module
 
 ! { dg-final { scan-tree-dump-times "(?n)#pragma omp target.* if\\(" 9 "omplower" } }
-! { dg-final { scan-tree-dump-times "(?n)#pragma omp simd.* if\\(" 4 "omplower" { target { ! offload_nvptx } } } }
+! { dg-final { scan-tree-dump-times "(?n)#pragma omp simd.* if\\(" 5 "omplower" { target { ! offload_nvptx } } } }
 ! { dg-final { scan-tree-dump-times "(?n)#pragma omp simd.* if\\(" 7 "omplower" { target { offload_nvptx } } } }
 ! { dg-final { scan-tree-dump-times "(?n)#pragma omp parallel.* if\\(" 6 "omplower" } }
diff --git a/gcc/testsuite/gfortran.dg/gomp/openmp-simd-5.f90 b/gcc/testsuite/gfortran.dg/gomp/openmp-simd-5.f90
new file mode 100644
index 00000000000..b6d4cfa7080
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/openmp-simd-5.f90
@@ -0,0 +1,24 @@ 
+! { dg-additional-options "-fdump-tree-original" }
+!
+! Related:
+!   PR fortran/95109
+!   PR fortran/94690
+!
+implicit none
+integer :: i, j, k, ll
+integer :: a
+!$omp target parallel do simd collapse(1)
+  do i = 1, 5
+    do j = 1, 5
+      do k = 1, 5
+        a = a + 1
+      end do
+      do ll = 1, 5
+        a = a + 1
+      end do
+    end do
+  end do
+!$omp end target parallel do simd
+end
+
+! { dg-final { scan-tree-dump-times "omp simd linear\\(i:1\\) private\\(j\\) private\\(ll\\) private\\(k\\) collapse\\(1\\)" 1 "original" } }