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  <jul...@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

Reply via email to