On Thu, Nov 18, 2021 at 07:30:36PM +0000, Hafiz Abid Qadeer wrote:
> + if (gfc_match (" : ") != MATCH_YES)
> + {
> + /* If no ":" then there is no allocator, we backtrack
> + and read the variable list. */
> + gfc_free_expr (allocator);
> + allocator = NULL;
> + gfc_current_locus = old_loc;
> + }
Ok, no leak above.
> +
> + gfc_omp_namelist **head = NULL;
> + m = gfc_match_omp_variable_list ("", &c->lists[OMP_LIST_ALLOCATE],
> + false, NULL, &head);
> +
> + if (m == MATCH_ERROR)
> + break;
But here it leaks. Just call gfc_free_expr (allocator); before break.
> +
> + gfc_omp_namelist *n;
> + for (n = *head; n; n = n->next)
> + if (allocator)
> + n->expr = gfc_copy_expr (allocator);
> + else
> + n->expr = NULL;
> + gfc_free_expr (allocator);
> + continue;
> + }
> if ((mask & OMP_CLAUSE_AT)
> && (m = gfc_match_dupl_check (c->at == OMP_AT_UNSET, "at", true))
> != MATCH_NO)
> + if (omp_clauses->lists[OMP_LIST_ALLOCATE])
> + {
> + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n; n = n->next)
> + if (n->expr && (n->expr->ts.type != BT_INTEGER
> + || n->expr->ts.kind != gfc_c_intptr_kind))
> + {
> + gfc_error ("Expected integer expression of the "
> + "'omp_allocator_handle_kind' kind at %L", &n->expr->where);
Formatting, "' should be indented below "Expected
> + break;
> + }
> +
> + /* 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 (non-composite case). */
> + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n; n = n->next)
> + n->sym->omp_allocate_clause = 0;
> +
> + gfc_omp_namelist *prev = NULL;
> + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n;)
> + {
> + if (n->sym->omp_allocate_clause == 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->omp_allocate_clause = 1;
> + prev = n;
> + n = n->next;
> + }
> +
> + /* non-composite constructs. */
> + if (code && code->op < EXEC_OMP_DO_SIMD)
> + {
> + 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->omp_allocate_clause = 0;
> + break;
> + default:
> + break;
> + }
> +
> + for (n = omp_clauses->lists[OMP_LIST_ALLOCATE]; n; n = n->next)
> + if (n->sym->omp_allocate_clause == 1)
> + gfc_error ("%qs specified in 'allocate' clause at %L but not "
> + "in an explicit privatization clause",
> + n->sym->name, &n->where);
> + }
> + }
Do you really need a new omp_allocate_clause bit? From what I can see,
other code uses n->sym->mark for such purposes (temporarily marking some
symbols).
Also, I think allocate clause like the privatization clauses should allow
common blocks and I see code that uses the marks uses something like:
for (list = OMP_LIST_TO; list != OMP_LIST_NUM;
list = (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM))
for (n = c->lists[list]; n; n = n->next)
if (n->sym)
n->sym->mark = 0;
else if (n->u.common->head)
n->u.common->head->mark = 0;
So, a question is if the above won't just crash if I specify
firstprivate(/foobar/) allocate(/foobar/)
etc.
> + case OMP_LIST_ALLOCATE:
> + for (; n != NULL; n = n->next)
> + if (n->sym->attr.referenced || declare_simd)
!$omp declare simd doesn't allow allocate clause, so why the
above " || declare_simd"?
> + {
> + tree t = gfc_trans_omp_variable (n->sym, declare_simd);
And not , false); above?
Jakub
