diff mbox series

[PR,fortran/91496] !GCC$ directives error if mistyped or unknown

Message ID 5D644B3D.60907@gmx.de
State New
Headers show
Series [PR,fortran/91496] !GCC$ directives error if mistyped or unknown | expand

Commit Message

Harald Anlauf Aug. 26, 2019, 9:12 p.m. UTC
Dear all,

the attached patch adds Fortran support for the following pragmas
(loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
NOVECTOR.  Furthermore, it downgrades unsupported directives from
error to warning (by default, it stays an error with -pedantic),
thus fixing the PR.

It has no effect on existing code (thus regtested cleanly on
x86_64-pc-linux-gnu), but gives users an option for fine-grained
control of optimization.  The above pragmas are supported by other
compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
sometimes with slightly different keywords).

OK for trunk, and backport to 9?

Thanks,
Harald


2019-08-26  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/91496
	* gfortran.h: Extend struct gfc_iterator for loop annotations.
	* array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
	VECTOR, and NOVECTOR pragmas.
	* decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
	(gfc_match_gcc_novector): New matcher functions handling IVDEP,
	VECTOR, and NOVECTOR pragmas.
	* match.h: Declare prototypes of matcher functions handling IVDEP,
	VECTOR, and NOVECTOR pragmas.
	* parse.c (decode_gcc_attribute, parse_do_block)
	(parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
	emit warning for unrecognized pragmas instead of error.
	* trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
	emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
	* gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.

2019-08-26  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/91496
	* gfortran.dg/pr91496.f90: New testcase.

Index: gcc/testsuite/gfortran.dg/pr91496.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pr91496.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr91496.f90	(working copy)
@@ -0,0 +1,38 @@
+! { dg-do compile }
+! { dg-options "-fdump-tree-original" }
+!
+subroutine foo (a, b, c, n)
+  implicit none
+  real a(*), b(*), c(*)
+  integer :: i, n
+  external bar
+!DIR$ unroll (4)
+!GCC$ unroll 4
+  do i = 1, n
+     a(i) = b(i) + c(i)
+  end do
+!DIR$ ivdep
+!GCC$ ivdep
+  do i = 1, n
+     a(i) = b(i) + c(i)
+  end do
+!DIR$ vector
+!GCC$ vector
+  do i = 1, n
+     a(i) = b(i) + c(i)
+  end do
+!DIR$ novector
+!GCC$ novector
+  do i = 1, n
+     a(i) = b(i) + c(i)
+  end do
+!GCC$ ivdep
+!GCC$ vector
+  do i = 1, n
+     a(i) = b(i) + c(i)
+  end do
+!DIR$ noinline
+!GCC$ noinline          ! { dg-warning "Unclassifiable GCC directive" }
+  call bar (a)
+end subroutine foo
+! { dg-final { scan-tree-dump-times "ANNOTATE_EXPR" 6 "original" } }

Comments

Paul Richard Thomas Aug. 27, 2019, 8:33 a.m. UTC | #1
Hi Harald,

This is OK for trunk.

Thanks!

Paul

On Mon, 26 Aug 2019 at 22:13, Harald Anlauf <anlauf@gmx.de> wrote:
>
> Dear all,
>
> the attached patch adds Fortran support for the following pragmas
> (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
> NOVECTOR.  Furthermore, it downgrades unsupported directives from
> error to warning (by default, it stays an error with -pedantic),
> thus fixing the PR.
>
> It has no effect on existing code (thus regtested cleanly on
> x86_64-pc-linux-gnu), but gives users an option for fine-grained
> control of optimization.  The above pragmas are supported by other
> compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
> sometimes with slightly different keywords).
>
> OK for trunk, and backport to 9?
>
> Thanks,
> Harald
>
>
> 2019-08-26  Harald Anlauf  <anlauf@gmx.de>
>
>         PR fortran/91496
>         * gfortran.h: Extend struct gfc_iterator for loop annotations.
>         * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
>         VECTOR, and NOVECTOR pragmas.
>         * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
>         (gfc_match_gcc_novector): New matcher functions handling IVDEP,
>         VECTOR, and NOVECTOR pragmas.
>         * match.h: Declare prototypes of matcher functions handling IVDEP,
>         VECTOR, and NOVECTOR pragmas.
>         * parse.c (decode_gcc_attribute, parse_do_block)
>         (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
>         emit warning for unrecognized pragmas instead of error.
>         * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
>         emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
>         * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.
>
> 2019-08-26  Harald Anlauf  <anlauf@gmx.de>
>
>         PR fortran/91496
>         * gfortran.dg/pr91496.f90: New testcase.
>
Harald Anlauf Aug. 27, 2019, 7:49 p.m. UTC | #2
Committed to trunk as svn revision 274966, after removing some
accidentally left-over unused variable declarations (copy&paste).
The actual committed version is attached.

Thanks, Paul, for the quick review!

Unless there are strong objections, I'd like to commit this to
9-branch, too, so that this can be used in the 9.3 release.
Applies/regtests cleanly.  Will wait for a week or so.

Harald

On 08/27/19 10:33, Paul Richard Thomas wrote:
> Hi Harald,
>
> This is OK for trunk.
>
> Thanks!
>
> Paul
>
> On Mon, 26 Aug 2019 at 22:13, Harald Anlauf <anlauf@gmx.de> wrote:
>>
>> Dear all,
>>
>> the attached patch adds Fortran support for the following pragmas
>> (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
>> NOVECTOR.  Furthermore, it downgrades unsupported directives from
>> error to warning (by default, it stays an error with -pedantic),
>> thus fixing the PR.
>>
>> It has no effect on existing code (thus regtested cleanly on
>> x86_64-pc-linux-gnu), but gives users an option for fine-grained
>> control of optimization.  The above pragmas are supported by other
>> compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
>> sometimes with slightly different keywords).
>>
>> OK for trunk, and backport to 9?
>>
>> Thanks,
>> Harald
>>
>>
>> 2019-08-26  Harald Anlauf  <anlauf@gmx.de>
>>
>>         PR fortran/91496
>>         * gfortran.h: Extend struct gfc_iterator for loop annotations.
>>         * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
>>         VECTOR, and NOVECTOR pragmas.
>>         * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
>>         (gfc_match_gcc_novector): New matcher functions handling IVDEP,
>>         VECTOR, and NOVECTOR pragmas.
>>         * match.h: Declare prototypes of matcher functions handling IVDEP,
>>         VECTOR, and NOVECTOR pragmas.
>>         * parse.c (decode_gcc_attribute, parse_do_block)
>>         (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
>>         emit warning for unrecognized pragmas instead of error.
>>         * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
>>         emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
>>         * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.
>>
>> 2019-08-26  Harald Anlauf  <anlauf@gmx.de>
>>
>>         PR fortran/91496
>>         * gfortran.dg/pr91496.f90: New testcase.
>>
>
>
Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(Revision 274964)
+++ gcc/fortran/array.c	(Arbeitskopie)
@@ -2185,6 +2185,9 @@
   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;
 
   return dest;
 }
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(Revision 274964)
+++ gcc/fortran/decl.c	(Arbeitskopie)
@@ -99,6 +99,11 @@
 /* Set upon parsing a !GCC$ unroll n directive for use in the next loop.  */
 int directive_unroll = -1;
 
+/* Set upon parsing supported !GCC$ pragmas for use in the next loop.  */
+bool directive_ivdep = false;
+bool directive_vector = false;
+bool directive_novector = false;
+
 /* Map of middle-end built-ins that should be vectorized.  */
 hash_map<nofree_string_hash, int> *gfc_vectorized_builtins;
 
@@ -11528,3 +11533,53 @@
 
   return MATCH_YES;
 }
+
+/* Match an !GCC$ IVDEP statement.
+   When we come here, we have already matched the !GCC$ IVDEP string.  */
+
+match
+gfc_match_gcc_ivdep (void)
+{
+  if (gfc_match_eos () == MATCH_YES)
+    {
+      directive_ivdep = true;
+      return MATCH_YES;
+    }
+
+  gfc_error ("Syntax error in !GCC$ IVDEP directive at %C");
+  return MATCH_ERROR;
+}
+
+/* Match an !GCC$ VECTOR statement.
+   When we come here, we have already matched the !GCC$ VECTOR string.  */
+
+match
+gfc_match_gcc_vector (void)
+{
+  if (gfc_match_eos () == MATCH_YES)
+    {
+      directive_vector = true;
+      directive_novector = false;
+      return MATCH_YES;
+    }
+
+  gfc_error ("Syntax error in !GCC$ VECTOR directive at %C");
+  return MATCH_ERROR;
+}
+
+/* Match an !GCC$ NOVECTOR statement.
+   When we come here, we have already matched the !GCC$ NOVECTOR string.  */
+
+match
+gfc_match_gcc_novector (void)
+{
+  if (gfc_match_eos () == MATCH_YES)
+    {
+      directive_novector = true;
+      directive_vector = false;
+      return MATCH_YES;
+    }
+
+  gfc_error ("Syntax error in !GCC$ NOVECTOR directive at %C");
+  return MATCH_ERROR;
+}
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(Revision 274964)
+++ gcc/fortran/gfortran.h	(Arbeitskopie)
@@ -2418,6 +2418,9 @@
 {
   gfc_expr *var, *start, *end, *step;
   unsigned short unroll;
+  bool ivdep;
+  bool vector;
+  bool novector;
 }
 gfc_iterator;
 
@@ -2794,6 +2797,9 @@
 bool gfc_in_match_data (void);
 match gfc_match_char_spec (gfc_typespec *);
 extern int directive_unroll;
+extern bool directive_ivdep;
+extern bool directive_vector;
+extern bool directive_novector;
 
 /* SIMD clause enum.  */
 enum gfc_simd_clause
Index: gcc/fortran/gfortran.texi
===================================================================
--- gcc/fortran/gfortran.texi	(Revision 274964)
+++ gcc/fortran/gfortran.texi	(Arbeitskopie)
@@ -3559,6 +3559,9 @@
 * ATTRIBUTES directive::
 * UNROLL directive::
 * BUILTIN directive::
+* IVDEP directive::
+* VECTOR directive::
+* NOVECTOR directive::
 @end menu
 
 @node ATTRIBUTES directive
@@ -3670,6 +3673,52 @@
 The purpose of the directive is to provide an API among the GCC compiler and
 the GNU C Library which would define vector implementations of math routines.
 
+
+@node IVDEP directive
+@subsection IVDEP directive
+
+The syntax of the directive is
+
+@code{!GCC$ ivdep}
+
+This directive tells the compiler to ignore vector dependencies in the
+following loop.  It must be placed immediately before a @code{DO} loop
+and applies only to the loop that follows.
+
+Sometimes the compiler may not have sufficient information to decide
+whether a particular loop is vectorizable due to potential
+dependencies between iterations.  The purpose of the directive is to
+tell the compiler that vectorization is safe.
+
+This directive is intended for annotation of existing code.  For new
+code it is recommended to consider OpenMP SIMD directives as potential
+alternative.
+
+
+@node VECTOR directive
+@subsection VECTOR directive
+
+The syntax of the directive is
+
+@code{!GCC$ vector}
+
+This directive tells the compiler to vectorize the following loop.  It
+must be placed immediately before a @code{DO} loop and applies only to
+the loop that follows.
+
+
+@node NOVECTOR directive
+@subsection NOVECTOR directive
+
+The syntax of the directive is
+
+@code{!GCC$ novector}
+
+This directive tells the compiler to not vectorize the following loop.
+It must be placed immediately before a @code{DO} loop and applies only
+to the loop that follows.
+
+
 @node Non-Fortran Main Program
 @section Non-Fortran Main Program
 
Index: gcc/fortran/match.h
===================================================================
--- gcc/fortran/match.h	(Revision 274964)
+++ gcc/fortran/match.h	(Arbeitskopie)
@@ -246,8 +246,11 @@
 match gfc_match_dimension (void);
 match gfc_match_external (void);
 match gfc_match_gcc_attributes (void);
+match gfc_match_gcc_builtin (void);
+match gfc_match_gcc_ivdep (void);
+match gfc_match_gcc_novector (void);
 match gfc_match_gcc_unroll (void);
-match gfc_match_gcc_builtin (void);
+match gfc_match_gcc_vector (void);
 match gfc_match_import (void);
 match gfc_match_intent (void);
 match gfc_match_intrinsic (void);
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(Revision 274964)
+++ gcc/fortran/parse.c	(Arbeitskopie)
@@ -1079,12 +1079,20 @@
   match ("attributes", gfc_match_gcc_attributes, ST_ATTR_DECL);
   match ("unroll", gfc_match_gcc_unroll, ST_NONE);
   match ("builtin", gfc_match_gcc_builtin, ST_NONE);
+  match ("ivdep", gfc_match_gcc_ivdep, ST_NONE);
+  match ("vector", gfc_match_gcc_vector, ST_NONE);
+  match ("novector", gfc_match_gcc_novector, ST_NONE);
 
   /* All else has failed, so give up.  See if any of the matchers has
      stored an error message of some sort.  */
 
   if (!gfc_error_check ())
-    gfc_error_now ("Unclassifiable GCC directive at %C");
+    {
+      if (pedantic)
+	gfc_error_now ("Unclassifiable GCC directive at %C");
+      else
+	gfc_warning_now (0, "Unclassifiable GCC directive at %C, ignored");
+    }
 
   reject_statement ();
 
@@ -4672,6 +4680,21 @@
 	  new_st.ext.iterator->unroll = directive_unroll;
 	  directive_unroll = -1;
 	}
