Re: [gomp4] fortran cleanups and c/c++ loop parsing backport

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi Cesar!

On Tue, 27 Oct 2015 11:36:10 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
>   * Proposed fortran cleanups and enhanced error reporting changes:
>     https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02288.html

... has now been applied to trunk, in an altered version, so we now got
some divergence between trunk and gomp-4_0-branch to resolve.

> I'll apply this patch to gomp-4_0-branch shortly. I know that I should
> have broken this patch down into smaller patches, but it was already
> arranged as one big patch in my source tree.

> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c

> @@ -1473,8 +1468,8 @@ gfc_match_oacc_declare (void)
>  	  if (n->u.map_op != OMP_MAP_FORCE_ALLOC
>  	      && n->u.map_op != OMP_MAP_FORCE_TO)
>  	    {
> -	      gfc_error ("Invalid clause in module with "
> -			 "$!ACC DECLARE at %C");
> +	      gfc_error ("Invalid clause in module with $!ACC DECLARE at %L",
> +			 &n->where);
>  	      return MATCH_ERROR;
>  	    }
>  
> @@ -1483,29 +1478,29 @@ gfc_match_oacc_declare (void)
>  
>        if (ns->proc_name->attr.oacc_function)
>  	{
> -	  gfc_error ("Invalid declare in routine with " "$!ACC DECLARE at %C");
> +	  gfc_error ("Invalid declare in routine with $!ACC DECLARE at %C");
>  	  return MATCH_ERROR;
>  	}

Shouldn't the "%C" -> "%L (&n->where)" substitution also be done for that
one?  If yes, please do so; if no, please add a source code comment, why.

>  
>        if (s->attr.in_common)
>  	{
> -	  gfc_error ("Unsupported: variable in a common block with "
> -		     "$!ACC DECLARE at %C");
> +	  gfc_error ("Variable in a common block with $!ACC DECLARE at %L",
> +		     &n->where);
>  	  return MATCH_ERROR;
>  	}
>  
>        if (s->attr.use_assoc)
>  	{
> -	  gfc_error ("Unsupported: variable is USE-associated with "
> -		     "$!ACC DECLARE at %C");
> +	  gfc_error ("Variable is USE-associated with $!ACC DECLARE at %L",
> +		     &n->where);
>  	  return MATCH_ERROR;
>  	}
>  
>        if ((s->attr.dimension || s->attr.codimension)
>  	  && s->attr.dummy && s->as->type != AS_EXPLICIT)
>  	{
> -	  gfc_error ("Unsupported: assumed-size dummy array with "
> -		     "$!ACC DECLARE at %C");
> +	  gfc_error ("Assumed-size dummy array with $!ACC DECLARE at %L",
> +		     &n->where);
>  	  return MATCH_ERROR;
>  	}

> -/* Returns true if clause in list 'list' is compatible with any of
> -   of the clauses in lists [0..list-1].  E.g., a reduction variable may
> -   appear in both reduction and private clauses, so this function
> -   will return true in this case.  */
> +/* Check if a variable appears in multiple clauses.  */
>  
> -static bool
> -oacc_compatible_clauses (gfc_omp_clauses *clauses, int list,
> -			   gfc_symbol *sym, bool openacc)
> +static void
> +resolve_omp_duplicate_list (gfc_omp_namelist *clause_list, bool openacc,
> +			    int list)

If I understand correctly, that has not been approved for trunk, so we
should revert this on gomp-4_0-branch, too?

>  {
>    gfc_omp_namelist *n;
> +  const char *error_msg = "Symbol %qs present on multiple clauses at %L";
>  
> -  if (!openacc)
> -    return false;
> +  /* OpenACC reduction clauses are compatible with everything.  We only
> +     need to check if a reduction variable is used more than once.  */
> +  if (openacc && list == OMP_LIST_REDUCTION)
> +    {
> +      hash_set<gfc_symbol *> reductions;
>  
> -  if (list != OMP_LIST_REDUCTION)
> -    return false;
> +      for (n = clause_list; n; n = n->next)
> +	{
> +	  if (reductions.contains (n->sym))
> +	    gfc_error (error_msg, n->sym->name, &n->where);
> +	  else
> +	    reductions.add (n->sym);
> +	}
>  
> -  for (n = clauses->lists[OMP_LIST_FIRST]; n; n = n->next)
> -    if (n->sym == sym)
> -      return true;
> +      return;
> +    }
>  
> -  return false;
> +  /* Ensure that variables are only used in one clause.  */
> +  for (n = clause_list; n; n = n->next)
> +    {
> +      if (n->sym->mark)
> +	gfc_error (error_msg, n->sym->name, &n->where);
> +      else
> +	n->sym->mark = 1;
> +    }
>  }

