From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id C7CEA384D1A9; Tue, 13 Sep 2022 21:02:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C7CEA384D1A9 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.93,313,1654588800"; d="scan'208";a="82932926" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 13 Sep 2022 13:02:14 -0800 IronPort-SDR: tTBIq/MQf98ElvedrRVf7Ph/HmsFbcli6zvr/379UimbJx6nRtqTG5u+yMuwKgVKNM8JlAZLwN IfSdB6Jf0xczhjRN4r74m48IIGaCu62SYlubiA5jmJcG5V+EOt3HBnOw5AU4+L70GFqpCuRoNj 7vu1PGqsjeX7RDqsRwgEHY135p4Ehzlz5TbVynv0qBWFaG9TuL55uJWT1gBxucA0jBJTiqo0XR Vb6JfTWgLUHK8RXfnHwjY+HNYeJWWP8rTBMEj5Mx3ooMqPUFH1j3NskSStOgoeMnjnNsDDJeTw E1s= From: Julian Brown To: CC: , Jakub Jelinek , , Subject: [PATCH v3 04/11] OpenMP/OpenACC: mapping group list-handling improvements Date: Tue, 13 Sep 2022 14:01:45 -0700 Message-ID: X-Mailer: git-send-email 2.29.2 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) To svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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_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; @@ -9199,6 +9201,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; @@ -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 * -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; @@ -9378,8 +9393,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); @@ -9408,7 +9422,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; @@ -9416,6 +9430,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; } @@ -9443,6 +9499,41 @@ omp_containing_struct (tree expr) return expr; } +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. */ @@ -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 *groups, hash_map - **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 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 *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 *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