+      if (directive_ivdep)
+	{
+	  new_st.ext.iterator->ivdep = directive_ivdep;
+	  directive_ivdep = false;
+	}
+      if (directive_vector)
+	{
+	  new_st.ext.iterator->vector = directive_vector;
+	  directive_vector = false;
+	}
+      if (directive_novector)
+	{
+	  new_st.ext.iterator->novector = directive_novector;
+	  directive_novector = false;
+	}
     }
   else
     stree = NULL;
@@ -5433,6 +5456,15 @@
       if (directive_unroll != -1)
 	gfc_error ("%<GCC unroll%> directive does not commence a loop at %C");
 
+      if (directive_ivdep)
+	gfc_error ("%<GCC ivdep%> directive does not commence a loop at %C");
+
+      if (directive_vector)
+	gfc_error ("%<GCC vector%> directive does not commence a loop at %C");
+
+      if (directive_novector)
+	gfc_error ("%<GCC novector%> directive does not commence a loop at %C");
+
       st = next_statement ();
     }
 }
Index: gcc/fortran/trans-stmt.c
===================================================================
--- gcc/fortran/trans-stmt.c	(Revision 274964)
+++ gcc/fortran/trans-stmt.c	(Arbeitskopie)
@@ -2173,6 +2173,19 @@
 		build_int_cst (integer_type_node, annot_expr_unroll_kind),
 		build_int_cst (integer_type_node, code->ext.iterator->unroll));
 
