Re: [PATCH v5 8/11] openmp: Fix struct handling for OpenMP iterators

[Tobias Burnus]

Kwok Cheung Yeung wrote:

This patch fixes some issues with the struct handling introduced in the patch for Fortran support. The problem is that build_omp_struct_comp_nodes and omp_accumulate_sibling_list can add extra clauses with iterators onto the target construct, but this occurs after the iterator loop has already been built, so no iterator element array has been allocated for the new clause and the iterator vector will refer to whatever the new clause was derived from, effectively sharing data between the two clauses.

I think this patch badly needs a testcase.

I think neither anything before this 8/11 patch requires it an it is
not obvious whether a later patch requires it.

9/11 seems to be a candidate as from the description, I would expect
that 'declare mapper' requires one for code like:

#pragma omp declare mapper(T t) ...

struct S {
  struct T t;
};

struct S[5] s;
... map(iterator(i=0:5), to: s[i].t)

However, I don't see such a testcase in 9/11. It might be that
Fortran deep mapping, 10/11 requires it, but that's not really
clear, either. [I have to admit that I still have to look more
closely at 10/11.]

Still, I think at least a reference in this thread to a testcase
would be good - and, if there is none, adding one would be good.
For instance, as part of 9/11 (custom mapper in C/C++).

* * *

Otherwise, LGTM.

Thanks for the patch:

A new function add_new_omp_iterators_clause has been added which allocates a new element array after the loop has already been built, and updates the clause iterator to point to the new array instead. This is called each time a new clauses is created from an existing one.

…

Subject: [PATCH 08/11] openmp: Fix struct handling for OpenMP iterators

New clauses can be created for structs, and these will also need to have
iterators applied to them if the base clause is using iterators.  As this
occurs after the initial iterator expansion, a new mechanism for allocating
new entries in the iterator loop is required.

gcc/

        * gimplify.cc (add_new_omp_iterators_clause): New.
        (build_omp_struct_comp_nodes): Add extra argument for loops sequence.
        Call add_new_omp_iterators_clause on newly generated clauses.
        (omp_accumulate_sibling_list): Add extra argument for loops sequence.
        Pass to calls to build_omp_struct_comp_nodes.  Add iterators to newly
        generator clauses for struct accesses.
        (omp_build_struct_sibling_lists): Add extra argument for loops
        sequence. Pass to call to omp_accumulate_sibling_list.
        (gimplify_adjust_omp_clauses): Pass loops sequence to
        omp_build_struct_sibling_lists.

Tobias