On Fri, Oct 22, 2021 at 02:05:02PM +0100, Hafiz Abid Qadeer wrote: > This patch adds support for OpenMP 5.0 allocate clause for fortran. It does > not > yet support the allocator-modifier as specified in OpenMP 5.1. The allocate > clause is already supported in C/C++. > > gcc/fortran/ChangeLog: > > * dump-parse-tree.c (show_omp_clauses): Handle OMP_LIST_ALLOCATE. > * gfortran.h (OMP_LIST_ALLOCATE): New enum value. > (allocate): New member in gfc_symbol. > * openmp.c (enum omp_mask1): Add OMP_CLAUSE_ALLOCATE. > (gfc_match_omp_clauses): Handle OMP_CLAUSE_ALLOCATE
Missing . at the end. > (OMP_PARALLEL_CLAUSES, OMP_DO_CLAUSES, OMP_SECTIONS_CLAUSES) > (OMP_TASK_CLAUSES, OMP_TASKLOOP_CLAUSES, OMP_TARGET_CLAUSES) > (OMP_TEAMS_CLAUSES, OMP_DISTRIBUTE_CLAUSES) > (OMP_SINGLE_CLAUSES): Add OMP_CLAUSE_ALLOCATE. > (OMP_TASKGROUP_CLAUSES): New Likewise. > (gfc_match_omp_taskgroup): Use 'OMP_TASKGROUP_CLAUSES' instead of > 'OMP_CLAUSE_TASK_REDUCTION' Likewise. Please also drop the ' characters. > @@ -1880,6 +1881,10 @@ typedef struct gfc_symbol > according to the Fortran standard. */ > unsigned pass_as_value:1; > > + /* Used to check if a variable used in allocate clause has also been > + used in privatization clause. */ > + unsigned allocate:1; I think it would be desirable to use omp_allocate here instead of allocate and mention OpenMP in the comment too. Fortran has allocate statement in the language, so not pointing to OpenMP would only cause confusion. > @@ -1540,6 +1541,40 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const > omp_mask mask, > } > continue; > } > + if ((mask & OMP_CLAUSE_ALLOCATE) > + && gfc_match ("allocate ( ") == MATCH_YES) > + { > + gfc_expr *allocator = NULL; > + old_loc = gfc_current_locus; > + m = gfc_match_expr (&allocator); > + if (m != MATCH_YES) > + { > + gfc_error ("Expected allocator or variable list at %C"); > + goto error; > + } > + if (gfc_match (" : ") != MATCH_YES) > + { > + /* If no ":" then there is no allocator, we backtrack > + and read the variable list. */ > + allocator = NULL; Isn't this a memory leak? I believe Fortran FE expressions are not GC allocated... So, shouldn't there be gfc_free_expr or something similar before clearing it? > + /* Check for 2 things here. > + 1. There is no duplication of variable in allocate clause. > + 2. Variable in allocate clause are also present in some > + privatization clase. */ > + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n; n = n->next) > + n->sym->allocate = 0; > + > + gfc_omp_namelist *prev = NULL; > + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n;) > + { > + if (n->sym->allocate == 1) > + { > + gfc_warning (0, "%qs appears more than once in %<allocate%> " > + "clauses at %L" , n->sym->name, &n->where); > + /* We have already seen this variable so it is a duplicate. > + Remove it. */ > + if (prev != NULL && prev->next == n) > + { > + prev->next = n->next; > + n->next = NULL; > + gfc_free_omp_namelist (n, 0); > + n = prev->next; > + } > + > + continue; > + } > + n->sym->allocate = 1; > + prev = n; > + n = n->next; > + } > + > + for (list = 0; list < OMP_LIST_NUM; list++) > + switch (list) > + { > + case OMP_LIST_PRIVATE: > + case OMP_LIST_FIRSTPRIVATE: > + case OMP_LIST_LASTPRIVATE: > + case OMP_LIST_REDUCTION: > + case OMP_LIST_REDUCTION_INSCAN: > + case OMP_LIST_REDUCTION_TASK: > + case OMP_LIST_IN_REDUCTION: > + case OMP_LIST_TASK_REDUCTION: > + case OMP_LIST_LINEAR: > + for (n = omp_clauses->lists[list]; n; n = n->next) > + n->sym->allocate = 0; > + break; > + default: > + break; > + } > + > + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n; n = n->next) > + if (n->sym->allocate == 1) > + gfc_error ("%qs specified in 'allocate' clause at %L but not in an " > + "explicit privatization clause", n->sym->name, &n->where); I'm not sure this is what the standard says, certainly C/C++ FE do this quite differently for combined/composite constructs. In particular, we first split the clauses to the individual leaf constructs in c_omp_split_clauses, which for allocate clause is even more complicated because as clarified in 5.2: "The effect of the allocate clause is as if it is applied to all leaf constructs that permit the clause and to which a data-sharing attribute clause that may create a private copy of the same list item is applied." so there is the has_dup_allocate stuff, we first duplicate it to all leaf constructs that allow the allocate clause and set has_dup_allocate if it is put on more than one construct, and then if has_dup_allocate is set, do more detailed processing. And finally then {,c_}finish_omp_clauses diagnoses what you are trying above, but only on each leaf construct separately. Now, Fortran is performing the splitting of clauses only much later in trans-openmp.c, I wonder if it doesn't have other issues on combined/composite constructs if it performs other checks only on the clauses on the whole combined/composite construct and not just each leaf separately. I'd say we should move that diagnostics and perhaps other similar later on into a separate routine that is invoked only after the clauses are split or for non-combined/composite construct clauses. Jakub