+  if (code->ext.iterator->ivdep && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_ivdep_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->vector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_vector_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->novector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_no_vector_kind),
+		   integer_zero_node);
+
   /* The loop exit.  */
   tmp = fold_build1_loc (loc, GOTO_EXPR, void_type_node, exit_label);
   TREE_USED (exit_label) = 1;
@@ -2503,6 +2516,20 @@
       = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
 		build_int_cst (integer_type_node, annot_expr_unroll_kind),
 		build_int_cst (integer_type_node, code->ext.iterator->unroll));
+
+  if (code->ext.iterator->ivdep && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_ivdep_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->vector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_vector_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->novector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_no_vector_kind),
+		   integer_zero_node);
+
   tmp = fold_build1_loc (loc, GOTO_EXPR, void_type_node, exit_label);
   tmp = fold_build3_loc (loc, COND_EXPR, void_type_node,
 			 cond, tmp, build_empty_stmt (loc));
Bernhard Reutner-Fischer Aug. 28, 2019, 6:57 p.m. UTC | #3
On Tue, 27 Aug 2019 21:49:38 +0200
Harald Anlauf <anlauf@gmx.de> wrote:

> Committed to trunk as svn revision 274966, after removing some
> accidentally left-over unused variable declarations (copy&paste).
> The actual committed version is attached.
> 
> Thanks, Paul, for the quick review!
> 
> Unless there are strong objections, I'd like to commit this to
> 9-branch, too, so that this can be used in the 9.3 release.
> Applies/regtests cleanly.  Will wait for a week or so.

