[OpenACC] update gfortran's tile clause error handling

Message ID e1b2337a-ef32-76f3-bc94-658155b08b62@codesourcery.com
State New
Headers show
Series
  • [OpenACC] update gfortran's tile clause error handling
Related show

Commit Message

Cesar Philippidis Aug. 7, 2018, 9:47 p.m.
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

Comments

Jakub Jelinek Dec. 4, 2018, 2:07 p.m. | #1
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
Thomas Schwinge June 6, 2019, 8:06 a.m. | #2
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

Patch

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