| Message ID | 20191006223237.81842-4-julian@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | OpenACC 2.6 manual deep copy (repost) | expand |
Hi Julian! On 2019-10-06T15:32:36-0700, Julian Brown <julian@codesourcery.com> wrote: > This patch factors out some code in gimplify_scan_omp_clauses into two > new outlined functions. Yay for such simplification, and yay for documenting what's going on! > Previously approved here: > > https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01309.html > > FAOD, OK for trunk? I have not reviewed whether it's a suitable refactoring; I'm not at all familiar with that code. I started to "functionally" review it by re-inlining your outlined code, and checking the differences to the original code. Additionally to the 'gcc_assert' item I just raised with Jakub, there are a few more things I don't understand: > gcc/ > * gimplify.c (insert_struct_comp_map, check_base_and_compare_lt): New. > (gimplify_scan_omp_clauses): Outline duplicated code into calls to > above two functions. > --- > gcc/gimplify.c | 307 ++++++++++++++++++++++++++++--------------------- > 1 file changed, 174 insertions(+), 133 deletions(-) > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -8120,6 +8120,160 @@ gimplify_omp_depend (tree *list_p, gimple_seq *pre_p) > return 1; > } > > +/* Insert a GOMP_MAP_ALLOC or GOMP_MAP_RELEASE node following a > + GOMP_MAP_STRUCT mapping. C is an always_pointer mapping. STRUCT_NODE is > + the struct node to insert the new mapping after (when the struct node is > + initially created). PREV_NODE is the first of two or three mappings for a > + pointer, and is either: > + - the node before C, when a pair of mappings is used, e.g. for a C/C++ > + array section. > + - not the node before C. This is true when we have a reference-to-pointer > + type (with a mapping for the reference and for the pointer), or for > + Fortran derived-type mappings with a GOMP_MAP_TO_PSET. > + If SCP is non-null, the new node is inserted before *SCP. > + if SCP is null, the new node is inserted before PREV_NODE. > + The return type is: > + - PREV_NODE, if SCP is non-null. > + - The newly-created ALLOC or RELEASE node, if SCP is null. > + - The second newly-created ALLOC or RELEASE node, if we are mapping a > + reference to a pointer. */ > + > +static tree > +insert_struct_comp_map (enum tree_code code, tree c, tree struct_node, > + tree prev_node, tree *scp) > +{ > + enum gomp_map_kind mkind > + = ((code == OMP_TARGET_EXIT_DATA || code == OACC_EXIT_DATA) > + ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC); The original code does not handle 'OACC_EXIT_DATA', so that will be a change separate from this refactoring? > + > + tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP); > + tree cl = scp ? prev_node : c2; > + OMP_CLAUSE_SET_MAP_KIND (c2, mkind); > + OMP_CLAUSE_DECL (c2) = unshare_expr (OMP_CLAUSE_DECL (c)); > + OMP_CLAUSE_CHAIN (c2) = scp ? *scp : prev_node; > + OMP_CLAUSE_SIZE (c2) = TYPE_SIZE_UNIT (ptr_type_node); > + if (struct_node) > + OMP_CLAUSE_CHAIN (struct_node) = c2; > + > + /* We might need to create an additional mapping if we have a reference to a > + pointer (in C++). Don't do this if we have something other than a > + GOMP_MAP_ALWAYS_POINTER though, i.e. a GOMP_MAP_TO_PSET. */ > + if (OMP_CLAUSE_CHAIN (prev_node) != c > + && OMP_CLAUSE_CODE (OMP_CLAUSE_CHAIN (prev_node)) == OMP_CLAUSE_MAP > + && (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (prev_node)) > + == GOMP_MAP_ALWAYS_POINTER)) In the original code, and with 'prev_node' being '*prev_list_p', this condition was just 'if (OMP_CLAUSE_CHAIN (*prev_list_p) != c)', without the 'GOMP_MAP_ALWAYS_POINTER' handling, so that'd be another change separate from this refactoring? > + { > + tree c4 = OMP_CLAUSE_CHAIN (prev_node); > + tree c3 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP); > + OMP_CLAUSE_SET_MAP_KIND (c3, mkind); > + OMP_CLAUSE_DECL (c3) = unshare_expr (OMP_CLAUSE_DECL (c4)); > + OMP_CLAUSE_SIZE (c3) = TYPE_SIZE_UNIT (ptr_type_node); > + OMP_CLAUSE_CHAIN (c3) = prev_node; > + if (!scp) > + OMP_CLAUSE_CHAIN (c2) = c3; > + else > + cl = c3; > + } > + > + if (scp) > + *scp = c2; > + > + return cl; > +} > + > +/* Called initially with ORIG_BASE non-null, sets PREV_BITPOS and PREV_POFFSET > + to the offset of the field given in BASE. Return type is 1 if BASE is equal > + to *ORIG_BASE after stripping off ARRAY_REF and INDIRECT_REF nodes and > + calling get_inner_reference, else 0. > + > + Called subsequently with ORIG_BASE null, compares the offset of the field > + given in BASE to PREV_BITPOS, PREV_POFFSET. Returns -1 if the base object > + has changed, 0 if the new value has a higher bit position than that > + described by the aforementioned arguments, or 1 if the new value is less > + than them. Used for (insertion) sorting components after a GOMP_MAP_STRUCT > + mapping. */ Hmm, that doesn't seem to be the most intuitive API: the 'orig_base' handling, specifically. (See my struggle below.) > + > +static int > +check_base_and_compare_lt (tree base, tree *orig_base, tree decl, > + poly_int64 *prev_bitpos, > + poly_offset_int *prev_poffset) > +{ > + tree offset; > + poly_int64 bitsize, bitpos; > + machine_mode mode; > + int unsignedp, reversep, volatilep = 0; > + poly_offset_int poffset; > + > + if (orig_base) > + { > + while (TREE_CODE (base) == ARRAY_REF) > + base = TREE_OPERAND (base, 0); > + > + if (TREE_CODE (base) == INDIRECT_REF) > + base = TREE_OPERAND (base, 0); > + } > + else > + { > + if (TREE_CODE (base) == ARRAY_REF) > + { > + while (TREE_CODE (base) == ARRAY_REF) > + base = TREE_OPERAND (base, 0); > + if (TREE_CODE (base) != COMPONENT_REF > + || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE) > + return -1; > + } > + else if (TREE_CODE (base) == INDIRECT_REF > + && TREE_CODE (TREE_OPERAND (base, 0)) == COMPONENT_REF > + && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) > + == REFERENCE_TYPE)) > + base = TREE_OPERAND (base, 0); > + } > + > + base = get_inner_reference (base, &bitsize, &bitpos, &offset, &mode, > + &unsignedp, &reversep, &volatilep); > + > + if (orig_base) > + *orig_base = base; > + > + if ((TREE_CODE (base) == INDIRECT_REF > + || (TREE_CODE (base) == MEM_REF > + && integer_zerop (TREE_OPERAND (base, 1)))) > + && DECL_P (TREE_OPERAND (base, 0)) > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) == REFERENCE_TYPE) > + base = TREE_OPERAND (base, 0); > + > + gcc_assert (offset == NULL_TREE || poly_int_tree_p (offset)); > + > + if (offset) > + poffset = wi::to_poly_offset (offset); > + else > + poffset = 0; > + > + if (maybe_ne (bitpos, 0)) > + poffset += bits_to_bytes_round_down (bitpos); > + I find the block of code following below until the end of the function a bit unwieldy to (a) understand, and (b) relate to the original code: > + if (orig_base) > + { > + gcc_assert (base == decl); > + > + *prev_bitpos = bitpos; > + *prev_poffset = poffset; > + > + return *orig_base == base; > + } > + else > + { > + if (base != decl) > + return -1; > + > + return (maybe_lt (*prev_poffset, poffset) > + || (known_eq (*prev_poffset, poffset) > + && maybe_lt (*prev_bitpos, bitpos))); > + } > + > + return 0; > +} Can this be done differently, to make it easier to understand? For example, would it help if that code was not outlined into 'insert_struct_comp_map', but kept in its original places? At least on first sight that seemsa reasonable thing to do, given that it's just two separate variants/code paths (as guarded by 'if (orig_base)'), where the first of two calls takes the first branch ('orig_base' supplied), and the other caller takes the second branch ('orig_base' not supplied). If you tell me that's not feasible, then I shall again try to understand that code in the form you've proposed. Grüße Thomas > @@ -8660,29 +8814,14 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > } > } > > - tree offset; > - poly_int64 bitsize, bitpos; > - machine_mode mode; > - int unsignedp, reversep, volatilep = 0; > - tree base = OMP_CLAUSE_DECL (c); > - while (TREE_CODE (base) == ARRAY_REF) > - base = TREE_OPERAND (base, 0); > - if (TREE_CODE (base) == INDIRECT_REF) > - base = TREE_OPERAND (base, 0); > - base = get_inner_reference (base, &bitsize, &bitpos, &offset, > - &mode, &unsignedp, &reversep, > - &volatilep); > - tree orig_base = base; > - if ((TREE_CODE (base) == INDIRECT_REF > - || (TREE_CODE (base) == MEM_REF > - && integer_zerop (TREE_OPERAND (base, 1)))) > - && DECL_P (TREE_OPERAND (base, 0)) > - && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) > - == REFERENCE_TYPE)) > - base = TREE_OPERAND (base, 0); > - gcc_assert (base == decl > - && (offset == NULL_TREE > - || poly_int_tree_p (offset))); > + tree orig_base; > + poly_int64 bitpos1; > + poly_offset_int offset1; > + > + int base_eq_orig_base > + = check_base_and_compare_lt (OMP_CLAUSE_DECL (c), > + &orig_base, decl, &bitpos1, > + &offset1); > > splay_tree_node n > = splay_tree_lookup (ctx->variables, (splay_tree_key)decl); > @@ -8693,7 +8832,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > tree l = build_omp_clause (OMP_CLAUSE_LOCATION (c), > OMP_CLAUSE_MAP); > OMP_CLAUSE_SET_MAP_KIND (l, GOMP_MAP_STRUCT); > - if (orig_base != base) > + if (!base_eq_orig_base) > OMP_CLAUSE_DECL (l) = unshare_expr (orig_base); > else > OMP_CLAUSE_DECL (l) = decl; > @@ -8703,32 +8842,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > struct_map_to_clause->put (decl, l); > if (ptr) > { > - enum gomp_map_kind mkind > - = code == OMP_TARGET_EXIT_DATA > - ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC; > - tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), > - OMP_CLAUSE_MAP); > - OMP_CLAUSE_SET_MAP_KIND (c2, mkind); > - OMP_CLAUSE_DECL (c2) > - = unshare_expr (OMP_CLAUSE_DECL (c)); > - OMP_CLAUSE_CHAIN (c2) = *prev_list_p; > - OMP_CLAUSE_SIZE (c2) > - = TYPE_SIZE_UNIT (ptr_type_node); > - OMP_CLAUSE_CHAIN (l) = c2; > - if (OMP_CLAUSE_CHAIN (*prev_list_p) != c) > - { > - tree c4 = OMP_CLAUSE_CHAIN (*prev_list_p); > - tree c3 > - = build_omp_clause (OMP_CLAUSE_LOCATION (c), > - OMP_CLAUSE_MAP); > - OMP_CLAUSE_SET_MAP_KIND (c3, mkind); > - OMP_CLAUSE_DECL (c3) > - = unshare_expr (OMP_CLAUSE_DECL (c4)); > - OMP_CLAUSE_SIZE (c3) > - = TYPE_SIZE_UNIT (ptr_type_node); > - OMP_CLAUSE_CHAIN (c3) = *prev_list_p; > - OMP_CLAUSE_CHAIN (c2) = c3; > - } > + insert_struct_comp_map (code, c, l, *prev_list_p, > + NULL); > *prev_list_p = l; > prev_list_p = NULL; > } > @@ -8738,7 +8853,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > *list_p = l; > list_p = &OMP_CLAUSE_CHAIN (l); > } > - if (orig_base != base && code == OMP_TARGET) > + if (!base_eq_orig_base && code == OMP_TARGET) > { > tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), > OMP_CLAUSE_MAP); > @@ -8761,13 +8876,6 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > tree *sc = NULL, *scp = NULL; > if (GOMP_MAP_ALWAYS_P (OMP_CLAUSE_MAP_KIND (c)) || ptr) > n->value |= GOVD_SEEN; > - poly_offset_int o1, o2; > - if (offset) > - o1 = wi::to_poly_offset (offset); > - else > - o1 = 0; > - if (maybe_ne (bitpos, 0)) > - o1 += bits_to_bytes_round_down (bitpos); > sc = &OMP_CLAUSE_CHAIN (*osc); > if (*sc != c > && (OMP_CLAUSE_MAP_KIND (*sc) > @@ -8785,44 +8893,14 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > break; > else > { > - tree offset2; > - poly_int64 bitsize2, bitpos2; > - base = OMP_CLAUSE_DECL (*sc); > - if (TREE_CODE (base) == ARRAY_REF) > - { > - while (TREE_CODE (base) == ARRAY_REF) > - base = TREE_OPERAND (base, 0); > - if (TREE_CODE (base) != COMPONENT_REF > - || (TREE_CODE (TREE_TYPE (base)) > - != ARRAY_TYPE)) > - break; > - } > - else if (TREE_CODE (base) == INDIRECT_REF > - && (TREE_CODE (TREE_OPERAND (base, 0)) > - == COMPONENT_REF) > - && (TREE_CODE (TREE_TYPE > - (TREE_OPERAND (base, 0))) > - == REFERENCE_TYPE)) > - base = TREE_OPERAND (base, 0); > - base = get_inner_reference (base, &bitsize2, > - &bitpos2, &offset2, > - &mode, &unsignedp, > - &reversep, &volatilep); > - if ((TREE_CODE (base) == INDIRECT_REF > - || (TREE_CODE (base) == MEM_REF > - && integer_zerop (TREE_OPERAND (base, > - 1)))) > - && DECL_P (TREE_OPERAND (base, 0)) > - && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, > - 0))) > - == REFERENCE_TYPE)) > - base = TREE_OPERAND (base, 0); > - if (base != decl) > + tree sc_decl = OMP_CLAUSE_DECL (*sc); > + int same_decl_offset_lt > + = check_base_and_compare_lt (sc_decl, NULL, decl, > + &bitpos1, &offset1); > + if (same_decl_offset_lt == -1) > break; > if (scp) > continue; > - gcc_assert (offset == NULL_TREE > - || poly_int_tree_p (offset)); > tree d1 = OMP_CLAUSE_DECL (*sc); > tree d2 = OMP_CLAUSE_DECL (c); > while (TREE_CODE (d1) == ARRAY_REF) > @@ -8851,14 +8929,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > remove = true; > break; > } > - if (offset2) > - o2 = wi::to_poly_offset (offset2); > - else > - o2 = 0; > - o2 += bits_to_bytes_round_down (bitpos2); > - if (maybe_lt (o1, o2) > - || (known_eq (o1, o2) > - && maybe_lt (bitpos, bitpos2))) > + if (same_decl_offset_lt) > { > if (ptr) > scp = sc; > @@ -8873,38 +8944,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, > size_one_node); > if (ptr) > { > - tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), > - OMP_CLAUSE_MAP); > - tree cl = NULL_TREE; > - enum gomp_map_kind mkind > - = code == OMP_TARGET_EXIT_DATA > - ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC; > - OMP_CLAUSE_SET_MAP_KIND (c2, mkind); > - OMP_CLAUSE_DECL (c2) > - = unshare_expr (OMP_CLAUSE_DECL (c)); > - OMP_CLAUSE_CHAIN (c2) = scp ? *scp : *prev_list_p; > - OMP_CLAUSE_SIZE (c2) > - = TYPE_SIZE_UNIT (ptr_type_node); > - cl = scp ? *prev_list_p : c2; > - if (OMP_CLAUSE_CHAIN (*prev_list_p) != c) > - { > - tree c4 = OMP_CLAUSE_CHAIN (*prev_list_p); > - tree c3 > - = build_omp_clause (OMP_CLAUSE_LOCATION (c), > - OMP_CLAUSE_MAP); > - OMP_CLAUSE_SET_MAP_KIND (c3, mkind); > - OMP_CLAUSE_DECL (c3) > - = unshare_expr (OMP_CLAUSE_DECL (c4)); > - OMP_CLAUSE_SIZE (c3) > - = TYPE_SIZE_UNIT (ptr_type_node); > - OMP_CLAUSE_CHAIN (c3) = *prev_list_p; > - if (!scp) > - OMP_CLAUSE_CHAIN (c2) = c3; > - else > - cl = c3; > - } > - if (scp) > - *scp = c2; > + tree cl = insert_struct_comp_map (code, c, NULL, > + *prev_list_p, scp); > if (sc == prev_list_p) > { > *sc = cl; Grüße Thomas
Hi Thomas! Here's a new version of the patch, with your comments below hopefully addressed. In particular the check_base_and_compare_lt function with the "weird" API has been replaced by a new, simpler extract_base_bit_offset function. Going back and re-examining the code, I think it makes more sense this way. (The comparison "orig_base != base" in the original code is checking if the base is a reference that has been indirected: I attempted to make that self-document better in this version.) On Wed, 16 Oct 2019 16:42:03 +0200 Thomas Schwinge <thomas@codesourcery.com> wrote: > The original code does not handle 'OACC_EXIT_DATA', so that will be a > change separate from this refactoring? Looks like that crept in during merging/rebasing. I've moved it to the manual deep copy patch where it belongs (to be reposted). > > + /* We might need to create an additional mapping if we have a > > reference to a > > + pointer (in C++). Don't do this if we have something other > > than a > > + GOMP_MAP_ALWAYS_POINTER though, i.e. a GOMP_MAP_TO_PSET. */ > > + if (OMP_CLAUSE_CHAIN (prev_node) != c > > + && OMP_CLAUSE_CODE (OMP_CLAUSE_CHAIN (prev_node)) == > > OMP_CLAUSE_MAP > > + && (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (prev_node)) > > + == GOMP_MAP_ALWAYS_POINTER)) > > In the original code, and with 'prev_node' being '*prev_list_p', this > condition was just 'if (OMP_CLAUSE_CHAIN (*prev_list_p) != c)', > without the 'GOMP_MAP_ALWAYS_POINTER' handling, so that'd be another > change separate from this refactoring? Again, moved. > > +/* Called initially with ORIG_BASE non-null, sets PREV_BITPOS and > > PREV_POFFSET > > + to the offset of the field given in BASE. Return type is 1 if > > BASE is equal > > + to *ORIG_BASE after stripping off ARRAY_REF and INDIRECT_REF > > nodes and > > + calling get_inner_reference, else 0. > > + > > + Called subsequently with ORIG_BASE null, compares the offset of > > the field > > + given in BASE to PREV_BITPOS, PREV_POFFSET. Returns -1 if the > > base object > > + has changed, 0 if the new value has a higher bit position than > > that > > + described by the aforementioned arguments, or 1 if the new > > value is less > > + than them. Used for (insertion) sorting components after a > > GOMP_MAP_STRUCT > > + mapping. */ > > Hmm, that doesn't seem to be the most intuitive API: the 'orig_base' > handling, specifically. (See my struggle below.) The new function just strips array refs or indirect refs off base, then extracts the bit offset of the inner reference. It's still worth it to de-duplicate that code, I think. The comparison... > Can this be done differently, to make it easier to understand? For > example, would it help if that code was not outlined into > 'insert_struct_comp_map', but kept in its original places? At least > on first sight that seemsa reasonable thing to do, given that it's > just two separate variants/code paths (as guarded by 'if > (orig_base)'), where the first of two calls takes the first branch > ('orig_base' supplied), and the other caller takes the second branch > ('orig_base' not supplied). If you tell me that's not feasible, then > I shall again try to understand that code in the form you've proposed. ...is left outside the refactored function, as you suggest here. Thanks for review! Does this version look OK? Re-tested with offloading to nvptx. Thanks, Julian
diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 88d6571976f..5d50713a9f1 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -8120,6 +8120,160 @@ gimplify_omp_depend (tree *list_p, gimple_seq *pre_p) return 1; } +/* Insert a GOMP_MAP_ALLOC or GOMP_MAP_RELEASE node following a + GOMP_MAP_STRUCT mapping. C is an always_pointer mapping. STRUCT_NODE is + the struct node to insert the new mapping after (when the struct node is + initially created). PREV_NODE is the first of two or three mappings for a + pointer, and is either: + - the node before C, when a pair of mappings is used, e.g. for a C/C++ + array section. + - not the node before C. This is true when we have a reference-to-pointer + type (with a mapping for the reference and for the pointer), or for + Fortran derived-type mappings with a GOMP_MAP_TO_PSET. + If SCP is non-null, the new node is inserted before *SCP. + if SCP is null, the new node is inserted before PREV_NODE. + The return type is: + - PREV_NODE, if SCP is non-null. + - The newly-created ALLOC or RELEASE node, if SCP is null. + - The second newly-created ALLOC or RELEASE node, if we are mapping a + reference to a pointer. */ + +static tree +insert_struct_comp_map (enum tree_code code, tree c, tree struct_node, + tree prev_node, tree *scp) +{ + enum gomp_map_kind mkind + = ((code == OMP_TARGET_EXIT_DATA || code == OACC_EXIT_DATA) + ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC); + + tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP); + tree cl = scp ? prev_node : c2; + OMP_CLAUSE_SET_MAP_KIND (c2, mkind); + OMP_CLAUSE_DECL (c2) = unshare_expr (OMP_CLAUSE_DECL (c)); + OMP_CLAUSE_CHAIN (c2) = scp ? *scp : prev_node; + OMP_CLAUSE_SIZE (c2) = TYPE_SIZE_UNIT (ptr_type_node); + if (struct_node) + OMP_CLAUSE_CHAIN (struct_node) = c2; + + /* We might need to create an additional mapping if we have a reference to a + pointer (in C++). Don't do this if we have something other than a + GOMP_MAP_ALWAYS_POINTER though, i.e. a GOMP_MAP_TO_PSET. */ + if (OMP_CLAUSE_CHAIN (prev_node) != c + && OMP_CLAUSE_CODE (OMP_CLAUSE_CHAIN (prev_node)) == OMP_CLAUSE_MAP + && (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (prev_node)) + == GOMP_MAP_ALWAYS_POINTER)) + { + tree c4 = OMP_CLAUSE_CHAIN (prev_node); + tree c3 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP); + OMP_CLAUSE_SET_MAP_KIND (c3, mkind); + OMP_CLAUSE_DECL (c3) = unshare_expr (OMP_CLAUSE_DECL (c4)); + OMP_CLAUSE_SIZE (c3) = TYPE_SIZE_UNIT (ptr_type_node); + OMP_CLAUSE_CHAIN (c3) = prev_node; + if (!scp) + OMP_CLAUSE_CHAIN (c2) = c3; + else + cl = c3; + } + + if (scp) + *scp = c2; + + return cl; +} + +/* Called initially with ORIG_BASE non-null, sets PREV_BITPOS and PREV_POFFSET + to the offset of the field given in BASE. Return type is 1 if BASE is equal + to *ORIG_BASE after stripping off ARRAY_REF and INDIRECT_REF nodes and + calling get_inner_reference, else 0. + + Called subsequently with ORIG_BASE null, compares the offset of the field + given in BASE to PREV_BITPOS, PREV_POFFSET. Returns -1 if the base object + has changed, 0 if the new value has a higher bit position than that + described by the aforementioned arguments, or 1 if the new value is less + than them. Used for (insertion) sorting components after a GOMP_MAP_STRUCT + mapping. */ + +static int +check_base_and_compare_lt (tree base, tree *orig_base, tree decl, + poly_int64 *prev_bitpos, + poly_offset_int *prev_poffset) +{ + tree offset; + poly_int64 bitsize, bitpos; + machine_mode mode; + int unsignedp, reversep, volatilep = 0; + poly_offset_int poffset; + + if (orig_base) + { + while (TREE_CODE (base) == ARRAY_REF) + base = TREE_OPERAND (base, 0); + + if (TREE_CODE (base) == INDIRECT_REF) + base = TREE_OPERAND (base, 0); + } + else + { + if (TREE_CODE (base) == ARRAY_REF) + { + while (TREE_CODE (base) == ARRAY_REF) + base = TREE_OPERAND (base, 0); + if (TREE_CODE (base) != COMPONENT_REF + || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE) + return -1; + } + else if (TREE_CODE (base) == INDIRECT_REF + && TREE_CODE (TREE_OPERAND (base, 0)) == COMPONENT_REF + && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) + == REFERENCE_TYPE)) + base = TREE_OPERAND (base, 0); + } + + base = get_inner_reference (base, &bitsize, &bitpos, &offset, &mode, + &unsignedp, &reversep, &volatilep); + + if (orig_base) + *orig_base = base; + + if ((TREE_CODE (base) == INDIRECT_REF + || (TREE_CODE (base) == MEM_REF + && integer_zerop (TREE_OPERAND (base, 1)))) + && DECL_P (TREE_OPERAND (base, 0)) + && TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) == REFERENCE_TYPE) + base = TREE_OPERAND (base, 0); + + gcc_assert (offset == NULL_TREE || poly_int_tree_p (offset)); + + if (offset) + poffset = wi::to_poly_offset (offset); + else + poffset = 0; + + if (maybe_ne (bitpos, 0)) + poffset += bits_to_bytes_round_down (bitpos); + + if (orig_base) + { + gcc_assert (base == decl); + + *prev_bitpos = bitpos; + *prev_poffset = poffset; + + return *orig_base == base; + } + else + { + if (base != decl) + return -1; + + return (maybe_lt (*prev_poffset, poffset) + || (known_eq (*prev_poffset, poffset) + && maybe_lt (*prev_bitpos, bitpos))); + } + + return 0; +} + /* Scan the OMP clauses in *LIST_P, installing mappings into a new and previous omp contexts. */ @@ -8660,29 +8814,14 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, } } - tree offset; - poly_int64 bitsize, bitpos; - machine_mode mode; - int unsignedp, reversep, volatilep = 0; - tree base = OMP_CLAUSE_DECL (c); - while (TREE_CODE (base) == ARRAY_REF) - base = TREE_OPERAND (base, 0); - if (TREE_CODE (base) == INDIRECT_REF) - base = TREE_OPERAND (base, 0); - base = get_inner_reference (base, &bitsize, &bitpos, &offset, - &mode, &unsignedp, &reversep, - &volatilep); - tree orig_base = base; - if ((TREE_CODE (base) == INDIRECT_REF - || (TREE_CODE (base) == MEM_REF - && integer_zerop (TREE_OPERAND (base, 1)))) - && DECL_P (TREE_OPERAND (base, 0)) - && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) - == REFERENCE_TYPE)) - base = TREE_OPERAND (base, 0); - gcc_assert (base == decl - && (offset == NULL_TREE - || poly_int_tree_p (offset))); + tree orig_base; + poly_int64 bitpos1; + poly_offset_int offset1; + + int base_eq_orig_base + = check_base_and_compare_lt (OMP_CLAUSE_DECL (c), + &orig_base, decl, &bitpos1, + &offset1); splay_tree_node n = splay_tree_lookup (ctx->variables, (splay_tree_key)decl); @@ -8693,7 +8832,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, tree l = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP); OMP_CLAUSE_SET_MAP_KIND (l, GOMP_MAP_STRUCT); - if (orig_base != base) + if (!base_eq_orig_base) OMP_CLAUSE_DECL (l) = unshare_expr (orig_base); else OMP_CLAUSE_DECL (l) = decl; @@ -8703,32 +8842,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, struct_map_to_clause->put (decl, l); if (ptr) { - enum gomp_map_kind mkind - = code == OMP_TARGET_EXIT_DATA - ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC; - tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), - OMP_CLAUSE_MAP); - OMP_CLAUSE_SET_MAP_KIND (c2, mkind); - OMP_CLAUSE_DECL (c2) - = unshare_expr (OMP_CLAUSE_DECL (c)); - OMP_CLAUSE_CHAIN (c2) = *prev_list_p; - OMP_CLAUSE_SIZE (c2) - = TYPE_SIZE_UNIT (ptr_type_node); - OMP_CLAUSE_CHAIN (l) = c2; - if (OMP_CLAUSE_CHAIN (*prev_list_p) != c) - { - tree c4 = OMP_CLAUSE_CHAIN (*prev_list_p); - tree c3 - = build_omp_clause (OMP_CLAUSE_LOCATION (c), - OMP_CLAUSE_MAP); - OMP_CLAUSE_SET_MAP_KIND (c3, mkind); - OMP_CLAUSE_DECL (c3) - = unshare_expr (OMP_CLAUSE_DECL (c4)); - OMP_CLAUSE_SIZE (c3) - = TYPE_SIZE_UNIT (ptr_type_node); - OMP_CLAUSE_CHAIN (c3) = *prev_list_p; - OMP_CLAUSE_CHAIN (c2) = c3; - } + insert_struct_comp_map (code, c, l, *prev_list_p, + NULL); *prev_list_p = l; prev_list_p = NULL; } @@ -8738,7 +8853,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, *list_p = l; list_p = &OMP_CLAUSE_CHAIN (l); } - if (orig_base != base && code == OMP_TARGET) + if (!base_eq_orig_base && code == OMP_TARGET) { tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP); @@ -8761,13 +8876,6 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, tree *sc = NULL, *scp = NULL; if (GOMP_MAP_ALWAYS_P (OMP_CLAUSE_MAP_KIND (c)) || ptr) n->value |= GOVD_SEEN; - poly_offset_int o1, o2; - if (offset) - o1 = wi::to_poly_offset (offset); - else - o1 = 0; - if (maybe_ne (bitpos, 0)) - o1 += bits_to_bytes_round_down (bitpos); sc = &OMP_CLAUSE_CHAIN (*osc); if (*sc != c && (OMP_CLAUSE_MAP_KIND (*sc) @@ -8785,44 +8893,14 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, break; else { - tree offset2; - poly_int64 bitsize2, bitpos2; - base = OMP_CLAUSE_DECL (*sc); - if (TREE_CODE (base) == ARRAY_REF) - { - while (TREE_CODE (base) == ARRAY_REF) - base = TREE_OPERAND (base, 0); - if (TREE_CODE (base) != COMPONENT_REF - || (TREE_CODE (TREE_TYPE (base)) - != ARRAY_TYPE)) - break; - } - else if (TREE_CODE (base) == INDIRECT_REF - && (TREE_CODE (TREE_OPERAND (base, 0)) - == COMPONENT_REF) - && (TREE_CODE (TREE_TYPE - (TREE_OPERAND (base, 0))) - == REFERENCE_TYPE)) - base = TREE_OPERAND (base, 0); - base = get_inner_reference (base, &bitsize2, - &bitpos2, &offset2, - &mode, &unsignedp, - &reversep, &volatilep); - if ((TREE_CODE (base) == INDIRECT_REF - || (TREE_CODE (base) == MEM_REF - && integer_zerop (TREE_OPERAND (base, - 1)))) - && DECL_P (TREE_OPERAND (base, 0)) - && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, - 0))) - == REFERENCE_TYPE)) - base = TREE_OPERAND (base, 0); - if (base != decl) + tree sc_decl = OMP_CLAUSE_DECL (*sc); + int same_decl_offset_lt + = check_base_and_compare_lt (sc_decl, NULL, decl, + &bitpos1, &offset1); + if (same_decl_offset_lt == -1) break; if (scp) continue; - gcc_assert (offset == NULL_TREE - || poly_int_tree_p (offset)); tree d1 = OMP_CLAUSE_DECL (*sc); tree d2 = OMP_CLAUSE_DECL (c); while (TREE_CODE (d1) == ARRAY_REF) @@ -8851,14 +8929,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, remove = true; break; } - if (offset2) - o2 = wi::to_poly_offset (offset2); - else - o2 = 0; - o2 += bits_to_bytes_round_down (bitpos2); - if (maybe_lt (o1, o2) - || (known_eq (o1, o2) - && maybe_lt (bitpos, bitpos2))) + if (same_decl_offset_lt) { if (ptr) scp = sc; @@ -8873,38 +8944,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, size_one_node); if (ptr) { - tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), - OMP_CLAUSE_MAP); - tree cl = NULL_TREE; - enum gomp_map_kind mkind - = code == OMP_TARGET_EXIT_DATA - ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC; - OMP_CLAUSE_SET_MAP_KIND (c2, mkind); - OMP_CLAUSE_DECL (c2) - = unshare_expr (OMP_CLAUSE_DECL (c)); - OMP_CLAUSE_CHAIN (c2) = scp ? *scp : *prev_list_p; - OMP_CLAUSE_SIZE (c2) - = TYPE_SIZE_UNIT (ptr_type_node); - cl = scp ? *prev_list_p : c2; - if (OMP_CLAUSE_CHAIN (*prev_list_p) != c) - { - tree c4 = OMP_CLAUSE_CHAIN (*prev_list_p); - tree c3 - = build_omp_clause (OMP_CLAUSE_LOCATION (c), - OMP_CLAUSE_MAP); - OMP_CLAUSE_SET_MAP_KIND (c3, mkind); - OMP_CLAUSE_DECL (c3) - = unshare_expr (OMP_CLAUSE_DECL (c4)); - OMP_CLAUSE_SIZE (c3) - = TYPE_SIZE_UNIT (ptr_type_node); - OMP_CLAUSE_CHAIN (c3) = *prev_list_p; - if (!scp) - OMP_CLAUSE_CHAIN (c2) = c3; - else - cl = c3; - } - if (scp) - *scp = c2; + tree cl = insert_struct_comp_map (code, c, NULL, + *prev_list_p, scp); if (sc == prev_list_p) { *sc = cl;