I see that you copied the unfortunate error-message "commence a loop"
and i see that i completely forgot to adjust it as per Mike's
preference in
 https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html

So can you please change your new errors (and the unroll error message
too) to something like the suggested
"directive not at the start of a loop at %C" please?

Many thanks in advance!
PS: This is border obvious, i'd send the patch for review anyway,
maybe a native speaker can provide a better wording.
PPS: I'm still a bit unhappy about the following kludges in unroll even
more so since you copied the concept:
(1) The globals to diagnose misplaced directives are very ugly.
(2) putting the payload into gfc_iterator is ugly, too. I don't
remember offhand how ugly or intrusive it would be to provide means for
passing down an additional optional structure to act as sink for the
directive payload data. Putting those into the iterator is AFAIR not
all that clean. Maybe you could have a look if you can extend this area
to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
seldomly used anyway and hence we don't care?

thanks and cheers,

> 
> Harald
> 
> On 08/27/19 10:33, Paul Richard Thomas wrote:
> > Hi Harald,
> >
> > This is OK for trunk.
> >
> > Thanks!
> >
> > Paul
> >
> > On Mon, 26 Aug 2019 at 22:13, Harald Anlauf <anlauf@gmx.de> wrote:  
> >>
> >> Dear all,
> >>
> >> the attached patch adds Fortran support for the following pragmas
> >> (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
> >> NOVECTOR.  Furthermore, it downgrades unsupported directives from
> >> error to warning (by default, it stays an error with -pedantic),
> >> thus fixing the PR.
> >>
> >> It has no effect on existing code (thus regtested cleanly on
> >> x86_64-pc-linux-gnu), but gives users an option for fine-grained
> >> control of optimization.  The above pragmas are supported by other
> >> compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
> >> sometimes with slightly different keywords).
> >>
> >> OK for trunk, and backport to 9?
> >>
> >> Thanks,
> >> Harald
> >>
> >>
> >> 2019-08-26  Harald Anlauf  <anlauf@gmx.de>
> >>
> >>         PR fortran/91496
> >>         * gfortran.h: Extend struct gfc_iterator for loop annotations.
> >>         * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
> >>         VECTOR, and NOVECTOR pragmas.
> >>         * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
> >>         (gfc_match_gcc_novector): New matcher functions handling IVDEP,
> >>         VECTOR, and NOVECTOR pragmas.
> >>         * match.h: Declare prototypes of matcher functions handling IVDEP,
> >>         VECTOR, and NOVECTOR pragmas.
> >>         * parse.c (decode_gcc_attribute, parse_do_block)
> >>         (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
> >>         emit warning for unrecognized pragmas instead of error.
> >>         * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
> >>         emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
> >>         * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.
> >>
> >> 2019-08-26  Harald Anlauf  <anlauf@gmx.de>
> >>
> >>         PR fortran/91496
> >>         * gfortran.dg/pr91496.f90: New testcase.
> >>  
> >
> >  
>
Harald Anlauf Aug. 28, 2019, 7:58 p.m. UTC | #4
Hi Bernhard,

On 08/28/19 20:57, Bernhard Reutner-Fischer wrote:
> I see that you copied the unfortunate error-message "commence a loop"
> and i see that i completely forgot to adjust it as per Mike's
> preference in
>  https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html
>
> So can you please change your new errors (and the unroll error message
> too) to something like the suggested
> "directive not at the start of a loop at %C" please?
>
> Many thanks in advance!
> PS: This is border obvious, i'd send the patch for review anyway,
> maybe a native speaker can provide a better wording.

see attachment and below for Changelog.  Regtested OK.

I have opted for the variant "directive not at the start of a loop",
which is what Mike preferred, and what matches the current capabilities.

Is that OK?

> PPS: I'm still a bit unhappy about the following kludges in unroll even
> more so since you copied the concept:
> (1) The globals to diagnose misplaced directives are very ugly.
> (2) putting the payload into gfc_iterator is ugly, too. I don't
> remember offhand how ugly or intrusive it would be to provide means for
> passing down an additional optional structure to act as sink for the
> directive payload data. Putting those into the iterator is AFAIR not
> all that clean. Maybe you could have a look if you can extend this area
> to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
> seldomly used anyway and hence we don't care?

Could you please open a new PR for this?

There are many things that could be improved in this context.
There is a lot that could be learned from e.g. the Cray or NEC
compilers, where you can explicitly control loop fusion, loop
blocking, which are very important in (my) practice.

The current implementation also does not support array notation,
which I consider a major limitation.  I'll have to learn how this
could be done.

> thanks and cheers,
>

Thanks for the feedback!

Harald

2019-08-28  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/91496
	* parse.c (parse_executable): Improve error messages for
	improperly placed pragmas not preceeding a loop.

2019-08-28  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/91496
	* gfortran.dg/directive_unroll_5.f90: Adjust error message.
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 274998)
+++ gcc/fortran/parse.c	(working copy)
@@ -5454,16 +5454,17 @@
 	}

       if (directive_unroll != -1)
