Re: [openacc] Teach gfortran to lower OpenACC routine dims

2018-09-25 Thread Bernhard Reutner-Fischer
On 24 September 2018 16:45:38 CEST, Cesar Philippidis  
wrote:

>I updated the patch by incorporating all of those changes. Is it OK for
>trunk?

LGTM but I cannot approve it.

thanks,


Re: [openacc] Teach gfortran to lower OpenACC routine dims

2018-09-24 Thread Cesar Philippidis
On 09/20/2018 09:10 AM, Bernhard Reutner-Fischer wrote:
> On Thu, 20 Sep 2018 07:41:08 -0700
> Cesar Philippidis  wrote:
> 
>> On 09/19/2018 03:27 PM, Bernhard Reutner-Fischer wrote:
>>> On Wed, 5 Sep 2018 12:52:03 -0700
>>> Cesar Philippidis  wrote:
> 
 diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
 index eea6b81ebfa..eed868f475b 100644
 --- a/gcc/fortran/trans-decl.c
 +++ b/gcc/fortran/trans-decl.c
 @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not
 see #include "trans-stmt.h"
  #include "gomp-constants.h"
  #include "gimplify.h"
 +#include "omp-general.h"  
>>>
>>> hmz. so the gomp-constants.h include would be redundant, but do we
>>> really need omp-general.h?  
>>
>> Good point. omp-general.h is required for oacc_build_routine_dims.
>>
>>> Doesn't this suggest to move this oacc dims lowering to
>>> trans-openmp.c instead, please?  
>>
>> So something like adding a new gfc_add_omp_offload_attributes to
>> trans-openmp.c and call it from add_attributes_to_decl?
> 
> yes.
> 
>> On a related note, I noticed that I forgot to incorporate this change
>> in gfortran.h:
>>
>> @@ -902,7 +912,7 @@ typedef struct
>>unsigned oacc_declare_link:1;
>>
>>/* This is an OpenACC acclerator function at level N - 1  */
>> -  unsigned oacc_function:3;
>> +  ENUM_BITFIELD (oacc_function) oacc_function:3;
>>
>> It's probably not huge, but I noticed that some other enum bitfields
>> are declared that way.
> 
> yea, some compilers had trouble with enum bitfields (where plain int
> bitfields like here worked fine, IIRC) but i'm not sure if it's
> considered legacy these days. Fine with me to be safe.

I updated the patch by incorporating all of those changes. Is it OK for
trunk?

Thanks,
Cesar
[openacc] Make GFC default to -1 for OpenACC routine dims

2018-09-24  Cesar Philippidis  

	* gfortran.h (enum oacc_function): New enum.
	(gfc_oacc_routine_name): Add locus loc field.
	(symbol_attribute): Update type of oacc_function field.	
	* openmp.c (gfc_oacc_routine_dims): Return oacc_function.
	(gfc_match_oacc_routine): Update routine clause syntax checking.
	Populate oacc_function attribute with dims.
	* trans-decl.c (add_attributes_to_decl): Use oacc_build_routine_dims
	to construct routine dims.
	* trans.h (gfc_add_omp_offload_attributes): Declare.
	* trans-decl.c (add_attributes_to_decl): Use it to set OMP and ACC
	offload function attributes.
	* trans-openmp.c (gfc_add_omp_offload_attributes): New function.

	gcc/testsuite/
	* gfortran.dg/goacc/classify-routine.f95: Adjust test.
	* gfortran.dg/goacc/pr71704.f90: Likewise.
	* gfortran.dg/goacc/routine-6.f90: Likewise.
	* gfortran.dg/goacc/routine-8.f90: Likewise.
	* gfortran.dg/goacc/routine-level-of-parallelism-1.f90: Likewise.

	libgomp/
	* testsuite/libgomp.oacc-fortran/routine-1.f90: Adjust test.
	* testsuite/libgomp.oacc-fortran/routine-2.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-3.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-4.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-5.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-7.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-9.f90: Likewise.
	* libgomp.oacc-fortran/host_data-2.f90: Likewise.
	* libgomp.oacc-fortran/host_data-3.f: Likewise.
	* libgomp.oacc-fortran/host_data-4.f90: Likewise.


diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 04b0024a992..3efd59c95f7 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -316,6 +316,16 @@ enum save_state
 { SAVE_NONE = 0, SAVE_EXPLICIT, SAVE_IMPLICIT
 };
 
+/* Flags to keep track of ACC routine states.  */
+enum oacc_function
+{ OACC_FUNCTION_NONE = 0,
+  OACC_FUNCTION_GANG,
+  OACC_FUNCTION_WORKER,
+  OACC_FUNCTION_VECTOR,
+  OACC_FUNCTION_SEQ,
+  OACC_FUNCTION_AUTO
+};
+
 /* Strings for all symbol attributes.  We use these for dumping the
parse tree, in error messages, and also when reading and writing
modules.  In symbol.c.  */
@@ -902,7 +912,7 @@ typedef struct
   unsigned oacc_declare_link:1;
 
   /* This is an OpenACC acclerator function at level N - 1  */
-  unsigned oacc_function:3;
+  ENUM_BITFIELD (oacc_function) oacc_function:3;
 
   /* Attributes set by compiler extensions (!GCC$ ATTRIBUTES).  */
   unsigned ext_attr:EXT_ATTR_NUM;
@@ -1726,6 +1736,7 @@ typedef struct gfc_oacc_routine_name
   struct gfc_symbol *sym;
   struct gfc_omp_clauses *clauses;
   struct gfc_oacc_routine_name *next;
+  locus loc;
 }
 gfc_oacc_routine_name;
 
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 94a7f7eaa50..ac1923ea06b 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2234,34 +2234,45 @@ gfc_match_oacc_cache (void)
   return MATCH_YES;
 }
 
-/* Determine the loop level for a routine.   */
+/* Determine the loop level for a routine.  Returns OACC_FUNCTION_NONE
+   if any error is detected.  */
 
