public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [gomp4] update gfortran's tile clause error handling
@ 2016-10-03 14:07 Cesar Philippidis
  2016-10-03 14:35 ` Nathan Sidwell
  2016-11-10 10:47 ` [Patch 4/5] OpenACC tile clause support, Fortran front-end parts Chung-Lin Tang
  0 siblings, 2 replies; 16+ messages in thread
From: Cesar Philippidis @ 2016-10-03 14:07 UTC (permalink / raw)
  To: gcc-patches, Fortran List, Nathan Sidwell

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

This patch updates the fortran FE to generate errors, rather than
warnings, for non-positive integer tile clause arguments. I noticed this
problem when I ported over the C/C++ compile time test cases to fortran.
In addition to the two new test files, a couple of other existing tests
needed to be updated to accommodate this new behavior. I've applied it
to gomp-4_0-branch.

Nathan, I haven't looked too deeply into your tile changes yet. Do you
know of the fortran FE is doing anything wrong? I haven't checked if
it's lowering the tile clause in the proper format yet.

Cesar

[-- Attachment #2: gomp4-fortran-tile.diff --]
[-- Type: text/x-patch, Size: 13456 bytes --]

2016-10-03  Cesar Philippidis  <cesar@codesourcery.com>

	gcc/fortran/
	* openmp.c (resolve_oacc_positive_int_expr):Promote the
          warning to an error.

	gcc/testsuite/
	* gfortran.dg/goacc/loop-2.f95: Change expected tile clause
	warnings to errors. 
	* gfortran.dg/goacc/loop-5.f95: Likewise.
	* gfortran.dg/goacc/sie.f95: Likewise.
	* gfortran.dg/goacc/tile-1.f90: New test.
	* gfortran.dg/goacc/tile-2.f90: New test.


diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 92b9afe..399b5d1 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -3266,8 +3266,8 @@ resolve_oacc_positive_int_expr (gfc_expr *expr, const char *clause)
   resolve_oacc_scalar_int_expr (expr, clause);
   if (expr->expr_type == EXPR_CONSTANT && expr->ts.type == BT_INTEGER
       && mpz_sgn(expr->value.integer) <= 0)
-    gfc_warning (0, "INTEGER expression of %s clause at %L must be positive",
-		     clause, &expr->where);
+    gfc_error ("INTEGER expression of %s clause at %L must be positive",
+	       clause, &expr->where);
 }
 
 /* Emits error when symbol is pointer, cray pointer or cray pointee
diff --git a/gcc/testsuite/gfortran.dg/goacc/loop-2.f95 b/gcc/testsuite/gfortran.dg/goacc/loop-2.f95
index 0c902b2..d4c6273 100644
--- a/gcc/testsuite/gfortran.dg/goacc/loop-2.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/loop-2.f95
@@ -143,7 +143,7 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
+    !$acc loop tile(-1) ! { dg-error "must be positive" }
     do i = 1,10
     enddo
     !$acc loop tile(i) ! { dg-error "constant expression" }
@@ -307,7 +307,7 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
+    !$acc loop tile(-1) ! { dg-error "must be positive" }
     do i = 1,10
     enddo
     !$acc loop tile(i) ! { dg-error "constant expression" }
@@ -460,7 +460,7 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc kernels loop tile(-1) ! { dg-warning "must be positive" }
+  !$acc kernels loop tile(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc kernels loop tile(i) ! { dg-error "constant expression" }
@@ -612,7 +612,7 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc parallel loop tile(-1) ! { dg-warning "must be positive" }
+  !$acc parallel loop tile(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc parallel loop tile(i) ! { dg-error "constant expression" }
diff --git a/gcc/testsuite/gfortran.dg/goacc/loop-5.f95 b/gcc/testsuite/gfortran.dg/goacc/loop-5.f95
index d059cf7..fe137d5 100644
--- a/gcc/testsuite/gfortran.dg/goacc/loop-5.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/loop-5.f95
@@ -93,9 +93,6 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
-    do i = 1,10
-    enddo
     !$acc loop vector tile(*)
     DO i = 1,10
     ENDDO
@@ -129,9 +126,6 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
-    do i = 1,10
-    enddo
     !$acc loop vector tile(*)
     DO i = 1,10
     ENDDO
@@ -242,9 +236,6 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc kernels loop tile(-1) ! { dg-warning "must be positive" }
-  do i = 1,10
-  enddo
   !$acc kernels loop vector tile(*)
   DO i = 1,10
   ENDDO
@@ -333,9 +324,6 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc parallel loop tile(-1) ! { dg-warning "must be positive" }
-  do i = 1,10
-  enddo
   !$acc parallel loop vector tile(*)
   DO i = 1,10
   ENDDO
diff --git a/gcc/testsuite/gfortran.dg/goacc/sie.f95 b/gcc/testsuite/gfortran.dg/goacc/sie.f95
index 2d66026..b4dd9ed 100644
--- a/gcc/testsuite/gfortran.dg/goacc/sie.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/sie.f95
@@ -78,10 +78,10 @@ program test
   !$acc parallel num_gangs(i+1)
   !$acc end parallel
 
-  !$acc parallel num_gangs(-1) ! { dg-warning "must be positive" }
+  !$acc parallel num_gangs(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel num_gangs(0) ! { dg-warning "must be positive" }
+  !$acc parallel num_gangs(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel num_gangs() ! { dg-error "Invalid character in name" }
@@ -107,10 +107,10 @@ program test
   !$acc parallel num_workers(i+1)
   !$acc end parallel
 
-  !$acc parallel num_workers(-1) ! { dg-warning "must be positive" }
+  !$acc parallel num_workers(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel num_workers(0) ! { dg-warning "must be positive" }
+  !$acc parallel num_workers(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel num_workers() ! { dg-error "Invalid character in name" }
@@ -136,10 +136,10 @@ program test
   !$acc parallel vector_length(i+1)
   !$acc end parallel
 
-  !$acc parallel vector_length(-1) ! { dg-warning "must be positive" }
+  !$acc parallel vector_length(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel vector_length(0) ! { dg-warning "must be positive" }
+  !$acc parallel vector_length(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel vector_length() ! { dg-error "Invalid character in name" }
@@ -166,10 +166,10 @@ program test
   !$acc loop gang(i+1)
   do i = 1,10
   enddo
-  !$acc loop gang(-1) ! { dg-warning "must be positive" }
+  !$acc loop gang(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop gang(0) ! { dg-warning "must be positive" }
+  !$acc loop gang(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop gang() ! { dg-error "Invalid character in name" }
@@ -198,10 +198,10 @@ program test
   !$acc loop worker(i+1)
   do i = 1,10
   enddo
-  !$acc loop worker(-1) ! { dg-warning "must be positive" }
+  !$acc loop worker(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop worker(0) ! { dg-warning "must be positive" }
+  !$acc loop worker(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop worker() ! { dg-error "Invalid character in name" }
@@ -230,10 +230,10 @@ program test
   !$acc loop vector(i+1)
   do i = 1,10
   enddo
-  !$acc loop vector(-1) ! { dg-warning "must be positive" }
+  !$acc loop vector(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop vector(0) ! { dg-warning "must be positive" }
+  !$acc loop vector(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop vector() ! { dg-error "Invalid character in name" }
@@ -249,4 +249,4 @@ program test
   do i = 1,10
   enddo
 
-end program test
\ No newline at end of file
+end program test
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-1.f90 b/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
new file mode 100644
index 0000000..967a7c3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
@@ -0,0 +1,315 @@
+subroutine parloop
+  integer, parameter :: n = 100
+  integer i, j, k, a
+
+  !$acc parallel loop tile(10)
+  do i = 1, n
+  end do
+  
+  !$acc parallel loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(10, *)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+ 
+  !$acc parallel loop tile(10, *, i) ! { dg-error "" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(,1) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(,,) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(1.1) ! { dg-error "requires a scalar INTEGER" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(-3) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(10, -3) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(-100, 10, 5) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(10, .true.) ! { dg-error "requires a scalar" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(1, a) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(a, 1) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+end subroutine parloop
+
+subroutine par
+  integer, parameter :: n = 100
+  integer i, j, k
+
+  !$acc parallel
+  !$acc loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(1)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop tile(-2) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(i) ! { dg-error "constant expression" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 2, 1)
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(2, 2)
+  do i = 1, n
+     do j = i+1, n, j ! { dg-error "rectangular iteration space" }
+     end do
+  end do
+
+  !$acc loop vector tile(*)
+  do i = 1, n
+  end do
+  
+  !$acc loop worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang worker tile(*)
+  do i = 1, n
+  end do
+  !$acc end parallel
+end subroutine par
+
+subroutine kern
+  integer, parameter :: n = 100
+  integer i, j, k
+
+  !$acc kernels
+  !$acc loop tile  ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(1)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop tile(-2) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(i) ! { dg-error "constant expression" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 2, 1)
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(2, 2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop vector tile(*)
+  do i = 1, n
+  end do
+  
+  !$acc loop worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang worker tile(*)
+  do i = 1, n
+  end do
+  !$acc end kernels
+end subroutine kern
+
+subroutine kernsloop
+  integer, parameter :: n = 100
+  integer i, j, k, a
+
+  !$acc kernels loop tile(10)
+  do i = 1, n
+  end do
+  
+  !$acc kernels loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(10, *)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+ 
+  !$acc kernels loop tile(10, *, i) ! { dg-error "" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc kernels loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(,1) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(,,) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(1.1) ! { dg-error "requires a scalar INTEGER" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(-3) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(10, -3) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(-100, 10, 5) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc kernels loop tile(10, .true.) ! { dg-error "requires a scalar" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(1, a) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(a, 1) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+end subroutine kernsloop
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-2.f90 b/gcc/testsuite/gfortran.dg/goacc/tile-2.f90
new file mode 100644
index 0000000..c567543
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-2.f90
@@ -0,0 +1,21 @@
+subroutine par
+  integer ix, jx
+
+  !$acc parallel
+  !$acc loop tile (*,*) ! { dg-error "not enough DO loops for tiled" }
+  do ix = 1, 30
+  end do
+
+  !$acc loop tile (*,*)
+  do ix = 1, 30
+     do jx = 1, ix ! { dg-error "tiled loops don.t form rectangular" }
+     end do
+  end do
+
+  !$acc loop tile (*)
+  do ix = 1, 30
+     do jx = 1, ix
+     end do
+  end do
+  !$acc end parallel
+end subroutine par

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [gomp4] update gfortran's tile clause error handling
  2016-10-03 14:07 [gomp4] update gfortran's tile clause error handling Cesar Philippidis
@ 2016-10-03 14:35 ` Nathan Sidwell
  2016-11-10 10:47 ` [Patch 4/5] OpenACC tile clause support, Fortran front-end parts Chung-Lin Tang
  1 sibling, 0 replies; 16+ messages in thread
From: Nathan Sidwell @ 2016-10-03 14:35 UTC (permalink / raw)
  To: Cesar Philippidis, gcc-patches, Fortran List

On 10/03/16 10:07, Cesar Philippidis wrote:

> Nathan, I haven't looked too deeply into your tile changes yet. Do you
> know of the fortran FE is doing anything wrong? I haven't checked if
> it's lowering the tile clause in the proper format yet.

thanks for working on this.  The problems I noticed (& fixed) in the C/c++ 
frontends were

1) map '*' onto integer_zero_node -- this makes my changes cleaner.

2) should only accept integer constant expressions (whatever the fortran 
equivalent of that is).  While runtime values could be made to work, the std 
doesn't require that, and it would perform quite badly due to the lack of 
constant folding

3) failing to parse nested loops correctly.  It only parsed the outermost loop 
as a parallel loop.  Tile in many ways looks like collapse

If those could be addressed that'd be great -- it doesn't need my tile WIP to do 
that.

-- 
Nathan Sidwell

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
@ 2016-11-10 10:47 ` Chung-Lin Tang
  2016-11-11 10:34   ` Jakub Jelinek
  0 siblings, 1 reply; 16+ messages in thread
From: Chung-Lin Tang @ 2016-11-10 10:47 UTC (permalink / raw)
  To: gcc-patches, Jakub Jelinek
  Cc: Nathan Sidwell, Cesar Philippidis, Chung-Lin Tang

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

The Fortran front-end patches. These were originally written by Cesar.

Thanks,
Chung-Lin

2016-XX-XX  Cesar Philippidis  <cesar@codesourcery.com>

	fortran/
	* openmp.c (resolve_oacc_positive_int_expr): Promote the warning
	to an error.
	(resolve_oacc_loop_blocks): Use integer zero to represent the '*'
	tile argument.
	(resolve_omp_clauses): Error on directives containing both tile
	and collapse clauses.
	* trans-openmp.c (gfc_trans_omp_do): Lower tiled loops like
	collapsed loops.


	gcc/testsuite/
	* gfortran.dg/goacc/loop-2.f95: Change expected tile clause
        warnings to errors.
        * gfortran.dg/goacc/loop-5.f95: Likewise.
        * gfortran.dg/goacc/sie.f95: Likewise.
        * gfortran.dg/goacc/tile-1.f90: New test.
        * gfortran.dg/goacc/tile-2.f90: New test
        * gfortran.dg/goacc/tile-lowering.f95: New test.

[-- Attachment #2: 04-fortran-front-end.patch --]
[-- Type: text/x-patch, Size: 20659 bytes --]

Index: fortran/openmp.c
===================================================================
--- fortran/openmp.c	(revision 241809)
+++ fortran/openmp.c	(working copy)
@@ -3024,8 +3024,8 @@ resolve_oacc_positive_int_expr (gfc_expr *expr, co
   resolve_oacc_scalar_int_expr (expr, clause);
   if (expr->expr_type == EXPR_CONSTANT && expr->ts.type == BT_INTEGER
       && mpz_sgn(expr->value.integer) <= 0)
-    gfc_warning (0, "INTEGER expression of %s clause at %L must be positive",
-		     clause, &expr->where);
+    gfc_error ("INTEGER expression of %s clause at %L must be positive",
+	       clause, &expr->where);
 }
 
 /* Emits error when symbol is pointer, cray pointer or cray pointee
@@ -3859,6 +3859,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_claus
     if (omp_clauses->wait_list)
       for (el = omp_clauses->wait_list; el; el = el->next)
 	resolve_oacc_scalar_int_expr (el->expr, "WAIT");
+  if (omp_clauses->collapse && omp_clauses->tile_list)
+    gfc_error ("Incompatible use of TILE and COLLAPSE at %L", &code->loc);
 }
 
 
@@ -4964,11 +4966,11 @@ resolve_oacc_loop_blocks (gfc_code *code)
 	  if (el->expr == NULL)
 	    {
 	      /* NULL expressions are used to represent '*' arguments.
-		 Convert those to a -1 expressions.  */
+		 Convert those to a 0 expressions.  */
 	      el->expr = gfc_get_constant_expr (BT_INTEGER,
 						gfc_default_integer_kind,
 						&code->loc);