-	gfc_error ("%<GCC unroll%> directive does not commence a loop at %C");
+	gfc_error ("%<GCC unroll%> directive not at the start of a loop at %C");

       if (directive_ivdep)
-	gfc_error ("%<GCC ivdep%> directive does not commence a loop at %C");
+	gfc_error ("%<GCC ivdep%> directive not at the start of a loop at %C");

       if (directive_vector)
-	gfc_error ("%<GCC vector%> directive does not commence a loop at %C");
+	gfc_error ("%<GCC vector%> directive not at the start of a loop at %C");

       if (directive_novector)
-	gfc_error ("%<GCC novector%> directive does not commence a loop at %C");
+	gfc_error ("%<GCC novector%> "
+		   "directive not at the start of a loop at %C");

       st = next_statement ();
     }
Index: gcc/testsuite/gfortran.dg/directive_unroll_5.f90
===================================================================
--- gcc/testsuite/gfortran.dg/directive_unroll_5.f90	(revision 274998)
+++ gcc/testsuite/gfortran.dg/directive_unroll_5.f90	(working copy)
@@ -31,7 +31,7 @@
   integer :: a(n), b(n)
   integer (kind=4) :: i
 !GCC$ unroll 8
-  write (*,*) "wrong"! { dg-error "directive does not commence a loop" }
+  write (*,*) "wrong"! { dg-error "directive not at the start of a loop" }
   DO i=n, 1, -1
     call dummy2(a(i), b(i), i)
   ENDDO
