From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1410) id 676093856263; Wed, 14 Sep 2022 14:00:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 676093856263 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1663164012; bh=kvfRa08SoER8UO5fUpPoBZ8Vj/cJvX43it4cTH7hldM=; h=From:To:Subject:Date:From; b=FjLo9oa+4ABHf1+Skhibnnp8LkDYT1Ivc3HGLFMpBHCL540SGA2ITDn86QQU+0sZd 2xqD9t7GfViTS2WNoHKCRZ3fysUnDwrZ0y1dJTG9cIEPT0ABHK4SHKd6tVin7ZTq/z 6bhNESDcMpV0MLL1EETaYHKda0HUum+iHt+0t5N4= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Julian Brown To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-2666] OpenMP/OpenACC: mapping group list-handling improvements X-Act-Checkin: gcc X-Git-Author: Julian Brown X-Git-Refname: refs/heads/master X-Git-Oldrev: 23baa717c991d77f206a9358ce2c04960ccf9eea X-Git-Newrev: f469ce1d3ef94092647125662ddd93847712909f Message-Id: <20220914140012.676093856263@sourceware.org> Date: Wed, 14 Sep 2022 14:00:12 +0000 (GMT) List-Id: https://gcc.gnu.org/g:f469ce1d3ef94092647125662ddd93847712909f commit r13-2666-gf469ce1d3ef94092647125662ddd93847712909f Author: Julian Brown Date: Tue Sep 6 22:34:38 2022 +0000 OpenMP/OpenACC: mapping group list-handling improvements 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. 2022-09-13 Julian Brown 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. Diff: --- gcc/c/c-typeck.cc | 24 +++++- gcc/cp/semantics.cc | 26 +++++- gcc/gimplify.cc | 231 +++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 213 insertions(+), 68 deletions(-) diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 9ada5d25531..43a910d5df2 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -14238,12 +14238,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: @@ -15001,6 +15008,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 @@ -15644,7 +15654,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 4f189978484..427b1ab5ebc 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: @@ -7982,6 +7989,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 @@ -8353,6 +8363,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) @@ -8763,7 +8776,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 e9b8aa8d55c..2ae0c8cb250 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -9154,7 +9154,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); @@ -9170,12 +9171,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_gather_mapping_groups (tree *list_p) +static void +omp_gather_mapping_groups_1 (tree *list_p, vec *groups, + tree gather_sentinel) { - vec *groups = new vec (); - - 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; @@ -9193,6 +9195,14 @@ omp_gather_mapping_groups (tree *list_p) cp = grp_last_p; } +} + +static vec * +omp_gather_mapping_groups (tree *list_p) +{ + vec *groups = new vec (); + + omp_gather_mapping_groups_1 (list_p, groups, NULL_TREE); if (groups->length () > 0) return groups; @@ -9241,7 +9251,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) @@ -9339,18 +9350,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 * -omp_index_mapping_groups (vec *groups) +static void +omp_index_mapping_groups_1 (hash_map *grpmap, + vec *groups, + tree reindex_sentinel) { - hash_map *grpmap - = new hash_map; - 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; @@ -9372,8 +9387,7 @@ omp_index_mapping_groups (vec *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); @@ -9402,7 +9416,7 @@ omp_index_mapping_groups (vec *groups) continue; omp_mapping_group **prev = grpmap->get (fpp); - if (prev) + if (prev && *prev != grp) { grp->sibling = (*prev)->sibling; (*prev)->sibling = grp; @@ -9410,6 +9424,48 @@ omp_index_mapping_groups (vec *groups) else grpmap->put (fpp, grp); } +} + +static hash_map * +omp_index_mapping_groups (vec *groups) +{ + hash_map *grpmap + = new hash_map; + + 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 * +omp_reindex_mapping_groups (tree *list_p, + vec *groups, + vec *processed_groups, + tree sentinel) +{ + hash_map *grpmap + = new hash_map; + + 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; } @@ -9437,6 +9493,45 @@ omp_containing_struct (tree expr) return expr; } +/* Return TRUE if DECL describes a component that is part of a whole structure + that is mapped elsewhere in GRPMAP. *MAPPED_BY_GROUP is set to the group + that maps that structure, if present. */ + +static bool +omp_mapped_by_containing_struct (hash_map *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. */ @@ -9464,9 +9559,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; @@ -9483,25 +9577,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) { @@ -10285,7 +10370,8 @@ omp_build_struct_sibling_lists (enum tree_code code, enum omp_region_type region_type, vec *groups, hash_map - **grpmap) + **grpmap, + tree *list_p) { unsigned i; omp_mapping_group *grp; @@ -10293,16 +10379,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 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; @@ -10342,36 +10434,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; } @@ -10416,7 +10488,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; @@ -10430,13 +10501,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; @@ -10497,7 +10594,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, hash_map *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; @@ -10532,7 +10630,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, hash_map *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;