> @@ -4966,64 +4927,27 @@ gfc_resolve_oacc_declare (gfc_namespace *ns)
>      {
>        loc = oc->where;

As far as I can tell, given the following changes, the "loc" local
variable is now unused, so should be removed?

>  
> -      for (list = OMP_LIST_DEVICE_RESIDENT;
> -	   list <= OMP_LIST_DEVICE_RESIDENT; list++)
> -	for (n = oc->clauses->lists[list]; n; n = n->next)
> -	  {
> -	    n->sym->mark = 0;
> -	    if (n->sym->attr.flavor == FL_PARAMETER)
> -	      gfc_error ("PARAMETER object %qs is not allowed at %L",
> -			 n->sym->name, &loc);
> -	  }
> -
> -      for (list = OMP_LIST_DEVICE_RESIDENT;
> -	    list <= OMP_LIST_DEVICE_RESIDENT; list++)
> -	for (n = oc->clauses->lists[list]; n; n = n->next)
> -	  {
> -	    if (n->sym->mark)
> -	      gfc_error ("Symbol %qs present on multiple clauses at %L",
> -			 n->sym->name, &loc);
> -	    else
> -	      n->sym->mark = 1;
> -	  }
> -
>        for (n = oc->clauses->lists[OMP_LIST_DEVICE_RESIDENT]; n; n = n->next)
> -	check_array_not_assumed (n->sym, loc, "DEVICE_RESIDENT");
> -
> -      for (n = oc->clauses->lists[OMP_LIST_MAP]; n; n = n->next)
>  	{
> -	  if (n->expr && n->expr->ref->type == REF_ARRAY)
> -	      gfc_error ("Subarray: %qs not allowed in $!ACC DECLARE at %L",
> -			 n->sym->name, &loc);
> -	}
> -    }
> -
> -  for (oc = ns->oacc_declare; oc; oc = oc->next)
> -    {
> -      for (list = OMP_LIST_LINK; list <= OMP_LIST_LINK; list++)
> -	for (n = oc->clauses->lists[list]; n; n = n->next)
>  	  n->sym->mark = 0;
> -    }
> +	  if (n->sym->attr.flavor == FL_PARAMETER)
> +	    gfc_error ("PARAMETER object %qs is not allowed at %L",
> +		       n->sym->name, &n->where);
>  
> -  for (oc = ns->oacc_declare; oc; oc = oc->next)
> -    {
> -      for (list = OMP_LIST_LINK; list <= OMP_LIST_LINK; list++)
> -	for (n = oc->clauses->lists[list]; n; n = n->next)
> -	  {
> -	    if (n->sym->mark)
> -	      gfc_error ("Symbol %qs present on multiple clauses at %L",
> -			 n->sym->name, &loc);
> -	    else
> -	      n->sym->mark = 1;
> -	  }
> -    }
> +	  check_array_not_assumed (n->sym, n->where,
> +				   "DEVICE_RESIDENT");	  
> +	}
>  
> -  for (oc = ns->oacc_declare; oc; oc = oc->next)
> -    {
> -      for (list = OMP_LIST_LINK; list <= OMP_LIST_LINK; list++)
> -	for (n = oc->clauses->lists[list]; n; n = n->next)
> -	  n->sym->mark = 0;
> +      for (n = oc->clauses->lists[OMP_LIST_MAP]; n; n = n->next)
> +	if (n->expr && n->expr->ref->type == REF_ARRAY)
> +	  gfc_error ("Subarray %qs is not allowed in $!ACC DECLARE at %L",
> +		     n->sym->name, &n->where);
>      }
> +
> +  /* Check for duplicate link, device_resident and data clauses.  */
> +  resolve_oacc_declare_map (ns->oacc_declare, OMP_LIST_LINK);
> +  resolve_oacc_declare_map (ns->oacc_declare, OMP_LIST_DEVICE_RESIDENT);
> +  resolve_oacc_declare_map (ns->oacc_declare, OMP_LIST_MAP);
>  }


GrÃÃe
 Thomas

Attachment: signature.asc
Description: PGP signature