Harald Anlauf Sept. 5, 2019, 7:55 p.m. UTC | #5
Ping!

On 08/28/19 21:58, Harald Anlauf wrote:
> Hi Bernhard,
>
> On 08/28/19 20:57, Bernhard Reutner-Fischer wrote:
>> I see that you copied the unfortunate error-message "commence a loop"
>> and i see that i completely forgot to adjust it as per Mike's
>> preference in
>>  https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html
>>
>> So can you please change your new errors (and the unroll error message
>> too) to something like the suggested
>> "directive not at the start of a loop at %C" please?
>>
>> Many thanks in advance!
>> PS: This is border obvious, i'd send the patch for review anyway,
>> maybe a native speaker can provide a better wording.
>
> see attachment and below for Changelog.  Regtested OK.
>
> I have opted for the variant "directive not at the start of a loop",
> which is what Mike preferred, and what matches the current capabilities.
>
> Is that OK?
>
>> PPS: I'm still a bit unhappy about the following kludges in unroll even
>> more so since you copied the concept:
>> (1) The globals to diagnose misplaced directives are very ugly.
>> (2) putting the payload into gfc_iterator is ugly, too. I don't
>> remember offhand how ugly or intrusive it would be to provide means for
>> passing down an additional optional structure to act as sink for the
>> directive payload data. Putting those into the iterator is AFAIR not
>> all that clean. Maybe you could have a look if you can extend this area
>> to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
>> seldomly used anyway and hence we don't care?
>
> Could you please open a new PR for this?
>
> There are many things that could be improved in this context.
> There is a lot that could be learned from e.g. the Cray or NEC
> compilers, where you can explicitly control loop fusion, loop
> blocking, which are very important in (my) practice.
>
> The current implementation also does not support array notation,
> which I consider a major limitation.  I'll have to learn how this
> could be done.
>
>> thanks and cheers,
>>
>
> Thanks for the feedback!
>
> Harald
>
> 2019-08-28  Harald Anlauf  <anlauf@gmx.de>
>
> 	PR fortran/91496
> 	* parse.c (parse_executable): Improve error messages for
> 	improperly placed pragmas not preceeding a loop.
>
> 2019-08-28  Harald Anlauf  <anlauf@gmx.de>
>
> 	PR fortran/91496
> 	* gfortran.dg/directive_unroll_5.f90: Adjust error message.
>
Steve Kargl Sept. 5, 2019, 8:05 p.m. UTC | #6
Looks like a straight forward mechanical change.
Probably doesn't need a review.

OK.
diff mbox series

Patch

Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 274933)
+++ gcc/fortran/array.c	(working copy)
@@ -2185,6 +2185,9 @@ 
   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;

   return dest;
 }
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(revision 274933)
+++ gcc/fortran/decl.c	(working copy)
@@ -99,6 +99,11 @@ 
 /* Set upon parsing a !GCC$ unroll n directive for use in the next loop.  */
 int directive_unroll = -1;

