On 11/08/2015 07:29 AM, James Norris wrote:
> The attached patch and ChangeLog reflect the updates from your
> review: https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00714.html.
> All of the issues pointed out, have been address.
>
> With the changes made in this patch I think I'm handling the
> situation that you pointed out here correctly:
>
> On Fri, Nov 06, 2015 at 01:45:09PM -0600, James Norris wrote:
>
> Also, wonder about BLOCK stmt in Fortran, that can give you variables that
> don't live through the whole function, but only a portion of it even in
> Fortran.
>
> OK to commit to trunk?
I'll defer to Jakub, but here are a couple of comments.
> void
> gfc_resolve_oacc_declare (gfc_namespace *ns)
> {
> int list;
> gfc_omp_namelist *n;
> locus loc;
> + gfc_oacc_declare *oc;
>
> - if (ns->oacc_declare_clauses == NULL)
> + if (ns->oacc_declare == NULL)
> return;
>
> - loc = ns->oacc_declare_clauses->loc;
> + loc = gfc_current_locus;
>
> - for (list = OMP_LIST_DEVICE_RESIDENT;
> - list <= OMP_LIST_DEVICE_RESIDENT; list++)
> - for (n = ns->oacc_declare_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 (oc = ns->oacc_declare; oc; oc = oc->next)
> + {
> + for (list = OMP_LIST_DEVICE_RESIDENT;
> + list <= OMP_LIST_DEVICE_RESIDENT; list++)
Why is this loop necessary?
> + 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 = ns->oacc_declare_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 (list = OMP_LIST_DEVICE_RESIDENT;
> + list <= OMP_LIST_DEVICE_RESIDENT; list++)
And here.
> + 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 = ns->oacc_declare_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_DEVICE_RESIDENT]; n; n = n->next)
This is better.
> + 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 ("Array sections: %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;
> + }
>
> + 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;
> + }
> + }
> +
> + 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;
> + }
> +}
I only noticed these because I thought I fixed them in the patch you
asked me to revert from gomp-4_0-branch. At the very least, please try
to be consistent on iterating OMP_LIST_*.
Cesar
