Re: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]

2024-01-12 Thread Bernhard Reutner-Fischer
On Wed, 10 Jan 2024 23:24:22 +0100
Harald Anlauf  wrote:

> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
> index 82f388c05f8..88502c1e3f0 100644
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -2926,6 +2926,10 @@ gfc_dt;
>  typedef struct gfc_forall_iterator
>  {
>gfc_expr *var, *start, *end, *stride;
> +  unsigned short unroll;
> +  bool ivdep;
> +  bool vector;
> +  bool novector;
>struct gfc_forall_iterator *next;
>  }
[]
> diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
> index a718dce237f..59a9cf99f9b 100644
> --- a/gcc/fortran/trans-stmt.cc
> +++ b/gcc/fortran/trans-stmt.cc
> @@ -41,6 +41,10 @@ typedef struct iter_info
>tree start;
>tree end;
>tree step;
> +  unsigned short unroll;
> +  bool ivdep;
> +  bool vector;
> +  bool novector;
>struct iter_info *next;
>  }

Given that we already have in gfortran.h

> typedef struct
> {
>   gfc_expr *var, *start, *end, *step;
>   unsigned short unroll;
>   bool ivdep;
>   bool vector;
>   bool novector;
> }
> gfc_iterator;

would it make sense to break out these loop annotation flags into its
own let's say struct gfc_iterator_flags and use pointers to that flags
instead?

LGTM otherwise.
Thanks for the patch!


[PATCH, v2] Fortran: annotations for DO CONCURRENT loops [PR113305]

2024-01-12 Thread Harald Anlauf

Hi Bernhard,

On 1/12/24 10:44, Bernhard Reutner-Fischer wrote:

On Wed, 10 Jan 2024 23:24:22 +0100
Harald Anlauf  wrote:


diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 82f388c05f8..88502c1e3f0 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2926,6 +2926,10 @@ gfc_dt;
  typedef struct gfc_forall_iterator
  {
gfc_expr *var, *start, *end, *stride;
+  unsigned short unroll;
+  bool ivdep;
+  bool vector;
+  bool novector;
struct gfc_forall_iterator *next;
  }

[]

diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
index a718dce237f..59a9cf99f9b 100644
--- a/gcc/fortran/trans-stmt.cc
+++ b/gcc/fortran/trans-stmt.cc
@@ -41,6 +41,10 @@ typedef struct iter_info
tree start;
tree end;
tree step;
+  unsigned short unroll;
+  bool ivdep;
+  bool vector;
+  bool novector;
struct iter_info *next;
  }


Given that we already have in gfortran.h


typedef struct
{
   gfc_expr *var, *start, *end, *step;
   unsigned short unroll;
   bool ivdep;
   bool vector;
   bool novector;
}
gfc_iterator;


would it make sense to break out these loop annotation flags into its
own let's say struct gfc_iterator_flags and use pointers to that flags
instead?


I've created a struct gfc_loop_annot and use that directly
as I think using pointers to it is probably not very efficient.
Well, the struct is smaller than a pointer on a 64-bit system...


LGTM otherwise.
Thanks for the patch!


Thanks for the review!

If there are no further comments, I'll commit the attached version
soon.

Harald

From 31d8957a95455663577a0e60109679d56aac234d Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Fri, 12 Jan 2024 19:51:11 +0100
Subject: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]

gcc/fortran/ChangeLog:

	PR fortran/113305
	* gfortran.h (gfc_loop_annot): New.
	(gfc_iterator, gfc_forall_iterator): Use for annotation control.
	* array.cc (gfc_copy_iterator): Adjust.
	* gfortran.texi: Document annotations IVDEP, UNROLL n, VECTOR,
	NOVECTOR as applied to DO CONCURRENT.
	* parse.cc (parse_do_block): Parse annotations IVDEP, UNROLL n,
	VECTOR, NOVECTOR as applied to DO CONCURRENT.  Apply UNROLL only to
	first loop control variable.
	* trans-stmt.cc (iter_info): Use gfc_loop_annot.
	(gfc_trans_simple_do): Adjust.
	(gfc_trans_forall_loop): Annotate loops with IVDEP, UNROLL n,
	VECTOR, NOVECTOR as needed for DO CONCURRENT.
	(gfc_trans_forall_1): Handle loop annotations.

gcc/testsuite/ChangeLog:

	PR fortran/113305
	* gfortran.dg/do_concurrent_7.f90: New test.
---
 gcc/fortran/array.cc  |  5 +-
 gcc/fortran/gfortran.h| 11 -
 gcc/fortran/gfortran.texi | 12 +
 gcc/fortran/parse.cc  | 34 --
 gcc/fortran/trans-stmt.cc | 46 ++-
 gcc/testsuite/gfortran.dg/do_concurrent_7.f90 | 26 +++
 6 files changed, 113 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_7.f90

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index 19456baf103..81fa99d219f 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -2308,10 +2308,7 @@ gfc_copy_iterator (gfc_iterator *src)
   dest->start = gfc_copy_expr (src->start);
   dest->end = gfc_copy_expr (src->end);
   dest->step = gfc_copy_expr (src->step);
-  dest->unroll = src->unroll;
-  dest->ivdep = src->ivdep;
-  dest->vector = src->vector;
-  dest->novector = src->novector;
+  dest->annot = src->annot;
 
   return dest;
 }
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 82f388c05f8..fd73e4ce431 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2830,14 +2830,22 @@ gfc_case;
 #define gfc_get_case() XCNEW (gfc_case)
 
 
+/* Annotations for loop constructs.  */
 typedef struct
 {
-  gfc_expr *var, *start, *end, *step;
   unsigned short unroll;
   bool ivdep;
   bool vector;
   bool novector;
 }
+gfc_loop_annot;
+
+
+typedef struct
+{
+  gfc_expr *var, *start, *end, *step;
+  gfc_loop_annot annot;
+}
 gfc_iterator;
 
 #define gfc_get_iterator() XCNEW (gfc_iterator)
@@ -2926,6 +2934,7 @@ gfc_dt;
 typedef struct gfc_forall_iterator
 {
   gfc_expr *var, *start, *end, *stride;
+  gfc_loop_annot annot;
   struct gfc_forall_iterator *next;
 }
 gfc_forall_iterator;
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 5615fee2897..371666dcbb6 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -3262,6 +3262,9 @@ It must be placed immediately before a @code{DO} loop and applies only to the
 loop that follows.  N is an integer constant specifying the unrolling factor.
 The values of 0 and 1 block any unrolling of the loop.
 
+For @code{DO CONCURRENT} constructs the unrolling specification applies
+only to the first loop control variable.
+
 
 @node BUILTIN directive
 @subsection BUILTIN directive
@@ -3300,6 +3303,9 @@ whether a particular loop is vectorizable due