-static int
+static oacc_function
 gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
 {
   

Re: [openacc] Teach gfortran to lower OpenACC routine dims

2018-09-20 Thread Bernhard Reutner-Fischer
On Thu, 20 Sep 2018 07:41:08 -0700
Cesar Philippidis  wrote:

> On 09/19/2018 03:27 PM, Bernhard Reutner-Fischer wrote:
> > On Wed, 5 Sep 2018 12:52:03 -0700
> > Cesar Philippidis  wrote:

> >> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
> >> index eea6b81ebfa..eed868f475b 100644
> >> --- a/gcc/fortran/trans-decl.c
> >> +++ b/gcc/fortran/trans-decl.c
> >> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not
> >> see #include "trans-stmt.h"
> >>  #include "gomp-constants.h"
> >>  #include "gimplify.h"
> >> +#include "omp-general.h"  
> > 
> > hmz. so the gomp-constants.h include would be redundant, but do we
> > really need omp-general.h?  
> 
> Good point. omp-general.h is required for oacc_build_routine_dims.
> 
> > Doesn't this suggest to move this oacc dims lowering to
> > trans-openmp.c instead, please?  
> 
> So something like adding a new gfc_add_omp_offload_attributes to
> trans-openmp.c and call it from add_attributes_to_decl?

yes.

> On a related note, I noticed that I forgot to incorporate this change
> in gfortran.h:
> 
> @@ -902,7 +912,7 @@ typedef struct
>unsigned oacc_declare_link:1;
> 
>/* This is an OpenACC acclerator function at level N - 1  */
> -  unsigned oacc_function:3;
> +  ENUM_BITFIELD (oacc_function) oacc_function:3;
> 
> It's probably not huge, but I noticed that some other enum bitfields
> are declared that way.

yea, some compilers had trouble with enum bitfields (where plain int
bitfields like here worked fine, IIRC) but i'm not sure if it's
considered legacy these days. Fine with me to be safe.

> 
> > btw.. the OACC merge from the gomp4 branch added a copy'n paste
> > error in an error message. May i ask you to regtest and install the
> > below:

> Sure. That looks reasonable. I'll also update and/or add new tests as
> necessary.

TIA and cheers,


Re: [openacc] Teach gfortran to lower OpenACC routine dims

2018-09-20 Thread Cesar Philippidis
On 09/19/2018 03:27 PM, Bernhard Reutner-Fischer wrote:
> On Wed, 5 Sep 2018 12:52:03 -0700
> Cesar Philippidis  wrote:
> 
>> At present, gfortran does not encode the gang, worker or vector
>> parallelism clauses when it creates acc routines dim attribute for
>> subroutines and functions. While support for acc routine is lacking in
>> other areas in gfortran (including modules), this patch is important
>> because it encodes the parallelism attributes using the same function
>> as the C and C++ FEs. This will become important with the forthcoming
>> nvptx vector length extensions, because large vectors are not
>> supported in acc routines yet.
>>
>> Is this OK for trunk? I regtested and bootstrapped for x86_64 with
>> nvptx offloading.
> 
>> diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
>> index 94a7f7eaa50..d48c9351e25 100644
>> --- a/gcc/fortran/openmp.c
>> +++ b/gcc/fortran/openmp.c
>> @@ -2234,34 +2234,45 @@ gfc_match_oacc_cache (void)
>>return MATCH_YES;
>>  }
>>  
>> -/* Determine the loop level for a routine.   */
>> +/* Determine the loop level for a routine.  Returns
>> OACC_FUNCTION_NONE
>> +   if any error is detected.  */
>>  
>> -static int
>> +static oacc_function
>>  gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
>>  {
>>int level = -1;
>> +  oacc_function ret = OACC_FUNCTION_AUTO;
>>  
>>if (clauses)
>>  {
>>unsigned mask = 0;
>>  
>>if (clauses->gang)
>> -level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
>> +{
>> +  level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
>> +  ret = OACC_FUNCTION_GANG;
>> +}
>>if (clauses->worker)
>> -level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
>> +{
>> +  level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
>> +  ret = OACC_FUNCTION_WORKER;
>> +}
>>if (clauses->vector)
>> -level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
>> +{
>> +  level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
>> +  ret = OACC_FUNCTION_VECTOR;
>> +}
>>if (clauses->seq)
>> -level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
>> +{
>> +  level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
>> +  ret = OACC_FUNCTION_SEQ;
>> +}
>>  
>>if (mask != (mask & -mask))
>> -gfc_error ("Multiple loop axes specified for routine");
>> +ret = OACC_FUNCTION_NONE;
>>  }
>>  
>> -  if (level < 0)
>> -level = GOMP_DIM_MAX;
>> -
>> -  return level;
>> +  return ret;
>>  }
>>  
>>  match
>> @@ -2272,6 +2283,8 @@ gfc_match_oacc_routine (void)
>>match m;
>>gfc_omp_clauses *c = NULL;
>>gfc_oacc_routine_name *n = NULL;
>> +  oacc_function dims = OACC_FUNCTION_NONE;
> 
> Unneeded initialisation of dims.

ACK.

