Update 'libgomp/libgomp.texi' for 'nvptx, libgfortran: Switch out of "minimal" mode' (was: nvptx, libgfortran: Switch out of "minimal" mode)

2023-01-24 Thread Thomas Schwinge
Hi!

On 2023-01-20T22:16:00+0100, I wrote:
> On 2023-01-20T22:04:02+0100, I wrote:
>> We've been (t)asked to enable (portions of) GCC/Fortran I/O for nvptx
>> offloading, which means building a normal (non-'LIBGFOR_MINIMAL')
>> configuration of libgfortran.
>
> This is achieved by 'nvptx, libgfortran: Switch out of "minimal" mode',
> see attached, again based on WIP work by Andrew Stubbs.  This I've just
> pushed to devel/omp/gcc-12 branch in
> commit c7734c6fbb5513b4da6306de7bc85de9b8547988, and would like to push
> to master branch once other pending GCC patches have been accepted.
>
>
> The OpenACC XFAILs: "[...] overflows the stack for nvptx offloading"
> are unresolved at this point; see the discussion around
> "Handling of large stack objects in GPU code generation -- maybe transform 
> into heap allocation?",
> and my "nvptx: '-mframe-malloc-threshold', '-Wframe-malloc-threshold'"
> experimenting.  (The latter works to some extent, but also has other
> issues that I shall detail at some later point in time.)

I had a note from Tobias to "update the the last but one bullet point at
https://gcc.gnu.org/onlinedocs/libgomp/nvptx.html";.  Thus pushed to
devel/omp/gcc-12 branch commit 8c29332e98ca4669a059ebc0d90903b409ae049f
"Update 'libgomp/libgomp.texi' for 'nvptx, libgfortran: Switch out of "minimal" 
mode'",
see attached.  Please consider that one 'fixup'ed into the GCC master
branch submission.


Grüße
 Thomas