-	      mpz_set_si (el->expr->value.integer, -1);
+	      mpz_set_si (el->expr->value.integer, 0);
 	    }
 	  else
 	    {
Index: fortran/trans-openmp.c
===================================================================
--- fortran/trans-openmp.c	(revision 241809)
+++ fortran/trans-openmp.c	(working copy)
@@ -3162,7 +3162,18 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op,
   vec<dovar_init> inits = vNULL;
   dovar_init *di;
   unsigned ix;
+  gfc_expr_list *tile = do_clauses ? do_clauses->tile_list : clauses->tile_list;
 
+  /* Both collapsed and tiled loops are lowered the same way.  In
+     OpenACC, those clauses are not compatible, so prioritize the tile
+     clause, if present.  */
+  if (tile)
+    {
+      collapse = 0;
+      for (gfc_expr_list *el = tile; el; el = el->next)
+	collapse++;
+    }
+
   if (collapse <= 0)
     collapse = 1;
 
Index: testsuite/gfortran.dg/goacc/loop-5.f95
===================================================================
--- testsuite/gfortran.dg/goacc/loop-5.f95	(revision 241809)
+++ testsuite/gfortran.dg/goacc/loop-5.f95	(working copy)
@@ -93,9 +93,6 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
-    do i = 1,10
-    enddo
     !$acc loop vector tile(*)
     DO i = 1,10
     ENDDO
@@ -129,9 +126,6 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
-    do i = 1,10
-    enddo
     !$acc loop vector tile(*)
     DO i = 1,10
     ENDDO
@@ -242,9 +236,6 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc kernels loop tile(-1) ! { dg-warning "must be positive" }
-  do i = 1,10
-  enddo
   !$acc kernels loop vector tile(*)
   DO i = 1,10
   ENDDO
@@ -333,9 +324,6 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc parallel loop tile(-1) ! { dg-warning "must be positive" }
-  do i = 1,10
-  enddo
   !$acc parallel loop vector tile(*)
   DO i = 1,10
   ENDDO
Index: testsuite/gfortran.dg/goacc/tile-1.f90
===================================================================
--- testsuite/gfortran.dg/goacc/tile-1.f90	(revision 0)
+++ testsuite/gfortran.dg/goacc/tile-1.f90	(revision 0)
@@ -0,0 +1,339 @@
+subroutine parloop
+  integer, parameter :: n = 100
+  integer i, j, k, a
+
+  !$acc parallel loop tile(10)
+  do i = 1, n
+  end do
+  
+  !$acc parallel loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(10, *)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+ 
+  !$acc parallel loop tile(10, *, i) ! { dg-error "" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(,1) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(,,) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(1.1) ! { dg-error "requires a scalar INTEGER" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(-3) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(10, -3) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(-100, 10, 5) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(10, .true.) ! { dg-error "requires a scalar" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(1, a) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(a, 1) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+end subroutine parloop
+
+subroutine par
+  integer, parameter :: n = 100
+  integer i, j, k
+
+  !$acc parallel
+  !$acc loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(1)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop tile(-2) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(i) ! { dg-error "constant expression" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 2, 1)
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(2, 2)
+  do i = 1, n
+     do j = i+1, n, j ! { dg-error "rectangular iteration space" }
+     end do
+  end do
+
+  !$acc loop vector tile(*)
+  do i = 1, n
+  end do
+  
+  !$acc loop worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+  !$acc end parallel
+end subroutine par
+
+subroutine kern
+  integer, parameter :: n = 100
+  integer i, j, k
+
+  !$acc kernels
+  !$acc loop tile  ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(1)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop tile(-2) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(i) ! { dg-error "constant expression" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 2, 1)
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(2, 2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop vector tile(*)
+  do i = 1, n
+  end do
+  
+  !$acc loop worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+  !$acc end kernels
+end subroutine kern
+
+subroutine kernsloop
+  integer, parameter :: n = 100
+  integer i, j, k, a
+
+  !$acc kernels loop tile(10)
+  do i = 1, n
+  end do
+  
+  !$acc kernels loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(10, *)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+ 
+  !$acc kernels loop tile(10, *, i) ! { dg-error "" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc kernels loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(,1) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(,,) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(1.1) ! { dg-error "requires a scalar INTEGER" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(-3) ! { dg-error "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(10, -3) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(-100, 10, 5) ! { dg-error "must be positive" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc kernels loop tile(10, .true.) ! { dg-error "requires a scalar" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(1, a) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(a, 1) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+end subroutine kernsloop
Index: testsuite/gfortran.dg/goacc/tile-2.f90
===================================================================
--- testsuite/gfortran.dg/goacc/tile-2.f90	(revision 0)
+++ testsuite/gfortran.dg/goacc/tile-2.f90	(revision 0)
@@ -0,0 +1,21 @@
+subroutine par
+  integer ix, jx
+
+  !$acc parallel
+  !$acc loop tile (*,*) ! { dg-error "not enough DO loops for tiled" }
+  do ix = 1, 30
+  end do
+
+  !$acc loop tile (*,*)
+  do ix = 1, 30
+     do jx = 1, ix ! { dg-error "tiled loops don.t form rectangular" }
+     end do
+  end do
+
+  !$acc loop tile (*)
+  do ix = 1, 30
+     do jx = 1, ix
+     end do
+  end do
+  !$acc end parallel
+end subroutine par
Index: testsuite/gfortran.dg/goacc/sie.f95
===================================================================
--- testsuite/gfortran.dg/goacc/sie.f95	(revision 241809)
+++ testsuite/gfortran.dg/goacc/sie.f95	(working copy)
@@ -78,10 +78,10 @@ program test
   !$acc parallel num_gangs(i+1)
   !$acc end parallel
 
-  !$acc parallel num_gangs(-1) ! { dg-warning "must be positive" }
+  !$acc parallel num_gangs(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel num_gangs(0) ! { dg-warning "must be positive" }
+  !$acc parallel num_gangs(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel num_gangs() ! { dg-error "Invalid character in name" }
@@ -107,10 +107,10 @@ program test
   !$acc parallel num_workers(i+1)
   !$acc end parallel
 
-  !$acc parallel num_workers(-1) ! { dg-warning "must be positive" }
+  !$acc parallel num_workers(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel num_workers(0) ! { dg-warning "must be positive" }
+  !$acc parallel num_workers(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel num_workers() ! { dg-error "Invalid character in name" }
@@ -136,10 +136,10 @@ program test
   !$acc parallel vector_length(i+1)
   !$acc end parallel
 
-  !$acc parallel vector_length(-1) ! { dg-warning "must be positive" }
+  !$acc parallel vector_length(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel vector_length(0) ! { dg-warning "must be positive" }
+  !$acc parallel vector_length(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel vector_length() ! { dg-error "Invalid character in name" }
@@ -166,10 +166,10 @@ program test
   !$acc loop gang(i+1)
   do i = 1,10
   enddo
-  !$acc loop gang(-1) ! { dg-warning "must be positive" }
+  !$acc loop gang(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop gang(0) ! { dg-warning "must be positive" }
+  !$acc loop gang(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop gang() ! { dg-error "Invalid character in name" }
@@ -198,10 +198,10 @@ program test
   !$acc loop worker(i+1)
   do i = 1,10
   enddo
-  !$acc loop worker(-1) ! { dg-warning "must be positive" }
+  !$acc loop worker(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop worker(0) ! { dg-warning "must be positive" }
+  !$acc loop worker(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop worker() ! { dg-error "Invalid character in name" }
@@ -230,10 +230,10 @@ program test
   !$acc loop vector(i+1)
   do i = 1,10
   enddo
-  !$acc loop vector(-1) ! { dg-warning "must be positive" }
+  !$acc loop vector(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop vector(0) ! { dg-warning "must be positive" }
+  !$acc loop vector(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop vector() ! { dg-error "Invalid character in name" }
@@ -249,4 +249,4 @@ program test
   do i = 1,10
   enddo
 
-end program test
\ No newline at end of file
+end program test
Index: testsuite/gfortran.dg/goacc/tile-lowering.f95
===================================================================
--- testsuite/gfortran.dg/goacc/tile-lowering.f95	(revision 0)
+++ testsuite/gfortran.dg/goacc/tile-lowering.f95	(revision 0)
@@ -0,0 +1,292 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-original" }
+
+subroutine par
+  integer i, j, k
+
+  !$acc parallel
+  !$acc loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+  !$acc end parallel
+end subroutine par
+
+subroutine kerns
+  integer i, j, k
+
+  !$acc kernels
+  !$acc loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+  !$acc end kernels
+end subroutine kerns
+
+subroutine parloop
+  integer i, j, k
+
+  !$acc parallel loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc parallel loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc parallel loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc parallel loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc parallel loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc parallel loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+end subroutine parloop
+
+subroutine kernloop
+  integer i, j, k
+
+  !$acc kernels loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc kernels loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc kernels loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc kernels loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc kernels loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc kernels loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+end subroutine kernloop
+
+
+! { dg-final { scan-tree-dump-times "tile\\(1\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 2\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0, 2\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0, 0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 2, 3\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0, 2, 3\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 0, 3\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 2, 0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "for \\(" 88 "original" } }
+! { dg-final { scan-tree-dump-times "while \\(" 0 "original" } }
Index: testsuite/gfortran.dg/goacc/loop-2.f95
===================================================================
--- testsuite/gfortran.dg/goacc/loop-2.f95	(revision 241809)
+++ testsuite/gfortran.dg/goacc/loop-2.f95	(working copy)
@@ -143,7 +143,7 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
+    !$acc loop tile(-1) ! { dg-error "must be positive" }
     do i = 1,10
     enddo
     !$acc loop tile(i) ! { dg-error "constant expression" }
@@ -307,7 +307,7 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
+    !$acc loop tile(-1) ! { dg-error "must be positive" }
     do i = 1,10
     enddo
     !$acc loop tile(i) ! { dg-error "constant expression" }
@@ -460,7 +460,7 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc kernels loop tile(-1) ! { dg-warning "must be positive" }
+  !$acc kernels loop tile(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc kernels loop tile(i) ! { dg-error "constant expression" }
@@ -612,7 +612,7 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc parallel loop tile(-1) ! { dg-warning "must be positive" }
+  !$acc parallel loop tile(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc parallel loop tile(i) ! { dg-error "constant expression" }

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-11-10 10:47 ` [Patch 4/5] OpenACC tile clause support, Fortran front-end parts Chung-Lin Tang
@ 2016-11-11 10:34   ` Jakub Jelinek
  2016-11-12 16:51     ` Cesar Philippidis
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Jelinek @ 2016-11-11 10:34 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Nathan Sidwell, Cesar Philippidis

On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:
> 
> Thanks,
> Chung-Lin
> 
> 2016-XX-XX  Cesar Philippidis  <cesar@codesourcery.com>
> 
> 	fortran/
> 	* openmp.c (resolve_oacc_positive_int_expr): Promote the warning
> 	to an error.
> 	(resolve_oacc_loop_blocks): Use integer zero to represent the '*'
> 	tile argument.
> 	(resolve_omp_clauses): Error on directives containing both tile
> 	and collapse clauses.
> 	* trans-openmp.c (gfc_trans_omp_do): Lower tiled loops like
> 	collapsed loops.
> 
> 
> 	gcc/testsuite/
> 	* gfortran.dg/goacc/loop-2.f95: Change expected tile clause
>         warnings to errors.
>         * gfortran.dg/goacc/loop-5.f95: Likewise.
>         * gfortran.dg/goacc/sie.f95: Likewise.
>         * gfortran.dg/goacc/tile-1.f90: New test.
>         * gfortran.dg/goacc/tile-2.f90: New test
>         * gfortran.dg/goacc/tile-lowering.f95: New test.

Again, 8 spaces in ChangeLog.  Missing full stop after New test

> --- fortran/openmp.c	(revision 241809)
> +++ fortran/openmp.c	(working copy)
> @@ -3024,8 +3024,8 @@ resolve_oacc_positive_int_expr (gfc_expr *expr, co
>    resolve_oacc_scalar_int_expr (expr, clause);
>    if (expr->expr_type == EXPR_CONSTANT && expr->ts.type == BT_INTEGER
>        && mpz_sgn(expr->value.integer) <= 0)
> -    gfc_warning (0, "INTEGER expression of %s clause at %L must be positive",
> -		     clause, &expr->where);
> +    gfc_error ("INTEGER expression of %s clause at %L must be positive",
> +	       clause, &expr->where);
>  }

This can't be against current trunk.  The current routine is shared with
OpenMP and gfc_error is undesirable there.

> @@ -3859,6 +3859,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_claus
>      if (omp_clauses->wait_list)
>        for (el = omp_clauses->wait_list; el; el = el->next)
>  	resolve_oacc_scalar_int_expr (el->expr, "WAIT");
> +  if (omp_clauses->collapse && omp_clauses->tile_list)
> +    gfc_error ("Incompatible use of TILE and COLLAPSE at %L", &code->loc);

Shouldn't you in that case for error recovery clear collapse (or tile_list)?

	Jakub

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-11-11 10:34   ` Jakub Jelinek
@ 2016-11-12 16:51     ` Cesar Philippidis
  2016-11-18 11:24       ` Jakub Jelinek
  0 siblings, 1 reply; 16+ messages in thread
From: Cesar Philippidis @ 2016-11-12 16:51 UTC (permalink / raw)
  To: Jakub Jelinek, Chung-Lin Tang; +Cc: gcc-patches, Nathan Sidwell

[-- Attachment #1: Type: text/plain, Size: 2186 bytes --]

On 11/11/2016 02:34 AM, Jakub Jelinek wrote:
> On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:

And here's the patch.

Cesar

>> 2016-XX-XX  Cesar Philippidis  <cesar@codesourcery.com>
>>
>> 	fortran/
>> 	* openmp.c (resolve_oacc_positive_int_expr): Promote the warning
>> 	to an error.
>> 	(resolve_oacc_loop_blocks): Use integer zero to represent the '*'
>> 	tile argument.
>> 	(resolve_omp_clauses): Error on directives containing both tile
>> 	and collapse clauses.
>> 	* trans-openmp.c (gfc_trans_omp_do): Lower tiled loops like
>> 	collapsed loops.
>>
>>
>> 	gcc/testsuite/
>> 	* gfortran.dg/goacc/loop-2.f95: Change expected tile clause
>>         warnings to errors.
>>         * gfortran.dg/goacc/loop-5.f95: Likewise.
>>         * gfortran.dg/goacc/sie.f95: Likewise.
>>         * gfortran.dg/goacc/tile-1.f90: New test.
>>         * gfortran.dg/goacc/tile-2.f90: New test
>>         * gfortran.dg/goacc/tile-lowering.f95: New test.
> 
> Again, 8 spaces in ChangeLog.  Missing full stop after New test
> 
>> --- fortran/openmp.c	(revision 241809)
>> +++ fortran/openmp.c	(working copy)
>> @@ -3024,8 +3024,8 @@ resolve_oacc_positive_int_expr (gfc_expr *expr, co
>>    resolve_oacc_scalar_int_expr (expr, clause);
>>    if (expr->expr_type == EXPR_CONSTANT && expr->ts.type == BT_INTEGER
>>        && mpz_sgn(expr->value.integer) <= 0)
>> -    gfc_warning (0, "INTEGER expression of %s clause at %L must be positive",
>> -		     clause, &expr->where);
>> +    gfc_error ("INTEGER expression of %s clause at %L must be positive",
>> +	       clause, &expr->where);
>>  }
> 
> This can't be against current trunk.  The current routine is shared with
> OpenMP and gfc_error is undesirable there.
> 
>> @@ -3859,6 +3859,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_claus
>>      if (omp_clauses->wait_list)
>>        for (el = omp_clauses->wait_list; el; el = el->next)
>>  	resolve_oacc_scalar_int_expr (el->expr, "WAIT");
>> +  if (omp_clauses->collapse && omp_clauses->tile_list)
>> +    gfc_error ("Incompatible use of TILE and COLLAPSE at %L", &code->loc);
> 
> Shouldn't you in that case for error recovery clear collapse (or tile_list)?
> 
> 	Jakub
> 


[-- Attachment #2: routine-cxx-fe.diff --]
[-- Type: text/x-patch, Size: 17243 bytes --]

2016-11-11  Cesar Philippidis  <cesar@codesourcery.com>
	    Thomas Schwinge  <thomas@codesourcery.com>

	gcc/cp/
	* cp-tree.h (bind_decls_match): Declare.
	* decl.c (bind_decls_match): New function.
	* parser.c (cp_parser_omp_clause_name): 
	(cp_parser_oacc_shape_clause): New location_t loc argument.  Use it
	to report more accurate diagnostics.
	(cp_parser_oacc_clause_bind): New function.
	(cp_parser_oacc_all_clauses): Handle OpenACC bind and nohost clauses.
	Update calls to c_parser_oacc_{simple,shape}_clause.
	(OACC_ROUTINE_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_{BIND,NOHOST}.
	(cp_parser_oacc_routine): Update diagnostics.
	(cp_parser_late_parsing_oacc_routine): Likewise.
	(cp_finalize_oacc_routine): Likewise.
	* semantics.c (finish_omp_clauses): Handle OMP_CLAUSE_{BIND,NOHOST}.


diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8183775..efd5450 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5772,6 +5772,7 @@ extern void finish_scope			(void);
 extern void push_switch				(tree);
 extern void pop_switch				(void);
 extern tree make_lambda_name			(void);
+extern int bind_decls_match			(tree, tree);
 extern int decls_match				(tree, tree);
 extern tree duplicate_decls			(tree, tree, bool);
 extern tree declare_local_label			(tree);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 185c98b..7b242781 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1198,6 +1198,138 @@ decls_match (tree newdecl, tree olddecl)
   return types_match;
 }
 
+/* Similiar to decls_match, but only applies to FUNCTION_DECLS.  Functions
+   in separate namespaces may match.
+*/
+
+int
+bind_decls_match (tree newdecl, tree olddecl)
+{
+  int types_match;
+
+  if (newdecl == olddecl)
+    return 1;
+
+  if (TREE_CODE (newdecl) != TREE_CODE (olddecl))
+    /* If the two DECLs are not even the same kind of thing, we're not
+       interested in their types.  */
+    return 0;
+
+  gcc_assert (DECL_P (newdecl));
+  gcc_assert (TREE_CODE (newdecl) == FUNCTION_DECL);
+
+  tree f1 = TREE_TYPE (newdecl);
+  tree f2 = TREE_TYPE (olddecl);
+  tree p1 = TYPE_ARG_TYPES (f1);
+  tree p2 = TYPE_ARG_TYPES (f2);
+  tree r2;
+
+  /* Specializations of different templates are different functions
+     even if they have the same type.  */
+  tree t1 = (DECL_USE_TEMPLATE (newdecl)
+	     ? DECL_TI_TEMPLATE (newdecl)
+	     : NULL_TREE);
+  tree t2 = (DECL_USE_TEMPLATE (olddecl)
+	     ? DECL_TI_TEMPLATE (olddecl)
+	     : NULL_TREE);
+  if (t1 != t2)
+    return 0;
+
+  if (CP_DECL_CONTEXT (newdecl) != CP_DECL_CONTEXT (olddecl)
+      && TREE_CODE (CP_DECL_CONTEXT (newdecl)) != NAMESPACE_DECL
+      && TREE_CODE (CP_DECL_CONTEXT (olddecl)) != NAMESPACE_DECL
+      && ! (DECL_EXTERN_C_P (newdecl)
+	    && DECL_EXTERN_C_P (olddecl)))
+    return 0;
+
+  /* A new declaration doesn't match a built-in one unless it
+     is also extern "C".  */
+  if (DECL_IS_BUILTIN (olddecl)
+      && DECL_EXTERN_C_P (olddecl) && !DECL_EXTERN_C_P (newdecl))
+    return 0;
+
+  if (TREE_CODE (f1) != TREE_CODE (f2))
+    return 0;
+
+  /* A declaration with deduced return type should use its pre-deduction
+     type for declaration matching.  */
+  r2 = fndecl_declared_return_type (olddecl);
+
+  if (same_type_p (TREE_TYPE (f1), r2))
+    {
+      if (!prototype_p (f2) && DECL_EXTERN_C_P (olddecl)
+	  && (DECL_BUILT_IN (olddecl)
+#ifndef NO_IMPLICIT_EXTERN_C
+	      || (DECL_IN_SYSTEM_HEADER (newdecl) && !DECL_CLASS_SCOPE_P (newdecl))
+	      || (DECL_IN_SYSTEM_HEADER (olddecl) && !DECL_CLASS_SCOPE_P (olddecl))
+#endif
+	      ))
+	{
+	  types_match = self_promoting_args_p (p1);
+	  if (p1 == void_list_node)
+	    TREE_TYPE (newdecl) = TREE_TYPE (olddecl);
+	}
+#ifndef NO_IMPLICIT_EXTERN_C
+      else if (!prototype_p (f1)
+	       && (DECL_EXTERN_C_P (olddecl)
+		   && DECL_IN_SYSTEM_HEADER (olddecl)
+		   && !DECL_CLASS_SCOPE_P (olddecl))
+	       && (DECL_EXTERN_C_P (newdecl)
+		   && DECL_IN_SYSTEM_HEADER (newdecl)
+		   && !DECL_CLASS_SCOPE_P (newdecl)))
+	{
+	  types_match = self_promoting_args_p (p2);
+	  TREE_TYPE (newdecl) = TREE_TYPE (olddecl);
+	}
+#endif
+      else
+	types_match =
+	  compparms (p1, p2)
+	  && type_memfn_rqual (f1) == type_memfn_rqual (f2)
+	  && (TYPE_ATTRIBUTES (TREE_TYPE (newdecl)) == NULL_TREE
+	      || comp_type_attributes (TREE_TYPE (newdecl),
+				       TREE_TYPE (olddecl)) != 0);
+    }
+  else
+    types_match = 0;
+
+  /* The decls dont match if they correspond to two different versions
+     of the same function.   Disallow extern "C" functions to be
+     versions for now.  */
+  if (types_match
+      && !DECL_EXTERN_C_P (newdecl)
+      && !DECL_EXTERN_C_P (olddecl)
+      && targetm.target_option.function_versions (newdecl, olddecl))
+    {
+      /* Mark functions as versions if necessary.  Modify the mangled decl
+	 name if necessary.  */
+      if (DECL_FUNCTION_VERSIONED (newdecl)
+	  && DECL_FUNCTION_VERSIONED (olddecl))
+	return 0;
+      if (!DECL_FUNCTION_VERSIONED (newdecl))
+	{
+	  DECL_FUNCTION_VERSIONED (newdecl) = 1;
+	  if (DECL_ASSEMBLER_NAME_SET_P (newdecl))
+	    mangle_decl (newdecl);
+	}
+      if (!DECL_FUNCTION_VERSIONED (olddecl))
+	{
+	  DECL_FUNCTION_VERSIONED (olddecl) = 1;
+	  if (DECL_ASSEMBLER_NAME_SET_P (olddecl))
+	    mangle_decl (olddecl);
+	}
+      cgraph_node::record_function_versions (olddecl, newdecl);
+      return 0;
+    }
+
+  /* Normal functions can be constrained, as can variable partial
+     specializations.  */
+  if (types_match && VAR_OR_FUNCTION_DECL_P (newdecl))
+    types_match = equivalently_constrained (newdecl, olddecl);
+
+  return types_match;
+}
+
 /* If NEWDECL is `static' and an `extern' was seen previously,
    warn about it.  OLDDECL is the previous declaration.
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7b95dba..33ee6a5 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -30223,6 +30223,10 @@ cp_parser_omp_clause_name (cp_parser *parser)
 	  else if (!strcmp ("async", p))
 	    result = PRAGMA_OACC_CLAUSE_ASYNC;
 	  break;
+	case 'b':
+	  if (!strcmp ("bind", p))
+	    result = PRAGMA_OACC_CLAUSE_BIND;
+	  break;
 	case 'c':
 	  if (!strcmp ("collapse", p))
 	    result = PRAGMA_OMP_CLAUSE_COLLAPSE;
@@ -30302,6 +30306,8 @@ cp_parser_omp_clause_name (cp_parser *parser)
 	    result = PRAGMA_OMP_CLAUSE_NOTINBRANCH;
 	  else if (!strcmp ("nowait", p))
 	    result = PRAGMA_OMP_CLAUSE_NOWAIT;
+	  else if (!strcmp ("nohost", p))
+	    result = PRAGMA_OACC_CLAUSE_NOHOST;
 	  else if (flag_cilkplus && !strcmp ("nomask", p))
 	    result = PRAGMA_CILK_CLAUSE_NOMASK;
 	  else if (!strcmp ("num_gangs", p))
@@ -30778,13 +30784,13 @@ cp_parser_oacc_single_int_clause (cp_parser *parser, omp_clause_code code,
 */
 
 static tree
-cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind,
+cp_parser_oacc_shape_clause (cp_parser *parser, location_t loc,
+			     omp_clause_code kind,
 			     const char *str, tree list)
 {
   const char *id = "num";
   cp_lexer *lexer = parser->lexer;
   tree ops[2] = { NULL_TREE, NULL_TREE }, c;
-  location_t loc = cp_lexer_peek_token (lexer)->location;
 
   if (kind == OMP_CLAUSE_VECTOR)
     id = "length";
@@ -32571,6 +32577,82 @@ cp_parser_oacc_clause_async (cp_parser *parser, tree list)
   return list;
 }
 
+/* OpenACC 2.0:
+   bind ( identifier )
+   bind ( string-literal ) */
+
+static tree
+cp_parser_oacc_clause_bind (cp_parser *parser, tree list)
+{
+  location_t loc = cp_lexer_peek_token (parser->lexer)->location;
+
+  check_no_duplicate_clause (list, OMP_CLAUSE_BIND, "bind", loc);
+
+  bool save_translate_strings_p = parser->translate_strings_p;
+  parser->translate_strings_p = false;
+  if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
+    {
+      parser->translate_strings_p = save_translate_strings_p;
+      return list;
+    }
+  tree name = error_mark_node;
+  cp_token *token = cp_lexer_peek_token (parser->lexer);
+  if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+    {
+      tree id = cp_parser_id_expression (parser, /*template_p=*/false,
+					 /*check_dependency_p=*/true,
+					 /*template_p=*/NULL,
+					 /*declarator_p=*/false,
+					 /*optional_p=*/false);
+      tree decl = cp_parser_lookup_name_simple (parser, id, token->location);
+      if (id != error_mark_node && decl == error_mark_node)
+	cp_parser_name_lookup_error (parser, id, decl, NLE_NULL,
+				     token->location);
+      if (!decl || decl == error_mark_node)
+	error_at (token->location, "%qE has not been declared",
+		  token->u.value);
+      else if (is_overloaded_fn (decl)
+	       && (TREE_CODE (decl) != FUNCTION_DECL
+		   || DECL_FUNCTION_TEMPLATE_P (decl)))
+	error_at (token->location, "%qE names a set of overloads",
+		  token->u.value);
+      else if (TREE_CODE (decl) != FUNCTION_DECL)
+	error_at (token->location,
+		  "%qE does not refer to a function",
+		  token->u.value);
+      else
+	name = decl;
+    }
+  else if (cp_lexer_next_token_is (parser->lexer, CPP_STRING))
+    {
+      name = token->u.value;
+      cp_lexer_consume_token (parser->lexer);
+
+      /* This shouldn't be an empty string.  */
+      if (strcmp (TREE_STRING_POINTER (name), "\"\"") == 0)
+	error_at (token->location,
+		  "bind argument must not be an empty string");
+
+      parser->translate_strings_p = save_translate_strings_p;
+    }
+  else
+    {
+      cp_parser_error (parser,
+		       "expected identifier or character string literal");
+      cp_parser_skip_to_closing_parenthesis (parser, false, false, true);
+      return list;
+    }
+  cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN);
+  if (name != error_mark_node)
+    {
+      tree c = build_omp_clause (loc, OMP_CLAUSE_BIND);
+      OMP_CLAUSE_BIND_NAME (c) = name;
+      OMP_CLAUSE_CHAIN (c) = list;
+      list = c;
+    }
+  return list;
+}
+
 /* Parse all OpenACC clauses.  The set clauses allowed by the directive
    is a bitmask in MASK.  Return the list of clauses found.  */
 
@@ -32607,6 +32689,10 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 						 clauses, here);
 	  c_name = "auto";
 	  break;
+	case PRAGMA_OACC_CLAUSE_BIND:
+	  clauses = cp_parser_oacc_clause_bind (parser, clauses);
+	  c_name = "bind";
+	  break;
 	case PRAGMA_OACC_CLAUSE_COLLAPSE:
 	  clauses = cp_parser_omp_clause_collapse (parser, clauses, here);
 	  c_name = "collapse";
@@ -32654,7 +32740,7 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_GANG:
 	  c_name = "gang";
-	  clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_GANG,
+	  clauses = cp_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_GANG,
 						 c_name, clauses);
 	  break;
 	case PRAGMA_OACC_CLAUSE_HOST:
@@ -32675,6 +32761,11 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  clauses = cp_parser_oacc_data_clause (parser, c_kind, clauses);
 	  c_name = "link";
 	  break;
+	case PRAGMA_OACC_CLAUSE_NOHOST:
+	  clauses = cp_parser_oacc_simple_clause (parser, OMP_CLAUSE_NOHOST,
+						  clauses, here);
+	  c_name = "nohost";
+	  break;
 	case PRAGMA_OACC_CLAUSE_NUM_GANGS:
 	  code = OMP_CLAUSE_NUM_GANGS;
 	  c_name = "num_gangs";
@@ -32736,7 +32827,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_VECTOR:
 	  c_name = "vector";
-	  clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_VECTOR,
+	  clauses = cp_parser_oacc_shape_clause (parser, here,
+						 OMP_CLAUSE_VECTOR,
 						 c_name, clauses);
 	  break;
 	case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH:
@@ -32751,7 +32843,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_WORKER:
 	  c_name = "worker";
-	  clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_WORKER,
+	  clauses = cp_parser_oacc_shape_clause (parser, here,
+						 OMP_CLAUSE_WORKER,
 						 c_name, clauses);
 	  break;
 	default:
@@ -36992,7 +37085,10 @@ cp_parser_omp_taskloop (cp_parser *parser, cp_token *pragma_tok,
 	( (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_GANG)		\
 	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_WORKER)		\
 	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_VECTOR)		\
-	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_SEQ))
+	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_SEQ)			\
+	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_BIND)		\
+	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_NOHOST))
+
 
 
 /* Parse the OpenACC routine pragma.  This has an optional '( name )'
@@ -37051,6 +37147,9 @@ cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok,
 	= cp_parser_oacc_all_clauses (parser, OACC_ROUTINE_CLAUSE_MASK,
 				      "#pragma acc routine",
 				      cp_lexer_peek_token (parser->lexer));
+      /* The clauses are in reverse order; fix that to make later diagnostic
+	 emission easier.  */
+      data.clauses = nreverse (data.clauses);
 
       if (decl && is_overloaded_fn (decl)
 	  && (TREE_CODE (decl) != FUNCTION_DECL
@@ -37062,16 +37161,6 @@ cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok,
 	  return;
 	}
 
-      /* Perhaps we should use the same rule as declarations in different
-	 namespaces?  */
-      if (!DECL_NAMESPACE_SCOPE_P (decl))
-	{
-	  error_at (name_loc,
-		    "%qD does not refer to a namespace scope function", decl);
-	  parser->oacc_routine = NULL;
-	  return;
-	}
-
       if (TREE_CODE (decl) != FUNCTION_DECL)
 	{
 	  error_at (name_loc, "%qD does not refer to a function", decl);
@@ -37147,6 +37236,9 @@ cp_parser_late_parsing_oacc_routine (cp_parser *parser, tree attrs)
   parser->oacc_routine->clauses
     = cp_parser_oacc_all_clauses (parser, OACC_ROUTINE_CLAUSE_MASK,
 				  "#pragma acc routine", pragma_tok);
+  /* The clauses are in reverse order; fix that to make later diagnostic
+     emission easier.  */
+  parser->oacc_routine->clauses = nreverse (parser->oacc_routine->clauses);
   cp_parser_pop_lexer (parser);
   /* Later, cp_finalize_oacc_routine will process the clauses, and then set
      fndecl_seen.  */
@@ -37181,31 +37273,70 @@ cp_finalize_oacc_routine (cp_parser *parser, tree fndecl, bool is_defn)
 	  return;
 	}
 
-      if (get_oacc_fn_attrib (fndecl))
+      /* Process the bind clause, if present.  */
+      for (tree c = parser->oacc_routine->clauses;
+	   c;
+	   c = OMP_CLAUSE_CHAIN (c))
 	{
-	  error_at (parser->oacc_routine->loc,
-		    "%<#pragma acc routine%> already applied to %qD", fndecl);
-	  parser->oacc_routine = NULL;
-	  return;
+	  if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_BIND)
+	    continue;
+
+	  tree bind_decl = OMP_CLAUSE_BIND_NAME (c);
+
+	  /* String arguments don't require any special treatment.  */
+	  if (TREE_CODE (bind_decl) != FUNCTION_DECL)
+	    break;
+
+	  if (!bind_decls_match (bind_decl, fndecl))
+	    {
+	      error_at (OMP_CLAUSE_LOCATION (c),
+			"bind identifier %qE is not compatible with "
+			"function %qE", bind_decl, fndecl);
+	      parser->oacc_routine = NULL;
+	      return;
+	    }
+
+	  tree name_id = decl_assembler_name (bind_decl);
+	  tree name = build_string (IDENTIFIER_LENGTH (name_id),
+				    IDENTIFIER_POINTER (name_id));
+	  OMP_CLAUSE_BIND_NAME (c) = name;
+
+	  break;
 	}
 
-      if (TREE_USED (fndecl) || (!is_defn && DECL_SAVED_TREE (fndecl)))
+      int compatible
+	= verify_oacc_routine_clauses (fndecl, &parser->oacc_routine->clauses,
+				       parser->oacc_routine->loc,
+				       "#pragma acc routine");
+      if (compatible < 0)
 	{
-	  error_at (parser->oacc_routine->loc,
-		    "%<#pragma acc routine%> must be applied before %s",
-		    TREE_USED (fndecl) ? "use" : "definition");
 	  parser->oacc_routine = NULL;
 	  return;
 	}
+      if (compatible > 0)
+	{
+	}
+      else
+	{
+	  if (TREE_USED (fndecl) || (!is_defn && DECL_SAVED_TREE (fndecl)))
+	    {
+	      error_at (parser->oacc_routine->loc,
+			"%<#pragma acc routine%> must be applied before %s",
+			TREE_USED (fndecl) ? "use" : "definition");
+	      parser->oacc_routine = NULL;
+	      return;
+	    }
 
-      /* Process the routine's dimension clauses.  */
-      tree dims = build_oacc_routine_dims (parser->oacc_routine->clauses);
-      replace_oacc_fn_attrib (fndecl, dims);
-      
-      /* Add an "omp declare target" attribute.  */
-      DECL_ATTRIBUTES (fndecl)
-	= tree_cons (get_identifier ("omp declare target"),
-		     NULL_TREE, DECL_ATTRIBUTES (fndecl));
+	  /* Set the routine's level of parallelism.  */
+	  tree dims = build_oacc_routine_dims (parser->oacc_routine->clauses);
+	  replace_oacc_fn_attrib (fndecl, dims);
+
+	  /* Add an "omp declare target" attribute.  */
+	  DECL_ATTRIBUTES (fndecl)
+	    = tree_cons (get_identifier ("omp declare target"),
+			 parser->oacc_routine->clauses,
+			 DECL_ATTRIBUTES (fndecl));
+	}
 
       /* Don't unset parser->oacc_routine here: we may still need it to
 	 diagnose wrong usage.  But, remember that we've used this "#pragma acc
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 1a7c478..8958347 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7073,6 +7073,8 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 	case OMP_CLAUSE_AUTO:
 	case OMP_CLAUSE_INDEPENDENT:
 	case OMP_CLAUSE_SEQ:
+	case OMP_CLAUSE_BIND:
+	case OMP_CLAUSE_NOHOST:
 	  break;
 
 	case OMP_CLAUSE_TILE:

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-11-12 16:51     ` Cesar Philippidis
@ 2016-11-18 11:24       ` Jakub Jelinek
  2016-11-30  1:47         ` Cesar Philippidis
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Jelinek @ 2016-11-18 11:24 UTC (permalink / raw)
  To: Cesar Philippidis; +Cc: Chung-Lin Tang, gcc-patches, Nathan Sidwell

On Sat, Nov 12, 2016 at 08:51:00AM -0800, Cesar Philippidis wrote:
> On 11/11/2016 02:34 AM, Jakub Jelinek wrote:
> > On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:
> 
> And here's the patch.

The patch doesn't look like OpenACC tile clause fortran support,
but bind/nohost clause C/C++ support.

	Jakub

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-11-18 11:24       ` Jakub Jelinek
@ 2016-11-30  1:47         ` Cesar Philippidis
  2016-12-14  8:50           ` Chung-Lin Tang
                             ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Cesar Philippidis @ 2016-11-30  1:47 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Chung-Lin Tang, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]

On 11/18/2016 03:24 AM, Jakub Jelinek wrote:
> On Sat, Nov 12, 2016 at 08:51:00AM -0800, Cesar Philippidis wrote:
>> On 11/11/2016 02:34 AM, Jakub Jelinek wrote:
>>> On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:
>>
>> And here's the patch.
> 
> The patch doesn't look like OpenACC tile clause fortran support,
> but bind/nohost clause C/C++ support.

I think I got that patch mixed up with the acc routines patch. Here is
the fortran tile clause patch.

One notable difference between the trunk and gomp4 implementation of the
tile clause is that gomp4 errors on negative value tile arguments,
whereas trunk issues warnings. Is there a reason why the fortran FE
generally emits a warning, on say num_threads(-5), instead of an error?

Chung-Lin, I noticed in your source tree that you included a change to
gfortranspec.c. Is that necessary for trunk? I've included in this patch
just in case the other tile patches require it.

Cesar

[-- Attachment #2: gfc-tile.diff --]
[-- Type: text/x-patch, Size: 16772 bytes --]

2016-11-29  Cesar Philippidis  <cesar@codesourcery.com>
	    Joseph Myers  <joseph@codesourcery.com>

	gcc/fortran/
	* gfortranspec.c (lang_specific_pre_link): Update call to do_spec.
	* openmp.c (resolve_omp_clauses): Error on directives
	containing both tile and collapse clauses.
	(resolve_oacc_loop_blocks): Represent '*' tile arguments as zero.
	* trans-openmp.c (gfc_trans_omp_do): Lower tiled loops like
 	collapsed loops.

	gcc/testsuite/
	* gfortran.dg/goacc/combined-directives.f90: Remove xfail.
	* gfortran.dg/goacc/tile-1.f90: New test.
	* gfortran.dg/goacc/tile-2.f90: New test.
	* gfortran.dg/goacc/tile-lowering.f95: New test.


diff --git a/gcc/fortran/gfortranspec.c b/gcc/fortran/gfortranspec.c
index 8a0e19a..6dc6fbe 100644
--- a/gcc/fortran/gfortranspec.c
+++ b/gcc/fortran/gfortranspec.c
@@ -439,7 +439,7 @@ int
 lang_specific_pre_link (void)
 {
   if (library)
-    do_spec ("%:include(libgfortran.spec)");
+    do_spec ("%:include(libgfortran.spec)", 0);
 
   return 0;
 }
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 11ffb5d..81f758e 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -4757,6 +4757,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
     if (omp_clauses->wait_list)
       for (el = omp_clauses->wait_list; el; el = el->next)
 	resolve_scalar_int_expr (el->expr, "WAIT");
+  if (omp_clauses->collapse && omp_clauses->tile_list)
+    gfc_error ("Incompatible use of TILE and COLLAPSE at %L", &code->loc);
   if (omp_clauses->depend_source && code->op != EXEC_OMP_ORDERED)
     gfc_error ("SOURCE dependence type only allowed "
 	       "on ORDERED directive at %L", &code->loc);
@@ -5903,11 +5905,11 @@ resolve_oacc_loop_blocks (gfc_code *code)
 	  if (el->expr == NULL)
 	    {
 	      /* NULL expressions are used to represent '*' arguments.
-		 Convert those to a -1 expressions.  */
+		 Convert those to a 0 expressions.  */
 	      el->expr = gfc_get_constant_expr (BT_INTEGER,
 						gfc_default_integer_kind,
 						&code->loc);
-	      mpz_set_si (el->expr->value.integer, -1);
+	      mpz_set_si (el->expr->value.integer, 0);
 	    }
 	  else
 	    {
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 59fd6b3..d38bebc 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -3455,6 +3455,17 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t *pblock,
   dovar_init *di;
   unsigned ix;
   vec<tree, va_heap, vl_embed> *saved_doacross_steps = doacross_steps;
+  gfc_expr_list *tile = do_clauses ? do_clauses->tile_list : clauses->tile_list;
+
+  /* Both collapsed and tiled loops are lowered the same way.  In
+     OpenACC, those clauses are not compatible, so prioritize the tile
+     clause, if present.  */
+  if (tile)
+    {
+      collapse = 0;
+      for (gfc_expr_list *el = tile; el; el = el->next)
+	collapse++;
+    }
 
   doacross_steps = NULL;
   if (clauses->orderedc)
diff --git a/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90 b/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90
index abb5e6b..42a447a 100644
--- a/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90
@@ -143,8 +143,7 @@ end subroutine test
 ! { dg-final { scan-tree-dump-times "acc loop private.i. private.j. vector" 2 "gimple" } }
 ! { dg-final { scan-tree-dump-times "acc loop private.i. private.j. seq" 2 "gimple" } }
 ! { dg-final { scan-tree-dump-times "acc loop private.i. private.j. auto" 2 "gimple" } }
-! XFAILed: OpenACC tile clauses are discarded during gimplification.
-! { dg-final { scan-tree-dump-times "acc loop private.i. private.j. tile.2, 3" 2 "gimple" { xfail *-*-* } } }
+! { dg-final { scan-tree-dump-times "acc loop private.i. private.j. tile.2, 3" 2 "gimple" } }
 ! { dg-final { scan-tree-dump-times "acc loop private.i. independent" 2 "gimple" } }
 ! { dg-final { scan-tree-dump-times "private.z" 2 "gimple" } }
 ! { dg-final { scan-tree-dump-times "omp target oacc_\[^ \]+ map.force_tofrom:y" 2 "gimple" } }
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-1.f90 b/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
new file mode 100644
index 0000000..3dbabda
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
@@ -0,0 +1,339 @@
+subroutine parloop
+  integer, parameter :: n = 100
+  integer i, j, k, a
+
+  !$acc parallel loop tile(10)
+  do i = 1, n
+  end do
+  
+  !$acc parallel loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(10, *)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+ 
+  !$acc parallel loop tile(10, *, i) ! { dg-error "" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(,1) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(,,) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(1.1) ! { dg-error "requires a scalar INTEGER" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(-3) ! { dg-warning "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc parallel loop tile(10, -3) ! { dg-warning "must be positive" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(-100, 10, 5) ! { dg-warning "must be positive" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(10, .true.) ! { dg-error "requires a scalar" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(1, a) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(a, 1) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc parallel loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+end subroutine parloop
+
+subroutine par
+  integer, parameter :: n = 100
+  integer i, j, k
+
+  !$acc parallel
+  !$acc loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(1)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop tile(-2) ! { dg-warning "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(i) ! { dg-error "constant expression" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 2, 1)
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(2, 2)
+  do i = 1, n
+     do j = i+1, n, j ! { dg-error "rectangular iteration space" }
+     end do
+  end do
+
+  !$acc loop vector tile(*)
+  do i = 1, n
+  end do
+  
+  !$acc loop worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+  !$acc end parallel
+end subroutine par
+
+subroutine kern
+  integer, parameter :: n = 100
+  integer i, j, k
+
+  !$acc kernels
+  !$acc loop tile  ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(1)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop tile(-2) ! { dg-warning "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(i) ! { dg-error "constant expression" }
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 2, 1)
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc parallel loop tile(2, 2)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc loop vector tile(*)
+  do i = 1, n
+  end do
+  
+  !$acc loop worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector gang tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop vector worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop gang worker tile(*)
+  do i = 1, n
+  end do
+
+  !$acc loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+  !$acc end kernels
+end subroutine kern
+
+subroutine kernsloop
+  integer, parameter :: n = 100
+  integer i, j, k, a
+
+  !$acc kernels loop tile(10)
+  do i = 1, n
+  end do
+  
+  !$acc kernels loop tile(*)
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(10, *)
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+ 
+  !$acc kernels loop tile(10, *, i) ! { dg-error "" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc kernels loop tile ! { dg-error "Unclassifiable" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile() ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(,1) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(,,) ! { dg-error "Syntax error" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(1.1) ! { dg-error "requires a scalar INTEGER" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(-3) ! { dg-warning "must be positive" }
+  do i = 1, n
+  end do
+
+  !$acc kernels loop tile(10, -3) ! { dg-warning "must be positive" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(-100, 10, 5) ! { dg-warning "must be positive" }
+  do i = 1, n
+     do j = 1, n
+        do k = 1, n
+        end do
+     end do
+  end do 
+
+  !$acc kernels loop tile(10, .true.) ! { dg-error "requires a scalar" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(1, a) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(a, 1) ! { dg-error "constant expression" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+
+  !$acc kernels loop tile(2, 3) collapse (2) ! { dg-error "Incompatible use" }
+  do i = 1, n
+     do j = 1, n
+     end do
+  end do
+end subroutine kernsloop
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-2.f90 b/gcc/testsuite/gfortran.dg/goacc/tile-2.f90
new file mode 100644
index 0000000..c567543
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-2.f90
@@ -0,0 +1,21 @@
+subroutine par
+  integer ix, jx
+
+  !$acc parallel
+  !$acc loop tile (*,*) ! { dg-error "not enough DO loops for tiled" }
+  do ix = 1, 30
+  end do
+
+  !$acc loop tile (*,*)
+  do ix = 1, 30
+     do jx = 1, ix ! { dg-error "tiled loops don.t form rectangular" }
+     end do
+  end do
+
+  !$acc loop tile (*)
+  do ix = 1, 30
+     do jx = 1, ix
+     end do
+  end do
+  !$acc end parallel
+end subroutine par
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-lowering.f95 b/gcc/testsuite/gfortran.dg/goacc/tile-lowering.f95
new file mode 100644
index 0000000..1cb8b9c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-lowering.f95
@@ -0,0 +1,292 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-original" }
+
+subroutine par
+  integer i, j, k
+
+  !$acc parallel
+  !$acc loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+  !$acc end parallel
+end subroutine par
+
+subroutine kerns
+  integer i, j, k
+
+  !$acc kernels
+  !$acc loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+  !$acc end kernels
+end subroutine kerns
+
+subroutine parloop
+  integer i, j, k
+
+  !$acc parallel loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc parallel loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc parallel loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc parallel loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc parallel loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc parallel loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc parallel loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+end subroutine parloop
+
+subroutine kernloop
+  integer i, j, k
+
+  !$acc kernels loop tile (1)
+  do i = 1, 10
+  end do
+
+  !$acc kernels loop tile (*)
+  do i = 1, 10
+  end do
+
+  !$acc kernels loop tile (1,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (*,2)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (1,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (*,*)
+  do i = 1, 10
+     do j = 1, 10
+     end do
+  end do
+
+  !$acc kernels loop tile (1,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc kernels loop tile (*,2,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc kernels loop tile (1,*,3)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+
+  !$acc kernels loop tile (1,2,*)
+  do i = 1, 10
+     do j = 1, 10
+        do k = 1, 10
+        end do
+     end do
+  end do
+end subroutine kernloop
+
+
+! { dg-final { scan-tree-dump-times "tile\\(1\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 2\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0, 2\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0, 0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 2, 3\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(0, 2, 3\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 0, 3\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "tile\\(1, 2, 0\\)" 4 "original" } }
+! { dg-final { scan-tree-dump-times "for \\(" 88 "original" } }
+! { dg-final { scan-tree-dump-times "while \\(" 0 "original" } }

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-11-30  1:47         ` Cesar Philippidis
@ 2016-12-14  8:50           ` Chung-Lin Tang
  2017-01-12 12:00             ` Chung-Lin Tang
  2017-01-12 13:07           ` Jakub Jelinek
                             ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Chung-Lin Tang @ 2016-12-14  8:50 UTC (permalink / raw)
  To: Cesar Philippidis, Jakub Jelinek; +Cc: gcc-patches

On 2016/11/30 10:47 AM, Cesar Philippidis wrote:
> On 11/18/2016 03:24 AM, Jakub Jelinek wrote:
>> On Sat, Nov 12, 2016 at 08:51:00AM -0800, Cesar Philippidis wrote:
>>> On 11/11/2016 02:34 AM, Jakub Jelinek wrote:
>>>> On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:
>>>
>>> And here's the patch.
>>
>> The patch doesn't look like OpenACC tile clause fortran support,
>> but bind/nohost clause C/C++ support.
> 
> I think I got that patch mixed up with the acc routines patch. Here is
> the fortran tile clause patch.
> 
> One notable difference between the trunk and gomp4 implementation of the
> tile clause is that gomp4 errors on negative value tile arguments,
> whereas trunk issues warnings. Is there a reason why the fortran FE
> generally emits a warning, on say num_threads(-5), instead of an error?
> 
> Chung-Lin, I noticed in your source tree that you included a change to
> gfortranspec.c. Is that necessary for trunk? I've included in this patch
> just in case the other tile patches require it.
> 
> Cesar
> 

About the gfortranspec.c change, that's just a testing artifact, not part of the Fortran FE patch.

Chung-Lin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-12-14  8:50           ` Chung-Lin Tang
@ 2017-01-12 12:00             ` Chung-Lin Tang
  0 siblings, 0 replies; 16+ messages in thread
From: Chung-Lin Tang @ 2017-01-12 12:00 UTC (permalink / raw)
  To: Cesar Philippidis, Jakub Jelinek; +Cc: gcc-patches

[ping]

Hi Jakub,
I don't think we ever got approval for the Fortran parts of the OpenACC tile patch?
Is this okay for trunk? I'll be committing all parts of the patch once the Fortran
parts are approved.

Thanks,
Chung-Lin


On 2016/12/14 04:37 PM, Chung-Lin Tang wrote:
> On 2016/11/30 10:47 AM, Cesar Philippidis wrote:
>> On 11/18/2016 03:24 AM, Jakub Jelinek wrote:
>>> On Sat, Nov 12, 2016 at 08:51:00AM -0800, Cesar Philippidis wrote:
>>>> On 11/11/2016 02:34 AM, Jakub Jelinek wrote:
>>>>> On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:
>>>>
>>>> And here's the patch.
>>>
>>> The patch doesn't look like OpenACC tile clause fortran support,
>>> but bind/nohost clause C/C++ support.
>>
>> I think I got that patch mixed up with the acc routines patch. Here is
>> the fortran tile clause patch.
>>
>> One notable difference between the trunk and gomp4 implementation of the
>> tile clause is that gomp4 errors on negative value tile arguments,
>> whereas trunk issues warnings. Is there a reason why the fortran FE
>> generally emits a warning, on say num_threads(-5), instead of an error?
>>
>> Chung-Lin, I noticed in your source tree that you included a change to
>> gfortranspec.c. Is that necessary for trunk? I've included in this patch
>> just in case the other tile patches require it.
>>
>> Cesar
>>
> 
> About the gfortranspec.c change, that's just a testing artifact, not part of the Fortran FE patch.
> 
> Chung-Lin
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts
  2016-11-30  1:47         ` Cesar Philippidis
  2016-12-14  8:50           ` Chung-Lin Tang
@ 2017-01-12 13:07           ` Jakub Jelinek
  2018-08-07 21:47           ` [PATCH][OpenACC] update gfortran's tile clause error handling Cesar Philippidis
  2019-04-09 16:36           ` Negative arguments in OpenMP 'num_threads' clause etc. (was: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts) Thomas Schwinge
  3 siblings, 0 replies; 16+ messages in thread
From: Jakub Jelinek @ 2017-01-12 13:07 UTC (permalink / raw)
  To: Cesar Philippidis; +Cc: Chung-Lin Tang, gcc-patches

On Tue, Nov 29, 2016 at 05:47:08PM -0800, Cesar Philippidis wrote:
> On 11/18/2016 03:24 AM, Jakub Jelinek wrote:
> > On Sat, Nov 12, 2016 at 08:51:00AM -0800, Cesar Philippidis wrote:
> >> On 11/11/2016 02:34 AM, Jakub Jelinek wrote:
> >>> On Thu, Nov 10, 2016 at 06:46:46PM +0800, Chung-Lin Tang wrote:
> >>
> >> And here's the patch.
> > 
> > The patch doesn't look like OpenACC tile clause fortran support,
> > but bind/nohost clause C/C++ support.
> 
> I think I got that patch mixed up with the acc routines patch. Here is
> the fortran tile clause patch.
> 
> One notable difference between the trunk and gomp4 implementation of the
> tile clause is that gomp4 errors on negative value tile arguments,
> whereas trunk issues warnings. Is there a reason why the fortran FE
> generally emits a warning, on say num_threads(-5), instead of an error?
> 
> Chung-Lin, I noticed in your source tree that you included a change to
> gfortranspec.c. Is that necessary for trunk? I've included in this patch
> just in case the other tile patches require it.

This is ok for trunk without the gfortranspec.c hunk (please remove it also
from the ChangeLog entry).

> 2016-11-29  Cesar Philippidis  <cesar@codesourcery.com>
> 	    Joseph Myers  <joseph@codesourcery.com>
> 
> 	gcc/fortran/
> 	* gfortranspec.c (lang_specific_pre_link): Update call to do_spec.
> 	* openmp.c (resolve_omp_clauses): Error on directives
> 	containing both tile and collapse clauses.
> 	(resolve_oacc_loop_blocks): Represent '*' tile arguments as zero.
> 	* trans-openmp.c (gfc_trans_omp_do): Lower tiled loops like
>  	collapsed loops.
> 
> 	gcc/testsuite/
> 	* gfortran.dg/goacc/combined-directives.f90: Remove xfail.
> 	* gfortran.dg/goacc/tile-1.f90: New test.
> 	* gfortran.dg/goacc/tile-2.f90: New test.
> 	* gfortran.dg/goacc/tile-lowering.f95: New test.

	Jakub

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH][OpenACC] update gfortran's tile clause error handling
@ 2018-08-07 21:47           ` Cesar Philippidis
  2018-12-04 14:07             ` Jakub Jelinek
  0 siblings, 1 reply; 16+ messages in thread
From: Cesar Philippidis @ 2018-08-07 21:47 UTC (permalink / raw)
  To: gcc-patches, Fortran List

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

This patch updates how the OpenACC tile clause is handled in the Fortran
FE to match it's behavior in C/C++. Specifically, the tile clause now
errors on negative integer arguments, instead of emitting a warning.

Is this OK for trunk?

Thanks,
Cesar

[-- Attachment #2: 0001-OpenACC-update-gfortran-s-tile-clause-error-handling.patch --]
[-- Type: text/x-patch, Size: 10983 bytes --]

From af39a6d65cfb46397fa62c88521189002fb3d705 Mon Sep 17 00:00:00 2001
From: Cesar Philippidis <cesar@codesourcery.com>
Date: Mon, 3 Oct 2016 13:58:59 +0000
Subject: [PATCH] [OpenACC] update gfortran's tile clause error handling

2018-XX-YY  Cesar Philippidis  <cesar@codesourcery.com>

	gcc/fortran/
	* openmp.c (resolve_positive_int_expr): Promote the warning to an
	error.

	gcc/testsuite/
	* gfortran.dg/goacc/loop-2.f95: Change expected tile clause
	warnings to errors.
	* gfortran.dg/goacc/loop-5.f95: Likewise.
	* gfortran.dg/goacc/sie.f95: Likewise.
	* gfortran.dg/goacc/tile-1.f90: New test.
	* gfortran.dg/goacc/tile-2.f90: New test.

---
 gcc/fortran/openmp.c                       |  4 ++--
 gcc/testsuite/gfortran.dg/goacc/loop-2.f95 |  8 +++----
 gcc/testsuite/gfortran.dg/goacc/loop-5.f95 | 12 ----------
 gcc/testsuite/gfortran.dg/goacc/sie.f95    | 36 +++++++++++++++---------------
 gcc/testsuite/gfortran.dg/goacc/tile-1.f90 | 16 ++++++-------
 gcc/testsuite/gfortran.dg/gomp/pr77516.f90 |  2 +-
 6 files changed, 33 insertions(+), 45 deletions(-)

diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 5c0ae45..b346b51 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -3719,8 +3719,8 @@ resolve_positive_int_expr (gfc_expr *expr, const char *clause)
   if (expr->expr_type == EXPR_CONSTANT
       && expr->ts.type == BT_INTEGER
       && mpz_sgn (expr->value.integer) <= 0)
-    gfc_warning (0, "INTEGER expression of %s clause at %L must be positive",
-		 clause, &expr->where);
+    gfc_error ("INTEGER expression of %s clause at %L must be positive",
+	       clause, &expr->where);
 }
 
 static void
diff --git a/gcc/testsuite/gfortran.dg/goacc/loop-2.f95 b/gcc/testsuite/gfortran.dg/goacc/loop-2.f95
index 0c902b2..d4c6273 100644
--- a/gcc/testsuite/gfortran.dg/goacc/loop-2.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/loop-2.f95
@@ -143,7 +143,7 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
+    !$acc loop tile(-1) ! { dg-error "must be positive" }
     do i = 1,10
     enddo
     !$acc loop tile(i) ! { dg-error "constant expression" }
@@ -307,7 +307,7 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
+    !$acc loop tile(-1) ! { dg-error "must be positive" }
     do i = 1,10
     enddo
     !$acc loop tile(i) ! { dg-error "constant expression" }
@@ -460,7 +460,7 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc kernels loop tile(-1) ! { dg-warning "must be positive" }
+  !$acc kernels loop tile(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc kernels loop tile(i) ! { dg-error "constant expression" }
@@ -612,7 +612,7 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc parallel loop tile(-1) ! { dg-warning "must be positive" }
+  !$acc parallel loop tile(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc parallel loop tile(i) ! { dg-error "constant expression" }
diff --git a/gcc/testsuite/gfortran.dg/goacc/loop-5.f95 b/gcc/testsuite/gfortran.dg/goacc/loop-5.f95
index d059cf7..fe137d5 100644
--- a/gcc/testsuite/gfortran.dg/goacc/loop-5.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/loop-5.f95
@@ -93,9 +93,6 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
-    do i = 1,10
-    enddo
     !$acc loop vector tile(*)
     DO i = 1,10
     ENDDO
@@ -129,9 +126,6 @@ program test
       DO j = 1,10
       ENDDO
     ENDDO
-    !$acc loop tile(-1) ! { dg-warning "must be positive" }
-    do i = 1,10
-    enddo
     !$acc loop vector tile(*)
     DO i = 1,10
     ENDDO
@@ -242,9 +236,6 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc kernels loop tile(-1) ! { dg-warning "must be positive" }
-  do i = 1,10
-  enddo
   !$acc kernels loop vector tile(*)
   DO i = 1,10
   ENDDO
@@ -333,9 +324,6 @@ program test
     DO j = 1,10
     ENDDO
   ENDDO
-  !$acc parallel loop tile(-1) ! { dg-warning "must be positive" }
-  do i = 1,10
-  enddo
   !$acc parallel loop vector tile(*)
   DO i = 1,10
   ENDDO
diff --git a/gcc/testsuite/gfortran.dg/goacc/sie.f95 b/gcc/testsuite/gfortran.dg/goacc/sie.f95
index abfe28b..3abf2c8 100644
--- a/gcc/testsuite/gfortran.dg/goacc/sie.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/sie.f95
@@ -78,10 +78,10 @@ program test
   !$acc parallel num_gangs(i+1)
   !$acc end parallel
 
-  !$acc parallel num_gangs(-1) ! { dg-warning "must be positive" }
+  !$acc parallel num_gangs(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel num_gangs(0) ! { dg-warning "must be positive" }
+  !$acc parallel num_gangs(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel num_gangs() ! { dg-error "Invalid character in name" }
@@ -106,10 +106,10 @@ program test
   !$acc kernels num_gangs(i+1)
   !$acc end kernels
 
-  !$acc kernels num_gangs(-1) ! { dg-warning "must be positive" }
+  !$acc kernels num_gangs(-1) ! { dg-error "must be positive" }
   !$acc end kernels
 
-  !$acc kernels num_gangs(0) ! { dg-warning "must be positive" }
+  !$acc kernels num_gangs(0) ! { dg-error "must be positive" }
   !$acc end kernels
 
   !$acc kernels num_gangs() ! { dg-error "Invalid character in name" }
@@ -135,10 +135,10 @@ program test
   !$acc parallel num_workers(i+1)
   !$acc end parallel
 
-  !$acc parallel num_workers(-1) ! { dg-warning "must be positive" }
+  !$acc parallel num_workers(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel num_workers(0) ! { dg-warning "must be positive" }
+  !$acc parallel num_workers(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel num_workers() ! { dg-error "Invalid character in name" }
@@ -163,10 +163,10 @@ program test
   !$acc kernels num_workers(i+1)
   !$acc end kernels
 
-  !$acc kernels num_workers(-1) ! { dg-warning "must be positive" }
+  !$acc kernels num_workers(-1) ! { dg-error "must be positive" }
   !$acc end kernels
 
-  !$acc kernels num_workers(0) ! { dg-warning "must be positive" }
+  !$acc kernels num_workers(0) ! { dg-error "must be positive" }
   !$acc end kernels
 
   !$acc kernels num_workers() ! { dg-error "Invalid character in name" }
@@ -192,10 +192,10 @@ program test
   !$acc parallel vector_length(i+1)
   !$acc end parallel
 
-  !$acc parallel vector_length(-1) ! { dg-warning "must be positive" }
+  !$acc parallel vector_length(-1) ! { dg-error "must be positive" }
   !$acc end parallel
 
-  !$acc parallel vector_length(0) ! { dg-warning "must be positive" }
+  !$acc parallel vector_length(0) ! { dg-error "must be positive" }
   !$acc end parallel
 
   !$acc parallel vector_length() ! { dg-error "Invalid character in name" }
@@ -220,10 +220,10 @@ program test
   !$acc kernels vector_length(i+1)
   !$acc end kernels
 
-  !$acc kernels vector_length(-1) ! { dg-warning "must be positive" }
+  !$acc kernels vector_length(-1) ! { dg-error "must be positive" }
   !$acc end kernels
 
-  !$acc kernels vector_length(0) ! { dg-warning "must be positive" }
+  !$acc kernels vector_length(0) ! { dg-error "must be positive" }
   !$acc end kernels
 
   !$acc kernels vector_length() ! { dg-error "Invalid character in name" }
@@ -250,10 +250,10 @@ program test
   !$acc loop gang(i+1)
   do i = 1,10
   enddo
-  !$acc loop gang(-1) ! { dg-warning "must be positive" }
+  !$acc loop gang(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop gang(0) ! { dg-warning "must be positive" }
+  !$acc loop gang(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop gang() ! { dg-error "Invalid character in name" }
@@ -282,10 +282,10 @@ program test
   !$acc loop worker(i+1)
   do i = 1,10
   enddo
-  !$acc loop worker(-1) ! { dg-warning "must be positive" }
+  !$acc loop worker(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop worker(0) ! { dg-warning "must be positive" }
+  !$acc loop worker(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop worker() ! { dg-error "Invalid character in name" }
@@ -314,10 +314,10 @@ program test
   !$acc loop vector(i+1)
   do i = 1,10
   enddo
-  !$acc loop vector(-1) ! { dg-warning "must be positive" }
+  !$acc loop vector(-1) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
-  !$acc loop vector(0) ! { dg-warning "must be positive" }
+  !$acc loop vector(0) ! { dg-error "must be positive" }
   do i = 1,10
   enddo
   !$acc loop vector() ! { dg-error "Invalid character in name" }
diff --git a/gcc/testsuite/gfortran.dg/goacc/tile-1.f90 b/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
index 3dbabda..17fd32c 100644
--- a/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/tile-1.f90
@@ -44,17 +44,17 @@ subroutine parloop
   do i = 1, n
   end do
 
-  !$acc parallel loop tile(-3) ! { dg-warning "must be positive" }
+  !$acc parallel loop tile(-3) ! { dg-error "must be positive" }
   do i = 1, n
   end do
 
-  !$acc parallel loop tile(10, -3) ! { dg-warning "must be positive" }
+  !$acc parallel loop tile(10, -3) ! { dg-error "must be positive" }
   do i = 1, n
      do j = 1, n
      end do
   end do
 
-  !$acc parallel loop tile(-100, 10, 5) ! { dg-warning "must be positive" }
+  !$acc parallel loop tile(-100, 10, 5) ! { dg-error "must be positive" }
   do i = 1, n
      do j = 1, n
         do k = 1, n
@@ -114,7 +114,7 @@ subroutine par
      end do
   end do
 
-  !$acc loop tile(-2) ! { dg-warning "must be positive" }
+  !$acc loop tile(-2) ! { dg-error "must be positive" }
   do i = 1, n
   end do
 
@@ -195,7 +195,7 @@ subroutine kern
      end do
   end do
 
-  !$acc loop tile(-2) ! { dg-warning "must be positive" }
+  !$acc loop tile(-2) ! { dg-error "must be positive" }
   do i = 1, n
   end do
 
@@ -295,17 +295,17 @@ subroutine kernsloop
   do i = 1, n
   end do
 
-  !$acc kernels loop tile(-3) ! { dg-warning "must be positive" }
+  !$acc kernels loop tile(-3) ! { dg-error "must be positive" }
   do i = 1, n
   end do
 
-  !$acc kernels loop tile(10, -3) ! { dg-warning "must be positive" }
+  !$acc kernels loop tile(10, -3) ! { dg-error "must be positive" }
   do i = 1, n
      do j = 1, n
      end do
   end do
 
-  !$acc kernels loop tile(-100, 10, 5) ! { dg-warning "must be positive" }
+  !$acc kernels loop tile(-100, 10, 5) ! { dg-error "must be positive" }
   do i = 1, n
      do j = 1, n
         do k = 1, n
diff --git a/gcc/testsuite/gfortran.dg/gomp/pr77516.f90 b/gcc/testsuite/gfortran.dg/gomp/pr77516.f90
index 9c0a95b..3ac3f55 100644
--- a/gcc/testsuite/gfortran.dg/gomp/pr77516.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/pr77516.f90
@@ -4,7 +4,7 @@
 program pr77516
    integer :: i, x
    x = 0
-!$omp simd safelen(0) reduction(+:x)	! { dg-warning "must be positive" }
+!$omp simd safelen(0) reduction(+:x)	! { dg-error "must be positive" }
    do i = 1, 8
       x = x + 1
    end do
-- 
2.7.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH][OpenACC] update gfortran's tile clause error handling
  2018-08-07 21:47           ` [PATCH][OpenACC] update gfortran's tile clause error handling Cesar Philippidis
@ 2018-12-04 14:07             ` Jakub Jelinek
  0 siblings, 0 replies; 16+ messages in thread
From: Jakub Jelinek @ 2018-12-04 14:07 UTC (permalink / raw)
  To: Cesar Philippidis; +Cc: gcc-patches, Fortran List

On Tue, Aug 07, 2018 at 02:47:07PM -0700, Cesar Philippidis wrote:
> This patch updates how the OpenACC tile clause is handled in the Fortran
> FE to match it's behavior in C/C++. Specifically, the tile clause now
> errors on negative integer arguments, instead of emitting a warning.
> 
> Is this OK for trunk?

I've reviewed this already in some other patch, this is not ok, if
that is what you want for OpenACC, you need to copy the function to some
other one and adjust callers to use it for OpenACC clauses only.

> >From af39a6d65cfb46397fa62c88521189002fb3d705 Mon Sep 17 00:00:00 2001
> From: Cesar Philippidis <cesar@codesourcery.com>
> Date: Mon, 3 Oct 2016 13:58:59 +0000
> Subject: [PATCH] [OpenACC] update gfortran's tile clause error handling
> 
> 2018-XX-YY  Cesar Philippidis  <cesar@codesourcery.com>
> 
> 	gcc/fortran/
> 	* openmp.c (resolve_positive_int_expr): Promote the warning to an
> 	error.
> 
> 	gcc/testsuite/
> 	* gfortran.dg/goacc/loop-2.f95: Change expected tile clause
> 	warnings to errors.
> 	* gfortran.dg/goacc/loop-5.f95: Likewise.
> 	* gfortran.dg/goacc/sie.f95: Likewise.
> 	* gfortran.dg/goacc/tile-1.f90: New test.
> 	* gfortran.dg/goacc/tile-2.f90: New test.

	Jakub

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Negative arguments in OpenMP 'num_threads' clause etc. (was: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts)
  2016-11-30  1:47         ` Cesar Philippidis
                             ` (2 preceding siblings ...)
  2018-08-07 21:47           ` [PATCH][OpenACC] update gfortran's tile clause error handling Cesar Philippidis
@ 2019-04-09 16:36           ` Thomas Schwinge
  2019-05-29 14:45             ` Negative arguments in OpenMP 'num_threads' clause etc Thomas Schwinge
  3 siblings, 1 reply; 16+ messages in thread
From: Thomas Schwinge @ 2019-04-09 16:36 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

Hi Jakub!

On Tue, 29 Nov 2016 17:47:08 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote:
> One notable difference between the trunk and gomp4 implementation of the
> tile clause is that gomp4 errors on negative value tile arguments,
> whereas trunk issues warnings.

I'm picking up these changes, which have been posted a few times, and
have been rejected (at least in their current incarnation) a few times,
too.  ;-\

> Is there a reason why the fortran FE
> generally emits a warning, on say num_threads(-5), instead of an error?

Same for the C/C++ front ends, which I'm looking into first.

Jakub, is the reason that even if the user is clearly doing something
"strage" there, the compiler doesn't have a problem to continue
compilation for 'num_threads(-5)', so it just emits a warning, but for
example for 'collapse(-5)' is has to stop with an error, because it can't
continue compilation in that case?  Or, is there a different reason for
the many 'warning_at ([...], "[...] must be positive"' (C front end, for
example), instead of using 'error_at' for these?


Grüße
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Negative arguments in OpenMP 'num_threads' clause etc.
  2019-04-09 16:36           ` Negative arguments in OpenMP 'num_threads' clause etc. (was: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts) Thomas Schwinge
@ 2019-05-29 14:45             ` Thomas Schwinge
  2019-05-29 15:14               ` Jakub Jelinek
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Schwinge @ 2019-05-29 14:45 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Hi Jakub!

Any comments on my question, please?

On Tue, 09 Apr 2019 17:51:46 +0200, I wrote:
> On Tue, 29 Nov 2016 17:47:08 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote:
> > One notable difference between the trunk and gomp4 implementation of the
> > tile clause is that gomp4 errors on negative value tile arguments,
> > whereas trunk issues warnings.
> 
> I'm picking up these changes, which have been posted a few times, and
> have been rejected (at least in their current incarnation) a few times,
> too.  ;-\
> 
> > Is there a reason why the fortran FE
> > generally emits a warning, on say num_threads(-5), instead of an error?
> 
> Same for the C/C++ front ends, which I'm looking into first.
> 
> Jakub, is the reason that even if the user is clearly doing something
> "strage" there, the compiler doesn't have a problem to continue
> compilation for 'num_threads(-5)', so it just emits a warning, but for
> example for 'collapse(-5)' is has to stop with an error, because it can't
> continue compilation in that case?  Or, is there a different reason for
> the many 'warning_at ([...], "[...] must be positive"' (C front end, for
> example), instead of using 'error_at' for these?


Grüße
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Negative arguments in OpenMP 'num_threads' clause etc.
  2019-05-29 14:45             ` Negative arguments in OpenMP 'num_threads' clause etc Thomas Schwinge
@ 2019-05-29 15:14               ` Jakub Jelinek
  2019-06-06  8:06                 ` Thomas Schwinge
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Jelinek @ 2019-05-29 15:14 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc-patches

On Wed, May 29, 2019 at 04:42:14PM +0200, Thomas Schwinge wrote:
> Any comments on my question, please?
> 
> On Tue, 09 Apr 2019 17:51:46 +0200, I wrote:
> > On Tue, 29 Nov 2016 17:47:08 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote:
> > > One notable difference between the trunk and gomp4 implementation of the
> > > tile clause is that gomp4 errors on negative value tile arguments,
> > > whereas trunk issues warnings.
> > 
> > I'm picking up these changes, which have been posted a few times, and
> > have been rejected (at least in their current incarnation) a few times,
> > too.  ;-\
> > 
> > > Is there a reason why the fortran FE
> > > generally emits a warning, on say num_threads(-5), instead of an error?
> > 
> > Same for the C/C++ front ends, which I'm looking into first.
> > 
> > Jakub, is the reason that even if the user is clearly doing something
> > "strage" there, the compiler doesn't have a problem to continue
> > compilation for 'num_threads(-5)', so it just emits a warning, but for
> > example for 'collapse(-5)' is has to stop with an error, because it can't
> > continue compilation in that case?  Or, is there a different reason for
> > the many 'warning_at ([...], "[...] must be positive"' (C front end, for
> > example), instead of using 'error_at' for these?

collapse has a constant expression argument and if the value is negative (or
0), then parsing doesn't make sense, so that case is clearly something where
an error is in order.  num_threads is an example of where the standard is
not completely clear if it is or is not ok to reject compilation as opposed
to just UB at runtime if that happens and no problem if that construct is
never encoutered at runtime.

Kind like a C++ difference between:
void
foo ()
{
  constexpr int a = __INT_MAX__ + 1;
  int b = __INT_MAX__ + 1;
}

where for a we need to error, but for b we just warn (by default, of course
one can use -Werror).

	Jakub

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Negative arguments in OpenMP 'num_threads' clause etc.
  2019-05-29 15:14               ` Jakub Jelinek
@ 2019-06-06  8:06                 ` Thomas Schwinge
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Schwinge @ 2019-06-06  8:06 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]

Hi Jakub!

On Wed, 29 May 2019 16:52:45 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 29, 2019 at 04:42:14PM +0200, Thomas Schwinge wrote:
> > On Tue, 09 Apr 2019 17:51:46 +0200, I wrote:
> > > On Tue, 29 Nov 2016 17:47:08 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote:
> > > > One notable difference between the trunk and gomp4 implementation of the
> > > > tile clause is that gomp4 errors on negative value tile arguments,
> > > > whereas trunk issues warnings.

> > > > Is there a reason why the fortran FE
> > > > generally emits a warning, on say num_threads(-5), instead of an error?
> > > 
> > > Same for the C/C++ front ends, which I'm looking into first.
> > > 
> > > Jakub, is the reason that even if the user is clearly doing something
> > > "strage" there, the compiler doesn't have a problem to continue
> > > compilation for 'num_threads(-5)', so it just emits a warning, but for
> > > example for 'collapse(-5)' is has to stop with an error, because it can't
> > > continue compilation in that case?  Or, is there a different reason for
> > > the many 'warning_at ([...], "[...] must be positive"' (C front end, for
> > > example), instead of using 'error_at' for these?
> 
> collapse has a constant expression argument and if the value is negative (or
> 0), then parsing doesn't make sense, so that case is clearly something where
> an error is in order.  num_threads is an example of where the standard is
> not completely clear if it is or is not ok to reject compilation as opposed
> to just UB at runtime if that happens and no problem if that construct is
> never encoutered at runtime.

Thanks, so that matches my understanding, and we shall thus retract the
OpenACC "update gfortran's tile clause error handling" patch, that got
posted several times in several variations.


Later, we shall audit all front end clauses handling to make sure that
this is done consistently.


Grüße
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-06  8:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-03 14:07 [gomp4] update gfortran's tile clause error handling Cesar Philippidis
2016-10-03 14:35 ` Nathan Sidwell
2016-11-10 10:47 ` [Patch 4/5] OpenACC tile clause support, Fortran front-end parts Chung-Lin Tang
2016-11-11 10:34   ` Jakub Jelinek
2016-11-12 16:51     ` Cesar Philippidis
2016-11-18 11:24       ` Jakub Jelinek
2016-11-30  1:47         ` Cesar Philippidis
2016-12-14  8:50           ` Chung-Lin Tang
2017-01-12 12:00             ` Chung-Lin Tang
2017-01-12 13:07           ` Jakub Jelinek
2018-08-07 21:47           ` [PATCH][OpenACC] update gfortran's tile clause error handling Cesar Philippidis
2018-12-04 14:07             ` Jakub Jelinek
2019-04-09 16:36           ` Negative arguments in OpenMP 'num_threads' clause etc. (was: [Patch 4/5] OpenACC tile clause support, Fortran front-end parts) Thomas Schwinge
2019-05-29 14:45             ` Negative arguments in OpenMP 'num_threads' clause etc Thomas Schwinge
2019-05-29 15:14               ` Jakub Jelinek
2019-06-06  8:06                 ` Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).