+/* Set upon parsing supported !GCC$ pragmas for use in the next loop.  */
+bool directive_ivdep = false;
+bool directive_vector = false;
+bool directive_novector = false;
+
 /* Map of middle-end built-ins that should be vectorized.  */
 hash_map<nofree_string_hash, int> *gfc_vectorized_builtins;

@@ -11528,3 +11533,57 @@ 

   return MATCH_YES;
 }
+
+/* Match an !GCC$ IVDEP statement.
+   When we come here, we have already matched the !GCC$ IVDEP string.  */
+
+match
+gfc_match_gcc_ivdep (void)
+{
+  int value;
+
+  if (gfc_match_eos () == MATCH_YES)
+    {
+      directive_ivdep = true;
+      return MATCH_YES;
+    }
+
+  gfc_error ("Syntax error in !GCC$ IVDEP directive at %C");
+  return MATCH_ERROR;
+}
+
+/* Match an !GCC$ VECTOR statement.
+   When we come here, we have already matched the !GCC$ VECTOR string.  */
+
+match
+gfc_match_gcc_vector (void)
+{
+  int value;
+
+  if (gfc_match_eos () == MATCH_YES)
+    {
+      directive_vector = true;
+      return MATCH_YES;
+    }
+
+  gfc_error ("Syntax error in !GCC$ VECTOR directive at %C");
+  return MATCH_ERROR;
+}
+
+/* Match an !GCC$ NOVECTOR statement.
+   When we come here, we have already matched the !GCC$ NOVECTOR string.  */
+
+match
+gfc_match_gcc_novector (void)
+{
+  int value;
+
+  if (gfc_match_eos () == MATCH_YES)
+    {
+      directive_novector = true;
+      return MATCH_YES;
+    }
+
+  gfc_error ("Syntax error in !GCC$ NOVECTOR directive at %C");
+  return MATCH_ERROR;
+}
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 274933)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -2418,6 +2418,9 @@ 
 {
   gfc_expr *var, *start, *end, *step;
   unsigned short unroll;
+  bool ivdep;
+  bool vector;
+  bool novector;
 }
 gfc_iterator;

@@ -2794,6 +2797,9 @@ 
 bool gfc_in_match_data (void);
 match gfc_match_char_spec (gfc_typespec *);
 extern int directive_unroll;
+extern bool directive_ivdep;
+extern bool directive_vector;
+extern bool directive_novector;

 /* SIMD clause enum.  */
 enum gfc_simd_clause
Index: gcc/fortran/gfortran.texi
===================================================================
--- gcc/fortran/gfortran.texi	(revision 274933)
+++ gcc/fortran/gfortran.texi	(working copy)
@@ -3559,6 +3559,9 @@ 
 * ATTRIBUTES directive::
 * UNROLL directive::
 * BUILTIN directive::
+* IVDEP directive::
+* VECTOR directive::
+* NOVECTOR directive::
 @end menu

 @node ATTRIBUTES directive
@@ -3670,6 +3673,52 @@ 
 The purpose of the directive is to provide an API among the GCC compiler and
 the GNU C Library which would define vector implementations of math routines.

+
+@node IVDEP directive
+@subsection IVDEP directive
+
+The syntax of the directive is
+
+@code{!GCC$ ivdep}
+
+This directive tells the compiler to ignore vector dependencies in the
+following loop.  It must be placed immediately before a @code{DO} loop
+and applies only to the loop that follows.
+
+Sometimes the compiler may not have sufficient information to decide
+whether a particular loop is vectorizable due to potential
+dependencies between iterations.  The purpose of the directive is to
+tell the compiler that vectorization is safe.
+
+This directive is intended for annotation of existing code.  For new
+code it is recommended to consider OpenMP SIMD directives as potential
+alternative.
+
+
+@node VECTOR directive
+@subsection VECTOR directive
+
+The syntax of the directive is
+
+@code{!GCC$ vector}
+
+This directive tells the compiler to vectorize the following loop.  It
+must be placed immediately before a @code{DO} loop and applies only to
+the loop that follows.
+
+
+@node NOVECTOR directive
+@subsection NOVECTOR directive
+
+The syntax of the directive is
+
+@code{!GCC$ novector}
+
+This directive tells the compiler to not vectorize the following loop.
+It must be placed immediately before a @code{DO} loop and applies only
+to the loop that follows.
+
+
 @node Non-Fortran Main Program
 @section Non-Fortran Main Program

