[PATCH v3 04/11] OpenMP/OpenACC: mapping group list-handling improvements
Julian Brown
julian@codesourcery.com
Tue Sep 13 21:01:45 GMT 2022
This patch adjusts OpenMP/OpenACC clause list handling in a couple of
places, in preparation for the following mapping-clause expansion rework
patch. Firstly mapping groups are removed as a whole in the C and C++
front-ends when an error is detected, which avoids leaving "badly-formed"
mapping clause groups in the list.
Secondly, reindexing of the omp_mapping_group hashmap (during
omp_build_struct_sibling_lists) has been reimplemented, fixing some
tricky corner-cases where mapping groups are removed from a list at the
same time as it is being reordered.
Thirdly, code to check if a different clause on the same directive maps
the whole of a struct that we have a component mapping for (for example)
has been outlined, removing a bit of code duplication.
(These changes could be split into different patches if necessary.)
2022-09-13 Julian Brown <julian@codesourcery.com>
gcc/
* gimplify.cc (omp_group_last): Allow GOMP_MAP_ATTACH_DETACH after
GOMP_MAP_STRUCT (for reindexing).
(omp_gather_mapping_groups): Reimplement using...
(omp_gather_mapping_groups_1): This new function. Stop processing at
GATHER_SENTINEL.
(omp_group_base): Allow GOMP_MAP_TO_PSET without any following node.
(omp_index_mapping_groups): Reimplement using...
(omp_index_mapping_groups_1): This new function. Handle
REINDEX_SENTINEL.
(omp_reindex_mapping_groups, omp_mapped_by_containing_struct): New
functions.
(omp_tsort_mapping_groups_1): Adjust handling of base group being the
same as current group. Use omp_mapped_by_containing_struct.
(omp_build_struct_sibling_lists): Use omp_mapped_by_containing_struct
and omp_reindex_mapping_groups. Robustify group deletion for reordered
lists.
(gimplify_scan_omp_clauses): Update calls to
omp_build_struct_sibling_lists.
gcc/c/
* c-typeck.cc (c_finish_omp_clauses): Remove whole mapping node group
on error.
gcc/cp/
* semantics.cc (finish_omp_clauses): Likewise.
---
gcc/c/c-typeck.cc | 24 ++++-
gcc/cp/semantics.cc | 26 ++++-
gcc/gimplify.cc | 227 +++++++++++++++++++++++++++++++-------------
3 files changed, 209 insertions(+), 68 deletions(-)
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index ee891ee33c2..7da8d70b3bd 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -14229,12 +14229,19 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
break;
}
+ tree *grp_start_p = NULL, grp_sentinel = NULL_TREE;
+
for (pc = &clauses, c = clauses; c ; c = *pc)
{
bool remove = false;
bool need_complete = false;
bool need_implicitly_determined = false;
+ /* We've reached the end of a list of expanded nodes. Reset the group
+ start pointer. */
+ if (c == grp_sentinel)
+ grp_start_p = NULL;
+
switch (OMP_CLAUSE_CODE (c))
{
case OMP_CLAUSE_SHARED:
@@ -14995,6 +15002,9 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
t = OMP_CLAUSE_DECL (c);
if (TREE_CODE (t) == TREE_LIST)
{
+ grp_start_p = pc;
+ grp_sentinel = OMP_CLAUSE_CHAIN (c);
+
if (handle_omp_array_sections (c, ort))
remove = true;
else
@@ -15638,7 +15648,19 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
}
if (remove)
- *pc = OMP_CLAUSE_CHAIN (c);
+ {
+ if (grp_start_p)
+ {
+ /* If we found a clause to remove, we want to remove the whole
+ expanded group, otherwise gimplify
+ (omp_resolve_clause_dependencies) can get confused. */
+ *grp_start_p = grp_sentinel;
+ pc = grp_start_p;
+ grp_start_p = NULL;
+ }
+ else
+ *pc = OMP_CLAUSE_CHAIN (c);
+ }
else
pc = &OMP_CLAUSE_CHAIN (c);
}
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index ae7c8ea7b1f..7302d21fc54 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -6755,11 +6755,18 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
break;
}
+ tree *grp_start_p = NULL, grp_sentinel = NULL_TREE;
+
for (pc = &clauses, c = clauses; c ; c = *pc)
{
bool remove = false;
bool field_ok = false;
+ /* We've reached the end of a list of expanded nodes. Reset the group
+ start pointer. */
+ if (c == grp_sentinel)
+ grp_start_p = NULL;
+
switch (OMP_CLAUSE_CODE (c))
{
case OMP_CLAUSE_SHARED:
@@ -7985,6 +7992,9 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
t = OMP_CLAUSE_DECL (c);
if (TREE_CODE (t) == TREE_LIST)
{
+ grp_start_p = pc;
+ grp_sentinel = OMP_CLAUSE_CHAIN (c);
+
if (handle_omp_array_sections (c, ort))
remove = true;
else
@@ -8356,6 +8366,9 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
&& (OMP_CLAUSE_MAP_KIND (c)
!= GOMP_MAP_ATTACH_DETACH))
{
+ grp_start_p = pc;
+ grp_sentinel = OMP_CLAUSE_CHAIN (c);
+
tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c),
OMP_CLAUSE_MAP);
if (TREE_CODE (t) == COMPONENT_REF)
@@ -8766,7 +8779,18 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
}
if (remove)
- *pc = OMP_CLAUSE_CHAIN (c);
+ {
+ if (grp_start_p)
+ {
+ /* If we found a clause to remove, we want to remove the whole
+ expanded group, otherwise gimplify can get confused. */
+ *grp_start_p = grp_sentinel;
+ pc = grp_start_p;
+ grp_start_p = NULL;
+ }
+ else
+ *pc = OMP_CLAUSE_CHAIN (c);
+ }
else
pc = &OMP_CLAUSE_CHAIN (c);
}
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 7037fb4eb6c..c7998c2ccbd 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -9160,7 +9160,8 @@ omp_group_last (tree *start_p)
unsigned HOST_WIDE_INT num_mappings
= tree_to_uhwi (OMP_CLAUSE_SIZE (c));
if (OMP_CLAUSE_MAP_KIND (nc) == GOMP_MAP_FIRSTPRIVATE_POINTER
- || OMP_CLAUSE_MAP_KIND (nc) == GOMP_MAP_FIRSTPRIVATE_REFERENCE)
+ || OMP_CLAUSE_MAP_KIND (nc) == GOMP_MAP_FIRSTPRIVATE_REFERENCE
+ || OMP_CLAUSE_MAP_KIND (nc) == GOMP_MAP_ATTACH_DETACH)
grp_last_p = &OMP_CLAUSE_CHAIN (*grp_last_p);
for (unsigned i = 0; i < num_mappings; i++)
grp_last_p = &OMP_CLAUSE_CHAIN (*grp_last_p);
@@ -9176,12 +9177,13 @@ omp_group_last (tree *start_p)
associated GOMP_MAP_POINTER mappings). Return a vector of omp_mapping_group
if we have more than one such group, else return NULL. */
-static vec<omp_mapping_group> *
-omp_gather_mapping_groups (tree *list_p)
+static void
+omp_gather_mapping_groups_1 (tree *list_p, vec<omp_mapping_group> *groups,
+ tree gather_sentinel)
{
- vec<omp_mapping_group> *groups = new vec<omp_mapping_group> ();
-
- for (tree *cp = list_p; *cp; cp = &OMP_CLAUSE_CHAIN (*cp))
+ for (tree *cp = list_p;
+ *cp && *cp != gather_sentinel;
+ cp = &OMP_CLAUSE_CHAIN (*cp))
{
if (OMP_CLAUSE_CODE (*cp) != OMP_CLAUSE_MAP)
continue;
@@ -9199,6 +9201,14 @@ omp_gather_mapping_groups (tree *list_p)
cp = grp_last_p;
}
+}
+
+static vec<omp_mapping_group> *
+omp_gather_mapping_groups (tree *list_p)
+{
+ vec<omp_mapping_group> *groups = new vec<omp_mapping_group> ();
+
+ omp_gather_mapping_groups_1 (list_p, groups, NULL_TREE);
if (groups->length () > 0)
return groups;
@@ -9247,7 +9257,8 @@ omp_group_base (omp_mapping_group *grp, unsigned int *chained,
node = OMP_CLAUSE_CHAIN (node);
if (node && OMP_CLAUSE_MAP_KIND (node) == GOMP_MAP_TO_PSET)
{
- gcc_assert (node != grp->grp_end);
+ if (node == grp->grp_end)
+ return *grp->grp_start;
node = OMP_CLAUSE_CHAIN (node);
}
if (node)
@@ -9345,18 +9356,22 @@ omp_group_base (omp_mapping_group *grp, unsigned int *chained,
/* Given a vector of omp_mapping_groups, build a hash table so we can look up
nodes by tree_operand_hash. */
-static hash_map<tree_operand_hash, omp_mapping_group *> *
-omp_index_mapping_groups (vec<omp_mapping_group> *groups)
+static void
+omp_index_mapping_groups_1 (hash_map<tree_operand_hash,
+ omp_mapping_group *> *grpmap,
+ vec<omp_mapping_group> *groups,
+ tree reindex_sentinel)
{
- hash_map<tree_operand_hash, omp_mapping_group *> *grpmap
- = new hash_map<tree_operand_hash, omp_mapping_group *>;
-
omp_mapping_group *grp;
unsigned int i;
+ bool reindexing = reindex_sentinel != NULL_TREE, above_hwm = false;
FOR_EACH_VEC_ELT (*groups, i, grp)
{
- if (grp->deleted)
+ if (reindexing && *grp->grp_start == reindex_sentinel)
+ above_hwm = true;
+
+ if (reindexing && !above_hwm)
continue;
tree fpp;
@@ -9378,8 +9393,7 @@ omp_index_mapping_groups (vec<omp_mapping_group> *groups)
source instead. FIXME. */
if (TREE_CODE (decl) == MEM_REF
&& integer_zerop (TREE_OPERAND (decl, 1)))
- decl = build1 (INDIRECT_REF, TREE_TYPE (decl),
- TREE_OPERAND (decl, 0));
+ decl = build_fold_indirect_ref (TREE_OPERAND (decl, 0));
omp_mapping_group **prev = grpmap->get (decl);
@@ -9408,7 +9422,7 @@ omp_index_mapping_groups (vec<omp_mapping_group> *groups)
continue;
omp_mapping_group **prev = grpmap->get (fpp);
- if (prev)
+ if (prev && *prev != grp)
{
grp->sibling = (*prev)->sibling;
(*prev)->sibling = grp;
@@ -9416,6 +9430,48 @@ omp_index_mapping_groups (vec<omp_mapping_group> *groups)
else
grpmap->put (fpp, grp);
}
+}
+
+static hash_map<tree_operand_hash, omp_mapping_group *> *
+omp_index_mapping_groups (vec<omp_mapping_group> *groups)
+{
+ hash_map<tree_operand_hash, omp_mapping_group *> *grpmap
+ = new hash_map<tree_operand_hash, omp_mapping_group *>;
+
+ omp_index_mapping_groups_1 (grpmap, groups, NULL_TREE);
+
+ return grpmap;
+}
+
+/* Rebuild group map from partially-processed clause list (during
+ omp_build_struct_sibling_lists). We have already processed nodes up until
+ a high-water mark (HWM). This is a bit tricky because the list is being
+ reordered as it is scanned, but we know:
+
+ 1. The list after HWM has not been touched yet, so we can reindex it safely.
+
+ 2. The list before and including HWM has been altered, but remains
+ well-formed throughout the sibling-list building operation.
+
+ so, we can do the reindex operation in two parts, on the processed and
+ then the unprocessed halves of the list. */
+
+static hash_map<tree_operand_hash, omp_mapping_group *> *
+omp_reindex_mapping_groups (tree *list_p,
+ vec<omp_mapping_group> *groups,
+ vec<omp_mapping_group> *processed_groups,
+ tree sentinel)
+{
+ hash_map<tree_operand_hash, omp_mapping_group *> *grpmap
+ = new hash_map<tree_operand_hash, omp_mapping_group *>;
+
+ processed_groups->truncate (0);
+
+ omp_gather_mapping_groups_1 (list_p, processed_groups, sentinel);
+ omp_index_mapping_groups_1 (grpmap, processed_groups, NULL_TREE);
+ if (sentinel)
+ omp_index_mapping_groups_1 (grpmap, groups, sentinel);
+
return grpmap;
}
@@ -9443,6 +9499,41 @@ omp_containing_struct (tree expr)
return expr;
}
+static bool
+omp_mapped_by_containing_struct (hash_map<tree_operand_hash,
+ omp_mapping_group *> *grpmap,
+ tree decl,
+ omp_mapping_group **mapped_by_group)
+{
+ tree wsdecl = NULL_TREE;
+
+ *mapped_by_group = NULL;
+
+ while (true)
+ {
+ wsdecl = omp_containing_struct (decl);
+ if (wsdecl == decl)
+ break;
+ omp_mapping_group **wholestruct = grpmap->get (wsdecl);
+ if (!wholestruct
+ && TREE_CODE (wsdecl) == MEM_REF
+ && integer_zerop (TREE_OPERAND (wsdecl, 1)))
+ {
+ tree deref = TREE_OPERAND (wsdecl, 0);
+ deref = build_fold_indirect_ref (deref);
+ wholestruct = grpmap->get (deref);
+ }
+ if (wholestruct)
+ {
+ *mapped_by_group = *wholestruct;
+ return true;
+ }
+ decl = wsdecl;
+ }
+
+ return false;
+}
+
/* Helper function for omp_tsort_mapping_groups. Returns TRUE on success, or
FALSE on error. */
@@ -9470,9 +9561,8 @@ omp_tsort_mapping_groups_1 (omp_mapping_group ***outlist,
{
omp_mapping_group **basep = grpmap->get (attaches_to);
- if (basep)
+ if (basep && *basep != grp)
{
- gcc_assert (*basep != grp);
for (omp_mapping_group *w = *basep; w; w = w->sibling)
if (!omp_tsort_mapping_groups_1 (outlist, groups, grpmap, w))
return false;
@@ -9489,25 +9579,16 @@ omp_tsort_mapping_groups_1 (omp_mapping_group ***outlist,
break;
omp_mapping_group **innerp = grpmap->get (base);
+ omp_mapping_group *wholestruct;
/* We should treat whole-structure mappings as if all (pointer, in this
case) members are mapped as individual list items. Check if we have
such a whole-structure mapping, if we don't have an explicit reference
to the pointer member itself. */
- if (!innerp && TREE_CODE (base) == COMPONENT_REF)
- {
- base = omp_containing_struct (base);
- innerp = grpmap->get (base);
-
- if (!innerp
- && TREE_CODE (base) == MEM_REF
- && integer_zerop (TREE_OPERAND (base, 1)))
- {
- tree ind = TREE_OPERAND (base, 0);
- ind = build1 (INDIRECT_REF, TREE_TYPE (base), ind);
- innerp = grpmap->get (ind);
- }
- }
+ if (!innerp
+ && TREE_CODE (base) == COMPONENT_REF
+ && omp_mapped_by_containing_struct (grpmap, base, &wholestruct))
+ innerp = &wholestruct;
if (innerp && *innerp != grp)
{
@@ -10296,7 +10377,8 @@ omp_build_struct_sibling_lists (enum tree_code code,
enum omp_region_type region_type,
vec<omp_mapping_group> *groups,
hash_map<tree_operand_hash, omp_mapping_group *>
- **grpmap)
+ **grpmap,
+ tree *list_p)
{
unsigned i;
omp_mapping_group *grp;
@@ -10304,16 +10386,22 @@ omp_build_struct_sibling_lists (enum tree_code code,
bool success = true;
tree *new_next = NULL;
tree *tail = &OMP_CLAUSE_CHAIN ((*groups)[groups->length () - 1].grp_end);
+ auto_vec<omp_mapping_group> pre_hwm_groups;
FOR_EACH_VEC_ELT (*groups, i, grp)
{
tree c = grp->grp_end;
tree decl = OMP_CLAUSE_DECL (c);
- tree *grp_start_p = new_next ? new_next : grp->grp_start;
tree grp_end = grp->grp_end;
+ tree sentinel = OMP_CLAUSE_CHAIN (grp_end);
+
+ if (new_next)
+ grp->grp_start = new_next;
new_next = NULL;
+ tree *grp_start_p = grp->grp_start;
+
if (DECL_P (decl))
continue;
@@ -10353,36 +10441,16 @@ omp_build_struct_sibling_lists (enum tree_code code,
if (TREE_CODE (decl) != COMPONENT_REF)
continue;
- omp_mapping_group **wholestruct = NULL;
- tree wsdecl = omp_containing_struct (OMP_CLAUSE_DECL (c));
-
- if (!(region_type & ORT_ACC) && wsdecl != OMP_CLAUSE_DECL (c))
- {
- wholestruct = (*grpmap)->get (wsdecl);
- if (!wholestruct
- && TREE_CODE (wsdecl) == MEM_REF
- && integer_zerop (TREE_OPERAND (wsdecl, 1)))
- {
- tree deref = TREE_OPERAND (wsdecl, 0);
- deref = build1 (INDIRECT_REF, TREE_TYPE (wsdecl), deref);
- wholestruct = (*grpmap)->get (deref);
- }
- }
-
- if (wholestruct)
+ /* If we're mapping the whole struct in another node, skip creation of
+ sibling lists. */
+ omp_mapping_group *wholestruct;
+ if (!(region_type & ORT_ACC)
+ && omp_mapped_by_containing_struct (*grpmap, OMP_CLAUSE_DECL (c),
+ &wholestruct))
{
if (*grp_start_p == grp_end)
- {
- /* Remove the whole of this mapping -- redundant. */
- if (i + 1 < groups->length ())
- {
- omp_mapping_group *nextgrp = &(*groups)[i + 1];
- nextgrp->grp_start = grp_start_p;
- }
- grp->deleted = true;
- new_next = grp_start_p;
- *grp_start_p = OMP_CLAUSE_CHAIN (grp_end);
- }
+ /* Remove the whole of this mapping -- redundant. */
+ grp->deleted = true;
continue;
}
@@ -10427,7 +10495,6 @@ omp_build_struct_sibling_lists (enum tree_code code,
*tail = inner;
OMP_CLAUSE_CHAIN (inner) = NULL_TREE;
-
omp_mapping_group newgrp;
newgrp.grp_start = new_next ? new_next : tail;
newgrp.grp_end = inner;
@@ -10441,13 +10508,39 @@ omp_build_struct_sibling_lists (enum tree_code code,
map. Rebuild it here. This is a bit inefficient, but
shouldn't happen very often. */
delete (*grpmap);
- *grpmap = omp_index_mapping_groups (groups);
+ *grpmap
+ = omp_reindex_mapping_groups (list_p, groups, &pre_hwm_groups,
+ sentinel);
tail = &OMP_CLAUSE_CHAIN (inner);
}
}
}
+ /* Delete groups marked for deletion above. At this point the order of the
+ groups may no longer correspond to the order of the underlying list,
+ which complicates this a little. First clear out OMP_CLAUSE_DECL for
+ deleted nodes... */
+
+ FOR_EACH_VEC_ELT (*groups, i, grp)
+ if (grp->deleted)
+ for (tree d = *grp->grp_start;
+ d != OMP_CLAUSE_CHAIN (grp->grp_end);
+ d = OMP_CLAUSE_CHAIN (d))
+ OMP_CLAUSE_DECL (d) = NULL_TREE;
+
+ /* ...then sweep through the list removing the now-empty nodes. */
+
+ tail = list_p;
+ while (*tail)
+ {
+ if (OMP_CLAUSE_CODE (*tail) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_DECL (*tail) == NULL_TREE)
+ *tail = OMP_CLAUSE_CHAIN (*tail);
+ else
+ tail = &OMP_CLAUSE_CHAIN (*tail);
+ }
+
error_out:
if (struct_map_to_clause)
delete struct_map_to_clause;
@@ -10508,7 +10601,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
hash_map<tree_operand_hash, omp_mapping_group *> *grpmap;
grpmap = omp_index_mapping_groups (groups);
- omp_build_struct_sibling_lists (code, region_type, groups, &grpmap);
+ omp_build_struct_sibling_lists (code, region_type, groups, &grpmap,
+ list_p);
omp_mapping_group *outlist = NULL;
@@ -10543,7 +10637,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
hash_map<tree_operand_hash, omp_mapping_group *> *grpmap;
grpmap = omp_index_mapping_groups (groups);
- omp_build_struct_sibling_lists (code, region_type, groups, &grpmap);
+ omp_build_struct_sibling_lists (code, region_type, groups, &grpmap,
+ list_p);
delete groups;
delete grpmap;
--
2.29.2
More information about the Gcc-patches
mailing list