>> +  bool seen_error = false;
>>  
>>old_loc = gfc_current_locus;
>>  
>> @@ -2318,17 +2331,15 @@ gfc_match_oacc_routine (void)
>>  }
>>else
>>  {
>> -  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %C");
>> -  gfc_current_locus = old_loc;
>> -  return MATCH_ERROR;
>> +  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %L",
>> _loc);
>> +  goto cleanup;
>>  }
>>  
>>if (gfc_match_char (')') != MATCH_YES)
>>  {
>> -  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %C,
>> expecting"
>> - " ')' after NAME");
>> -  gfc_current_locus = old_loc;
>> -  return MATCH_ERROR;
>> +  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %L,
>> expecting"
>> + " ')' after NAME", _loc);
>> +  goto cleanup;
>>  }
>>  }
>>  
>> @@ -2337,26 +2348,83 @@ gfc_match_oacc_routine (void)
>>!= MATCH_YES))
>>  return MATCH_ERROR;
>>  
>> +  /* Scan for invalid routine geometry.  */
>> +  dims = gfc_oacc_routine_dims (c);
>> +  if (dims == OACC_FUNCTION_NONE)
>> +{
>> +  gfc_error ("Multiple loop axes specified in !$ACC ROUTINE at
>> %L",
>> + _loc);
>> +
>> +  /* Don't abort early, because it's important to let the user
>> + know of any potential duplicate routine directives.  */
>> +  seen_error = true;
>> +}
>> +  else if (dims == OACC_FUNCTION_AUTO)
>> +{
>> +  gfc_warning (0, "Expected one of %, %,
>> % or "
>> +   "% clauses in !$ACC ROUTINE at %L",
>> _loc);
>> +  dims = OACC_FUNCTION_SEQ;
>> +}
>> +
>>if (sym != NULL)
>>  {
>> -  n = gfc_get_oacc_routine_name ();
>> -  n->sym = sym;
>> -  n->clauses = NULL;
>> -  n->next = NULL;
>> -  if (gfc_current_ns->oacc_routine_names != NULL)
>> -n->next = gfc_current_ns->oacc_routine_names;
>> -
>> -  gfc_current_ns->oacc_routine_names = n;
>> +  bool needs_entry = true;
>> +
>> +  /* Scan for any repeated routine directives on 'sym' and report
>> + an error if necessary.  TODO: Extend this function to scan
>> + for compatible DEVICE_TYPE dims.  */
>> +  for (n = gfc_current_ns->oacc_routine_names; n; n 

Re: [openacc] Teach gfortran to lower OpenACC routine dims

2018-09-19 Thread Bernhard Reutner-Fischer
On Wed, 5 Sep 2018 12:52:03 -0700
Cesar Philippidis  wrote:

> At present, gfortran does not encode the gang, worker or vector
> parallelism clauses when it creates acc routines dim attribute for
> subroutines and functions. While support for acc routine is lacking in
> other areas in gfortran (including modules), this patch is important
> because it encodes the parallelism attributes using the same function
> as the C and C++ FEs. This will become important with the forthcoming
> nvptx vector length extensions, because large vectors are not
> supported in acc routines yet.
> 
> Is this OK for trunk? I regtested and bootstrapped for x86_64 with
> nvptx offloading.

> diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
> index 94a7f7eaa50..d48c9351e25 100644
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -2234,34 +2234,45 @@ gfc_match_oacc_cache (void)
>return MATCH_YES;
>  }
>  
> -/* Determine the loop level for a routine.   */
> +/* Determine the loop level for a routine.  Returns
> OACC_FUNCTION_NONE
> +   if any error is detected.  */
>  
> -static int
> +static oacc_function
>  gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
>  {
>int level = -1;
> +  oacc_function ret = OACC_FUNCTION_AUTO;
>  
>if (clauses)
>  {
>unsigned mask = 0;
>  
>if (clauses->gang)
> - level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
> + {
> +   level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
> +   ret = OACC_FUNCTION_GANG;
> + }
>if (clauses->worker)
> - level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
> + {
> +   level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
> +   ret = OACC_FUNCTION_WORKER;
> + }
>if (clauses->vector)
> - level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
> + {
> +   level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
> +   ret = OACC_FUNCTION_VECTOR;
> + }
>if (clauses->seq)
> - level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
> + {
> +   level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
> +   ret = OACC_FUNCTION_SEQ;
> + }
>  
>if (mask != (mask & -mask))
> - gfc_error ("Multiple loop axes specified for routine");
> + ret = OACC_FUNCTION_NONE;
>  }
>  
> -  if (level < 0)
> -level = GOMP_DIM_MAX;
> -
> -  return level;
> +  return ret;
>  }
>  
>  match
> @@ -2272,6 +2283,8 @@ gfc_match_oacc_routine (void)
>match m;
>gfc_omp_clauses *c = NULL;
>gfc_oacc_routine_name *n = NULL;
> +  oacc_function dims = OACC_FUNCTION_NONE;

Unneeded initialisation of dims.

> +  bool seen_error = false;
>  
>old_loc = gfc_current_locus;
>  
> @@ -2318,17 +2331,15 @@ gfc_match_oacc_routine (void)
>   }
>else
>  {
> -   gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %C");
> -   gfc_current_locus = old_loc;
> -   return MATCH_ERROR;
> +   gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %L",
> _loc);
> +   goto cleanup;
>   }
>  
>if (gfc_match_char (')') != MATCH_YES)
>   {
> -   gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %C,
> expecting"
> -  " ')' after NAME");
> -   gfc_current_locus = old_loc;
> -   return MATCH_ERROR;
> +   gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %L,
> expecting"
> +  " ')' after NAME", _loc);
> +   goto cleanup;
>   }
>  }
>  
> @@ -2337,26 +2348,83 @@ gfc_match_oacc_routine (void)
> != MATCH_YES))
>  return MATCH_ERROR;
>  
> +  /* Scan for invalid routine geometry.  */
> +  dims = gfc_oacc_routine_dims (c);
> +  if (dims == OACC_FUNCTION_NONE)
> +{
> +  gfc_error ("Multiple loop axes specified in !$ACC ROUTINE at
> %L",
> +  _loc);
> +
> +  /* Don't abort early, because it's important to let the user
> +  know of any potential duplicate routine directives.  */
> +  seen_error = true;
> +}
> +  else if (dims == OACC_FUNCTION_AUTO)
> +{
> +  gfc_warning (0, "Expected one of %, %,
> % or "
> +"% clauses in !$ACC ROUTINE at %L",
> _loc);
> +  dims = OACC_FUNCTION_SEQ;
> +}
> +
>if (sym != NULL)
>  {
> -  n = gfc_get_oacc_routine_name ();
> -  n->sym = sym;
> -  n->clauses = NULL;
> -  n->next = NULL;
> -  if (gfc_current_ns->oacc_routine_names != NULL)
> - n->next = gfc_current_ns->oacc_routine_names;
> -
> -  gfc_current_ns->oacc_routine_names = n;
> +  bool needs_entry = true;
> +
> +  /* Scan for any repeated routine directives on 'sym' and report
> +  an error if necessary.  TODO: Extend this function to scan
> +  for compatible DEVICE_TYPE dims.  */
> +  for (n = gfc_current_ns->oacc_routine_names; n; n = n->next)
> + if (n->sym == sym)
> +   {
> + needs_entry = false;
> + if (dims != gfc_oacc_routine_dims (n->clauses))
> +   {
> + 

[openacc] Teach gfortran to lower OpenACC routine dims

2018-09-05 Thread Cesar Philippidis
At present, gfortran does not encode the gang, worker or vector
parallelism clauses when it creates acc routines dim attribute for
subroutines and functions. While support for acc routine is lacking in
other areas in gfortran (including modules), this patch is important
because it encodes the parallelism attributes using the same function as
the C and C++ FEs. This will become important with the forthcoming nvptx
vector length extensions, because large vectors are not supported in acc
routines yet.

Is this OK for trunk? I regtested and bootstrapped for x86_64 with nvptx
offloading.

Thanks,
Cesar
[openacc] Teach gfortran to lower OpenACC routine dims

	gcc/fortran/
	* gfortran.h (oacc_function): New enum.
	(gfc_oacc_routine_name): Add locus loc field.
	* openmp.c (gfc_oacc_routine_dims): Return oacc_function.
	(gfc_match_oacc_routine): Update routine clause syntax checking.
	Populate oacc_function attribute with dims.
	* trans-decl.c (add_attributes_to_decl): Use oacc_build_routine_dims
	to construct routine dims.

	gcc/testsuite/
	* gfortran.dg/goacc/classify-routine.f95: Adjust test.
	* gfortran.dg/goacc/pr71704.f90: Likewise.
	* gfortran.dg/goacc/routine-6.f90: Likewise.
	* gfortran.dg/goacc/routine-8.f90: Likewise.
	* gfortran.dg/goacc/routine-level-of-parallelism-1.f90: Likewise.

	libgomp/
	* testsuite/libgomp.oacc-fortran/routine-1.f90: Adjust test.
	* testsuite/libgomp.oacc-fortran/routine-2.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-3.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-4.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-5.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-7.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/routine-9.f90: Likewise.
	* libgomp.oacc-fortran/host_data-2.f90: Likewise.
	* libgomp.oacc-fortran/host_data-3.f: Likewise.
	* libgomp.oacc-fortran/host_data-4.f90: Likewise.


diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 04b0024a992..3675f2e8d52 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -316,6 +316,16 @@ enum save_state
 { SAVE_NONE = 0, SAVE_EXPLICIT, SAVE_IMPLICIT
 };
 
+/* Flags to keep track of ACC routine states.  */
+enum oacc_function
+{ OACC_FUNCTION_NONE = 0,
+  OACC_FUNCTION_GANG,
+  OACC_FUNCTION_WORKER,
+  OACC_FUNCTION_VECTOR,
+  OACC_FUNCTION_SEQ,
+  OACC_FUNCTION_AUTO
+};
+
 /* Strings for all symbol attributes.  We use these for dumping the
parse tree, in error messages, and also when reading and writing
modules.  In symbol.c.  */
@@ -1726,6 +1736,7 @@ typedef struct gfc_oacc_routine_name
   struct gfc_symbol *sym;
   struct gfc_omp_clauses *clauses;
   struct gfc_oacc_routine_name *next;
+  locus loc;
 }
 gfc_oacc_routine_name;
 
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 94a7f7eaa50..d48c9351e25 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2234,34 +2234,45 @@ gfc_match_oacc_cache (void)
   return MATCH_YES;
 }
 
-/* Determine the loop level for a routine.   */
+/* Determine the loop level for a routine.  Returns OACC_FUNCTION_NONE
+   if any error is detected.  */
 
-static int
+static oacc_function
 gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
 {
   int level = -1;
+  oacc_function ret = OACC_FUNCTION_AUTO;
 
   if (clauses)
 {
   unsigned mask = 0;
 
   if (clauses->gang)
-	level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
+	{
+	  level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
+	  ret = OACC_FUNCTION_GANG;
+	}
   if (clauses->worker)
-	level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
+	{
+	  level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
+	  ret = OACC_FUNCTION_WORKER;
+	}
   if (clauses->vector)
-	level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
+	{
+	  level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
+	  ret = OACC_FUNCTION_VECTOR;
+	}
   if (clauses->seq)
-	level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
+	{
+	  level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
+	  ret = OACC_FUNCTION_SEQ;
+	}
 
   if (mask != (mask & -mask))
-	gfc_error ("Multiple loop axes specified for routine");
+	ret = OACC_FUNCTION_NONE;
 }
 
-  if (level < 0)
-level = GOMP_DIM_MAX;
-
-  return level;
+  return ret;
 }
 
 match
@@ -2272,6 +2283,8 @@ gfc_match_oacc_routine (void)
   match m;
   gfc_omp_clauses *c = NULL;
   gfc_oacc_routine_name *n = NULL;
+  oacc_function dims = OACC_FUNCTION_NONE;
+  bool seen_error = false;
 
   old_loc = gfc_current_locus;
 
@@ -2318,17 +2331,15 @@ gfc_match_oacc_routine (void)
 	}
   else
 {
-	  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %C");
-	  gfc_current_locus = old_loc;
-	  return MATCH_ERROR;
+	  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %L", _loc);
+	  goto cleanup;
 	}
 
   if (gfc_match_char (')') != MATCH_YES)
 	{
-	  gfc_error ("Syntax error in !$ACC ROUTINE ( NAME ) at %C, expecting"
-		 &quo