diff mbox series

[committed] OpenACC's tile clause fix for implicit typing (PR93825)

Message ID 39096dbd-a2a4-6fe2-3c43-e97ca72a9cf8@codesourcery.com
State New
Headers show
Series [committed] OpenACC's tile clause fix for implicit typing (PR93825) | expand

Commit Message

Tobias Burnus Feb. 20, 2020, 5:21 p.m. UTC 
I committed it as simple and obvious patch, but
of course I am happy for comment :-)

OpenACC loop directives can have both 'collapse' and 'tile'
clauses. They are resolved as follows:
* gfc_resolve_oacc_blocks is called, which in turn calls
   resolve_oacc_loop_blocks followed by gfc_resolve_blocks,
* Then gfc_resolve_oacc_directive is called, which calls
   resolve_oacc_loop

It turned out that one needs to handle first the checks
in resolve_oacc_loop_blocks for the clauses itself – if one
moved this call after the gfc_resolve_blocks in the
callee gfc_resolve_oacc_blocks it won't work.

On the other hand, if one directly calls resolve_oacc_nested_loops
within resolve_oacc_loop_blocks, the loop data (code->ext.iterator)
is not yet resolved. – That's visible in the included test case as
the variables are still BT_UNKNOWN instead of implicitly being typed
to integer.

Hence, the solution is to keep the clause checks in
resolve_oacc_loop_blocks but move the checking of the loop data
(via resolve_oacc_nested_loops) to resolve_oacc_loop — next to
the handling for collapsed, which also makes sense code-organization
wise.

NOTE: In line with the tree-conversion code, which does ignore "collapse",
this check ignores "collapse" in terms of checking. That only matters if
"n" in collapse(n) is greater than the "n" in "tile" as otherwise all the
check are already done for "tile".

Installed as Rev. r10-6761-g2c52b2884ba10b1c5050fe066bae651680c8ebae

Cheers,

Tobias
diff mbox series

Patch

commit 2c52b2884ba10b1c5050fe066bae651680c8ebae
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Thu Feb 20 18:11:32 2020 +0100

    OpenACC's tile clause fix for implicit typing (PR93825)
    
    2020-02-20  Tobias Burnus  <tobias@codesourcery.com>
    
            PR fortran/93825
            * openmp.c (resolve_oacc_loop_blocks): Move call to
            resolve_oacc_nested_loops from here ...
            (resolve_oacc_loop): ... to here.
    
            PR fortran/93825
            * gfortran.dg/goacc/tile-3.f90: New.
---
 gcc/fortran/ChangeLog                      | 25 ++++++++++++++++---------
 gcc/fortran/openmp.c                       | 15 ++++++++++++---
 gcc/testsuite/ChangeLog                    |  5 +++++
 gcc/testsuite/gfortran.dg/goacc/tile-3.f90 | 13 +++++++++++++
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 7547dccd79a..6795b82b74c 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,10 @@ 
+2020-02-20  Tobias Burnus  <tobias@codesourcery.com>
+
+	PR fortran/93825
+	* openmp.c (resolve_oacc_loop_blocks): Move call to
+	resolve_oacc_nested_loops from here ...
+	(resolve_oacc_loop): ... to here.
+
 2020-02-18  Mark Eggleston  <markeggleston@gcc.gnu.org>
 
 	PR fortran/93714
@@ -29,15 +36,15 @@ 
 
 2020-02-10  Andrew Benson  <abensonca@gmail.com>
 
-        PR fortran/83113
-        * array.c: Do not attempt to set the array spec for a submodule
-        function symbol (as it has already been set in the corresponding
-        module procedure interface).
-        * symbol.c: Do not reject duplicate POINTER, ALLOCATABLE, or
-        DIMENSION attributes in declarations of a submodule function.
-        * gfortran.h: Add a macro that tests for a module procedure in a
-        submodule.
-        * gfortran.dg/pr83113.f90: New test.
+	PR fortran/83113
+	* array.c: Do not attempt to set the array spec for a submodule
+	function symbol (as it has already been set in the corresponding
+	module procedure interface).
+	* symbol.c: Do not reject duplicate POINTER, ALLOCATABLE, or
+	DIMENSION attributes in declarations of a submodule function.
+	* gfortran.h: Add a macro that tests for a module procedure in a
+	submodule.
+	* gfortran.dg/pr83113.f90: New test.
 
 2020-02-03  Julian Brown  <julian@codesourcery.com>
 	    Tobias Burnus  <tobias@codesourcery.com>
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index b885e86c658..35f6b2f4938 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -6192,10 +6192,8 @@  resolve_oacc_loop_blocks (gfc_code *code)
   if (code->ext.omp_clauses->tile_list)
     {
       gfc_expr_list *el;
-      int num = 0;
       for (el = code->ext.omp_clauses->tile_list; el; el = el->next)
 	{
-	  num++;
 	  if (el->expr == NULL)
 	    {
 	      /* NULL expressions are used to represent '*' arguments.
@@ -6213,7 +6211,6 @@  resolve_oacc_loop_blocks (gfc_code *code)
 			   &code->loc);
 	    }
 	}
-      resolve_oacc_nested_loops (code, code->block->next, num, "tiled");
     }
 }
 
@@ -6266,6 +6263,18 @@  resolve_oacc_loop (gfc_code *code)
   do_code = code->block->next;
   collapse = code->ext.omp_clauses->collapse;
 
+  /* Both collapsed and tiled loops are lowered the same way, but are not
+     compatible.  In gfc_trans_omp_do, the tile is prioritized.  */
+  if (code->ext.omp_clauses->tile_list)
+    {
+      int num = 0;
+      gfc_expr_list *el;
+      for (el = code->ext.omp_clauses->tile_list; el; el = el->next)
+	++num;
+      resolve_oacc_nested_loops (code, code->block->next, num, "tiled");
+      return;
+    }
+
   if (collapse <= 0)
     collapse = 1;
   resolve_oacc_nested_loops (code, do_code, collapse, "collapsed");
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a54505ed73e..89acd3f518f 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2020-02-20  Tobias Burnus  <tobias@codesourcery.com>
+
+	PR fortran/93825
+	* gfortran.dg/goacc/tile-3.f90: New.
+
 2020-02-19  Marek Polacek  <polacek@redhat.com>
 
 	PR c++/93169 - wrong-code with a non-constexpr constructor.
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-3.f90 b/gcc/testsuite/gfortran.dg/goacc/tile-3.f90
new file mode 100644
index 00000000000..00b4f1df2d8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-3.f90
@@ -0,0 +1,13 @@ 
+! { dg-do compile }
+!
+! PR fortran/93825
+!
+! Check that implicit typing works
+
+program p
+   !$acc parallel loop tile(2,2)
+   do i = 1, 8
+      do j = 1, 8
+      end do
+   end do
+end