Index: gcc/fortran/match.h
===================================================================
--- gcc/fortran/match.h	(revision 274933)
+++ gcc/fortran/match.h	(working copy)
@@ -246,8 +246,11 @@ 
 match gfc_match_dimension (void);
 match gfc_match_external (void);
 match gfc_match_gcc_attributes (void);
+match gfc_match_gcc_builtin (void);
+match gfc_match_gcc_ivdep (void);
+match gfc_match_gcc_novector (void);
 match gfc_match_gcc_unroll (void);
-match gfc_match_gcc_builtin (void);
+match gfc_match_gcc_vector (void);
 match gfc_match_import (void);
 match gfc_match_intent (void);
 match gfc_match_intrinsic (void);
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 274933)
+++ gcc/fortran/parse.c	(working copy)
@@ -1079,12 +1079,20 @@ 
   match ("attributes", gfc_match_gcc_attributes, ST_ATTR_DECL);
   match ("unroll", gfc_match_gcc_unroll, ST_NONE);
   match ("builtin", gfc_match_gcc_builtin, ST_NONE);
+  match ("ivdep", gfc_match_gcc_ivdep, ST_NONE);
+  match ("vector", gfc_match_gcc_vector, ST_NONE);
+  match ("novector", gfc_match_gcc_novector, ST_NONE);

   /* All else has failed, so give up.  See if any of the matchers has
      stored an error message of some sort.  */

   if (!gfc_error_check ())
-    gfc_error_now ("Unclassifiable GCC directive at %C");
+    {
+      if (pedantic)
+	gfc_error_now ("Unclassifiable GCC directive at %C");
+      else
+	gfc_warning_now (0, "Unclassifiable GCC directive at %C, ignored");
+    }

   reject_statement ();

@@ -4672,6 +4680,21 @@ 
 	  new_st.ext.iterator->unroll = directive_unroll;
 	  directive_unroll = -1;
 	}
+      if (directive_ivdep)
+	{
+	  new_st.ext.iterator->ivdep = directive_ivdep;
+	  directive_ivdep = false;
+	}
+      if (directive_vector)
+	{
+	  new_st.ext.iterator->vector = directive_vector;
+	  directive_vector = false;
+	}
+      if (directive_novector)
+	{
+	  new_st.ext.iterator->novector = directive_novector;
+	  directive_novector = false;
+	}
     }
   else
     stree = NULL;
@@ -5433,6 +5456,15 @@ 
       if (directive_unroll != -1)
 	gfc_error ("%<GCC unroll%> directive does not commence a loop at %C");

+      if (directive_ivdep)
+	gfc_error ("%<GCC ivdep%> directive does not commence a loop at %C");
+
+      if (directive_vector)
+	gfc_error ("%<GCC vector%> directive does not commence a loop at %C");
+
+      if (directive_novector)
+	gfc_error ("%<GCC novector%> directive does not commence a loop at %C");
+
       st = next_statement ();
     }
 }
Index: gcc/fortran/trans-stmt.c
===================================================================
--- gcc/fortran/trans-stmt.c	(revision 274933)
+++ gcc/fortran/trans-stmt.c	(working copy)
@@ -2173,6 +2173,19 @@ 
 		build_int_cst (integer_type_node, annot_expr_unroll_kind),
 		build_int_cst (integer_type_node, code->ext.iterator->unroll));

+  if (code->ext.iterator->ivdep && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_ivdep_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->vector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_vector_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->novector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_no_vector_kind),
+		   integer_zero_node);
+
   /* The loop exit.  */
   tmp = fold_build1_loc (loc, GOTO_EXPR, void_type_node, exit_label);
   TREE_USED (exit_label) = 1;
@@ -2503,6 +2516,20 @@ 
       = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
 		build_int_cst (integer_type_node, annot_expr_unroll_kind),
 		build_int_cst (integer_type_node, code->ext.iterator->unroll));
+
+  if (code->ext.iterator->ivdep && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_ivdep_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->vector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_vector_kind),
+		   integer_zero_node);
+  if (code->ext.iterator->novector && cond != error_mark_node)
+    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
+		   build_int_cst (integer_type_node, annot_expr_no_vector_kind),
+		   integer_zero_node);
+
   tmp = fold_build1_loc (loc, GOTO_EXPR, void_type_node, exit_label);
   tmp = fold_build3_loc (loc, COND_EXPR, void_type_node,
 			 cond, tmp, build_empty_stmt (loc));