> From c7734c6fbb5513b4da6306de7bc85de9b8547988 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge 
> Date: Wed, 21 Sep 2022 18:58:34 +0200
> Subject: [PATCH] nvptx, libgfortran: Switch out of "minimal" mode
>
> ..., in order to enable (portions of) Fortran I/O, for example.
>
> libgfortran/ChangeLog:
>
>   * configure: Regenerate.
>   * configure.ac: No longer set LIBGFOR_MINIMAL for nvptx.
>
> libgomp/ChangeLog:
>
>   * testsuite/libgomp.fortran/target-print-1.f90: Adjust.
>   * testsuite/libgomp.fortran/target-print-1-nvptx.f90: Remove.
>   * testsuite/libgomp.oacc-fortran/print-1.f90: Adjust.
>   * testsuite/libgomp.oacc-fortran/print-1-nvptx.f90: Remove.
>   * testsuite/libgomp.oacc-fortran/error_stop-2.f: Adjust.
>   * testsuite/libgomp.oacc-fortran/stop-2.f: Likewise.
>
> Co-authored-by: Andrew Stubbs 
> ---
>  libgfortran/ChangeLog.omp   |  6 ++
>  libgfortran/configure   | 17 ++---
>  libgfortran/configure.ac| 17 ++---
>  libgomp/ChangeLog.omp   |  7 +++
>  .../libgomp.fortran/target-print-1-nvptx.f90| 11 ---
>  .../libgomp.fortran/target-print-1.f90  |  3 ---
>  .../libgomp.oacc-fortran/error_stop-2.f |  4 +++-
>  .../libgomp.oacc-fortran/print-1-nvptx.f90  | 11 ---
>  .../testsuite/libgomp.oacc-fortran/print-1.f90  |  5 ++---
>  libgomp/testsuite/libgomp.oacc-fortran/stop-2.f |  4 +++-
>  10 files changed, 33 insertions(+), 52 deletions(-)
>  delete mode 100644 libgomp/testsuite/libgomp.fortran/target-print-1-nvptx.f90
>  delete mode 100644 libgomp/testsuite/libgomp.oacc-fortran/print-1-nvptx.f90
>
> diff --git a/libgfortran/ChangeLog.omp b/libgfortran/ChangeLog.omp
> index b08c264daf9..925575e65fa 100644
> --- a/libgfortran/ChangeLog.omp
> +++ b/libgfortran/ChangeLog.omp
> @@ -1,3 +1,9 @@
> +2023-01-20  Thomas Schwinge  
> + Andrew Stubbs  
> +
> + * configure: Regenerate.
> + * configure.ac: No longer set LIBGFOR_MINIMAL for nvptx.
> +
>  2023-01-20  Thomas Schwinge  
>
>   PR target/85463
> diff --git a/libgfortran/configure b/libgfortran/configure
> index ae64dca3114..3e5c931d4ad 100755
> --- a/libgfortran/configure
> +++ b/libgfortran/configure
> @@ -6230,17 +6230,12 @@ else
>  fi
>
>
> -# For GPU offloading, not everything in libfortran can be supported.
> -# Currently, the only target that has this problem is nvptx.  The
> -# following is a (partial) list of features that are unsupportable on
> -# this particular target:
> -# * Constructors
> -# * alloca
> -# * C library support for I/O, with printf as the one notable exception
> -# * C library support for other features such as signal, environment
> -#   variables, time functions
> -
> - if test "x${target_cpu}" = xnvptx; then
> +# "Minimal" mode is for targets that cannot (yet) support all features of
> +# libgfortran.  It avoids the need for working constructors, alloca, and C
> +# library support for I/O, signals, environment variables, time functions, 
> etc.
> +# At present there are no targets that require this mode.
> +
> + if false; then
>LIBGFOR_MINIMAL_TRUE=
>LIBGFOR_MINIMAL_FALSE='#'
>  else
> diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
> index 97cc490cb5e..e5552949cc6 100644
> --- a/libgfortran/configure.ac
> +++ b/libgfortran/configure.ac
> @@ -222,17 +222,12 @@ AM_CONDITIONAL(LIBGFOR_USE_SYMVER, [test 
> "x$gfortran_use_symver" != xno])
>  AM_CONDITIONAL(LIBGFOR_USE_SYMVE

[Patch] OpenMP/Fortran: Fix loop-iter var privatization with !$OMP LOOP [PR108512]

2023-01-24 Thread Tobias Burnus

I stumbled over a new FAIL (regression) in sollve_vv today, which was due to an
odd corner case (see commit log for a description).

The mentioned in-scan error is tested for in gomp/loop-2.f90 ("'inscan' 
REDUCTION
clause on construct other than DO, SIMD, DO SIMD, PARALLEL DO, PARALLEL DO 
SIMD").

I hope that this patch covers all cases and no other surprises exist...

OK for mainline?

 * * *

The ICE is new in GCC 13 due to the duplicate diagnostic (cf. PR); the original 
issue
existed before but seemingly did not affect the code, at least the sollve_vv 
testcase
passed before.

Still, it could be backported to GCC 12. (Fortran '!$omp loop' support was 
added with r12-1206.)
Thoughts?

Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
OpenMP/Fortran: Fix loop-iter var privatization with !$OMP LOOP [PR108512]

For 'parallel', loop-iteration variables are marked are marked as 'private',
unless they either appear in an omp do/simd loop or an data-sharing clause
already exists for those on 'parallel'. 'omp loop' wasn't handled, leading
to (potentially) multiple data-sharing clauses in gfc_resolve_do_iterator
as omp_current_ctx pointed to the 'parallel' directive, ignoring the
in-betwen 'loop' directive.

The latter lead to a bogus diagnostic - or rather an ICE as the source
location var contained only '\0'.

gcc/fortran/ChangeLog:

	PR fortran/108512
	* openmp.cc (gfc_resolve_omp_do_blocks): Don't check 'inscan'
	restrictions for loop as rejected elsewhere.
	(gfc_resolve_do_iterator): Set a source location for added
	'private'-clause arguments.
	* resolve.cc (gfc_resolve_code): Call gfc_resolve_omp_do_blocks
	also for EXEC_OMP_LOOP.

gcc/testsuite/ChangeLog:

	PR fortran/108512
	* gfortran.dg/gomp/loop-5.f90: New test.

 gcc/fortran/openmp.cc |  5 +-
 gcc/fortran/resolve.cc|  1 +
 gcc/testsuite/gfortran.dg/gomp/loop-5.f90 | 84 +++
 3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index cc1eab90b8c..7673a52249f 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -9056,7 +9056,9 @@ gfc_resolve_omp_do_blocks (gfc_code *code, gfc_namespace *ns)
 	}
   if (i < omp_current_do_collapse || omp_current_do_collapse <= 0)
 	omp_current_do_collapse = 1;
-  if (code->ext.omp_clauses->lists[OMP_LIST_REDUCTION_INSCAN])
+  if (code->op == EXEC_OMP_LOOP)
+	;  /* Already rejected in resolve_omp_clauses.  */
+  else if (code->ext.omp_clauses->lists[OMP_LIST_REDUCTION_INSCAN])
 	{
 	  locus *loc
 	= &code->ext.omp_clauses->lists[OMP_LIST_REDUCTION_INSCAN]->where;
@@ -9224,6 +9226,7 @@ gfc_resolve_do_iterator (gfc_code *code, gfc_symbol *sym, bool add_clause)
 
   p = gfc_get_omp_namelist ();
   p->sym = sym;
+  p->where = omp_current_ctx->code->loc;
   p->next = omp_clauses->lists[OMP_LIST_PRIVATE];
   omp_clauses->lists[OMP_LIST_PRIVATE] = p;
 }
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 94213cd3cd4..bd2a749776d 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -11950,6 +11950,7 @@ gfc_resolve_code (gfc_code *code, gfc_namespace *ns)
 	case EXEC_OMP_DISTRIBUTE_SIMD:
 	case EXEC_OMP_DO:
 	case EXEC_OMP_DO_SIMD:
+	case EXEC_OMP_LOOP:
 	case EXEC_OMP_SIMD:
 	case EXEC_OMP_TARGET_SIMD:
 	  gfc_resolve_omp_do_blocks (code, ns);
diff --git a/gcc/testsuite/gfortran.dg/gomp/loop-5.f90 b/gcc/testsuite/gfortran.dg/gomp/loop-5.f90
new file mode 100644
index 000..1948e782653
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/loop-5.f90
@@ -0,0 +1,84 @@
+! { dg-additional-options "-fdump-tree-original" }
+!
+! PR fortran/108512
+
+! The problem was that the context wasn't reset for the 'LOOP'
+! such that the clauses of the loops weren't seen when adding
+! PRIVATE clauses.
+!
+! In the following, only the loop variable of the non-OpenMP loop
+! in 'subroutine four' should get a front-end addded PRIVATE clause
+
+implicit none
+integer :: x, a(10), b(10), n
+n = 10
+a = -42
+b = [(2*x, x=1,10)]
+
+! { dg-final { scan-tree-dump-times "#pragma omp target map\\(tofrom:a\\) map\\(tofrom:b\\) map\\(tofrom:x\\)\[\r\n\]" 1 "original" } }
+! { dg-final { scan-tree-dump-times "#pragma omp parallel\[\r\n\]" 2 "original" } }
+!  ^- shows up twice; checked only here.
+! { dg-final { scan-tree-dump-times "#pragma omp loop lastprivate\\(x\\)\[\r\n\]" 1 "original" } }
+
+!$omp target parallel map(tofrom: a, b, x)
+!$omp loop lastprivate(x)
+DO x = 1, n
+  a(x) = a(x) + b(x)
+END DO
+!$omp end loop
+!$omp end target parallel
+if (x /= 11) error stop
+if (any (a /= [(2*x - 42, x=1,10)])) error stop
+call two()
+call three()

[PATCH, committed] Fortran: ICE in transformational_result [PR108529]

2023-01-24 Thread Harald Anlauf via Fortran
Dear all,

we ICE'd in the simplification of the transformational intrinsic
ANY when the passed ARRAY argument had an invalid declaration.
The reason was a reference to array->shape which was NULL.

Obvious solution: then just don't attempt to simplify.

Regtested on x86_64-pc-linux-gnu and pushed to mainline as

https://gcc.gnu.org/g:6c96382eed96a9285611f2e3e2e59557094172b8

The PR is marked as a 10/11/12/13 regression, thus I plan to
backport.

Thanks,
Harald

From 6c96382eed96a9285611f2e3e2e59557094172b8 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Tue, 24 Jan 2023 21:39:43 +0100
Subject: [PATCH] Fortran: ICE in transformational_result [PR108529]

gcc/fortran/ChangeLog:

	PR fortran/108529
	* simplify.cc (simplify_transformation): Do not try to simplify
	transformational intrinsic when the ARRAY argument has a NULL shape.

gcc/testsuite/ChangeLog:

	PR fortran/108529
	* gfortran.dg/pr108529.f90: New test.
---
 gcc/fortran/simplify.cc| 1 +
 gcc/testsuite/gfortran.dg/pr108529.f90 | 9 +
 2 files changed, 10 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/pr108529.f90

diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index f413f132b3f..20ea38e0007 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -720,6 +720,7 @@ simplify_transformation (gfc_expr *array, gfc_expr *dim, gfc_expr *mask,
   size_zero = gfc_is_size_zero_array (array);

   if (!(is_constant_array_expr (array) || size_zero)
+  || array->shape == NULL
   || !gfc_is_constant_expr (dim))
 return NULL;

diff --git a/gcc/testsuite/gfortran.dg/pr108529.f90 b/gcc/testsuite/gfortran.dg/pr108529.f90
new file mode 100644
index 000..34c9691fae1
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr108529.f90
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/108529 - ICE in transformational_result
+! Contributed by G.Steinmetz
+
+program p
+  integer, parameter :: a(*,*) = reshape([1, 2, 3, 4], [2, 2])
+  logical, parameter :: b(2,*) = a > 2 ! { dg-error "Assumed size" }
+  logical, parameter :: c(*)   = all(b, 1) ! { dg-error "Bad shape" }
+end
--
2.35.3



[PATCH] Fortran: fix ICE in compare_bound_int [PR108527]

2023-01-24 Thread Harald Anlauf via Fortran
Dear all,

when checking expressions for array sections, we need to ensure
that these use only type INTEGER.  However, it does not make sense
to generate an internal error when encountering wrong types,
but rather take the ordinary route of error recovery.

The initial fix was provided by Steve.

While working on the same PR, I found that the comments related
to the logic needed a minor adjustment, and the logic could be
cleaned up to actually match the intended comment.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald

From bd9afd0835bfe956400978503041323aea894ef5 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Tue, 24 Jan 2023 22:36:25 +0100
Subject: [PATCH] Fortran: fix ICE in compare_bound_int [PR108527]

gcc/fortran/ChangeLog:

	PR fortran/108527
	* resolve.cc (compare_bound_int): Expression to compare must be of
	type INTEGER.
	(compare_bound_mpz_t): Likewise.
	(check_dimension): Fix comment on checks applied to array section
	and clean up associated logic.

gcc/testsuite/ChangeLog:

	PR fortran/108527
	* gfortran.dg/pr108527.f90: New test.

Co-authored-by: Steven G. Kargl 
---
 gcc/fortran/resolve.cc | 29 +-
 gcc/testsuite/gfortran.dg/pr108527.f90 | 10 +
 2 files changed, 24 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr108527.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 94213cd3cd4..0538029da26 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -4575,12 +4575,11 @@ compare_bound_int (gfc_expr *a, int b)
 {
   int i;

-  if (a == NULL || a->expr_type != EXPR_CONSTANT)
+  if (a == NULL
+  || a->expr_type != EXPR_CONSTANT
+  || a->ts.type != BT_INTEGER)
 return CMP_UNKNOWN;

-  if (a->ts.type != BT_INTEGER)
-gfc_internal_error ("compare_bound_int(): Bad expression");
-
   i = mpz_cmp_si (a->value.integer, b);

   if (i < 0)
@@ -4598,12 +4597,11 @@ compare_bound_mpz_t (gfc_expr *a, mpz_t b)
 {
   int i;

-  if (a == NULL || a->expr_type != EXPR_CONSTANT)
+  if (a == NULL
+  || a->expr_type != EXPR_CONSTANT
+  || a->ts.type != BT_INTEGER)
 return CMP_UNKNOWN;

-  if (a->ts.type != BT_INTEGER)
-gfc_internal_error ("compare_bound_int(): Bad expression");
-
   i = mpz_cmp (a->value.integer, b);

   if (i < 0)
@@ -4733,23 +4731,24 @@ check_dimension (int i, gfc_array_ref *ar, gfc_array_spec *as)
 #define AR_END (ar->end[i] ? ar->end[i] : as->upper[i])

 	compare_result comp_start_end = compare_bound (AR_START, AR_END);
+	compare_result comp_stride_zero = compare_bound_int (ar->stride[i], 0);

 	/* Check for zero stride, which is not allowed.  */
-	if (compare_bound_int (ar->stride[i], 0) == CMP_EQ)
+	if (comp_stride_zero == CMP_EQ)
 	  {
 	gfc_error ("Illegal stride of zero at %L", &ar->c_where[i]);
 	return false;
 	  }

-	/* if start == len || (stride > 0 && start < len)
-			   || (stride < 0 && start > len),
+	/* if start == end || (stride > 0 && start < end)
+			   || (stride < 0 && start > end),
 	   then the array section contains at least one element.  In this
 	   case, there is an out-of-bounds access if
 	   (start < lower || start > upper).  */
-	if (compare_bound (AR_START, AR_END) == CMP_EQ
-	|| ((compare_bound_int (ar->stride[i], 0) == CMP_GT
-		 || ar->stride[i] == NULL) && comp_start_end == CMP_LT)
-	|| (compare_bound_int (ar->stride[i], 0) == CMP_LT
+	if (comp_start_end == CMP_EQ
+	|| ((comp_stride_zero == CMP_GT || ar->stride[i] == NULL)
+		&& comp_start_end == CMP_LT)
+	|| (comp_stride_zero == CMP_LT
 	&& comp_start_end == CMP_GT))
 	  {
 	if (compare_bound (AR_START, as->lower[i]) == CMP_LT)
diff --git a/gcc/testsuite/gfortran.dg/pr108527.f90 b/gcc/testsuite/gfortran.dg/pr108527.f90
new file mode 100644
index 000..c97ba3111cb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr108527.f90
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! PR fortran/108527 - ICE in compare_bound_int
+! Contributed by G.Steinmetz
+
+program p
+  integer, parameter :: a((2.)) = [4,8] ! { dg-error "must be of INTEGER type" }
+  integer(a(1:1)) :: b  ! { dg-error "out of bounds" }
+end
+
+! { dg-prune-output "Parameter array" }
--
2.35.3