Re: [gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives

[Cesar Philippidis <cesar@codesourcery.com>]

On 06/07/2015 02:05 PM, Ilmir Usmanov wrote:
> Fixed fortran mail-list address. Sorry for inconvenience.
> 
> 08.06.2015, 00:01, "Ilmir Usmanov" <me@ilmir.us>:
>>> Hi Cesar!
>>> 
>>> This patch fixes checks of OpenMP and OpenACC continuations in
>>> case if someone mixes them (i.e. continues OpenMP directive with
>>> !$ACC sentinel or vice versa).
>>> 
>>> OK for gomp branch?

Thanks for working on this. Does this fix PR63858 by any chance?

two minor nits...

> 0001-Fix-mix-of-OpenACC-and-OpenMP-sentinels-in-continuat.patch
> 
> 
> From 5492bf5bc991b6924f5e3b35c11eeaed745df073 Mon Sep 17 00:00:00 2001
> From: Ilmir Usmanov <i.usmanov@samsung.com>
> Date: Sun, 7 Jun 2015 23:55:22 +0300
> Subject: [PATCH] Fix mix of OpenACC and OpenMP sentinels in continuation
> 
> ---
>  gcc/fortran/ChangeLog                   |  5 +++++

Use ChangeLog.gomp for gomp-4_0-branch.

>  gcc/fortran/scanner.c                   | 28 ++++++++++++++++++++++++----
>  gcc/testsuite/ChangeLog                 |  5 +++++
>  gcc/testsuite/gfortran.dg/goacc/omp.f95 |  8 ++++++++
>  4 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
> index 67f9e09..f61e0e9 100644
> --- a/gcc/fortran/ChangeLog
> +++ b/gcc/fortran/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-06-07  Ilmir Usmanov  <me@ilmir.us>
> +
> +	* scanner.c (gfc_next_char_literal): Fix mix of OpenACC and OpenMP
> +	sentinels in continuation.
> +
>  2015-05-05  David Malcolm  <dmalcolm@redhat.com>
>  
>  	* expr.c (check_inquiry): Fix indentation so that it reflects the
> diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
> index f0e6404..5af4eea 100644
> --- a/gcc/fortran/scanner.c
> +++ b/gcc/fortran/scanner.c
> @@ -1331,7 +1331,7 @@ restart:
>  	continue_line = gfc_linebuf_linenum (gfc_current_locus.lb);
>  
>        if (flag_openmp)
> -	if (prev_openmp_flag != openmp_flag)
> +	if (prev_openmp_flag != openmp_flag && !openacc_flag)
>  	  {
>  	    gfc_current_locus = old_loc;
>  	    openmp_flag = prev_openmp_flag;
> @@ -1340,7 +1340,7 @@ restart:
>  	  }
>  
>        if (flag_openacc)
> -	if (prev_openacc_flag != openacc_flag)
> +	if (prev_openacc_flag != openacc_flag && !openmp_flag)
>  	  {
>  	    gfc_current_locus = old_loc;
>  	    openacc_flag = prev_openacc_flag;
> @@ -1359,7 +1359,7 @@ restart:
>        while (gfc_is_whitespace (c))
>  	c = next_char ();
>  
> -      if (openmp_flag)
> +      if (openmp_flag && !openacc_flag)
>  	{
>  	  for (i = 0; i < 5; i++, c = next_char ())
>  	    {
> @@ -1370,7 +1370,7 @@ restart:
>  	  while (gfc_is_whitespace (c))
>  	    c = next_char ();
>  	}
> -      if (openacc_flag)
> +      if (openacc_flag && !openmp_flag)
>  	{
>  	  for (i = 0; i < 5; i++, c = next_char ())
>  	    {
> @@ -1382,6 +1382,26 @@ restart:
>  	    c = next_char ();
>  	}
>  
> +      /* In case we have an OpenMP directive continued by OpenACC
> +	 sentinel, or vice versa, we get both openmp_flag and
> +	 openacc_flag on.  */
> +
> +      if (openacc_flag && openmp_flag)
> +	{
> +	  int is_openmp = 0;
> +	  for (i = 0; i < 5; i++, c = next_char ())
> +	    {
> +	      if (gfc_wide_tolower (c) != (unsigned char) "!$acc"[i])
> +		is_openmp = 1;
> +	      if (i == 4)
> +		old_loc = gfc_current_locus;
> +	    }
> +	  gfc_error ("Wrong %s continuation at %C: expected %s, got %s",
> +	             is_openmp ? "OpenACC" : "OpenMP",
> +	             is_openmp ? "!$ACC" : "!$OMP",
> +	             is_openmp ? "!$OMP" : "!$ACC");

I think it's better for the translation project if you made this a
complete string. So maybe change this line into

  gfc_error (is_openmp ? "Wrong continuation at %C: expected !$ACC, got"
             " !$OMP",
             : "Wrong continuation at %C: expected !$OMP, got !$ACC");

Other than that, it looks fine.

Thanks,
Cesar