From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 5C98C382D45A for ; Tue, 24 May 2022 13:18:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5C98C382D45A Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-623-5PbUvHcUPoCLU2oTcf_OVA-1; Tue, 24 May 2022 09:17:40 -0400 X-MC-Unique: 5PbUvHcUPoCLU2oTcf_OVA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5353C803D5B; Tue, 24 May 2022 13:17:40 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0E6C0492C3B; Tue, 24 May 2022 13:17:39 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 24ODHbPa2106995 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 24 May 2022 15:17:37 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24ODHaeh2106994; Tue, 24 May 2022 15:17:36 +0200 Date: Tue, 24 May 2022 15:17:36 +0200 From: Jakub Jelinek To: Julian Brown Cc: gcc-patches@gcc.gnu.org, Thomas Schwinge , Tobias Burnus , Fortran List Subject: Re: [PATCH v2 03/11] OpenMP/OpenACC struct sibling list gimplification extension and rework Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 May 2022 13:18:42 -0000 On Fri, Mar 18, 2022 at 09:24:53AM -0700, Julian Brown wrote: > 2022-03-17 Julian Brown > > gcc/fortran/ > * trans-openmp.cc (gfc_trans_omp_clauses): Don't create > GOMP_MAP_TO_PSET mappings for class metadata, nor GOMP_MAP_POINTER > mappings for POINTER_TYPE_P decls. > > gcc/ > * gimplify.c (gimplify_omp_var_data): Remove GOVD_MAP_HAS_ATTACHMENTS. > (insert_struct_comp_map): Refactor function into... > (build_struct_comp_nodes): This new function. Remove list handling > and improve self-documentation. > (extract_base_bit_offset): Remove BASE_REF, OFFSETP parameters. Move > code to strip outer parts of address out of function, but strip no-op > conversions. > (omp_mapping_group): Add DELETED field for use during reindexing. > (strip_components_and_deref, strip_indirections): New functions. > (omp_group_last, omp_group_base): Add GOMP_MAP_STRUCT handling. > (omp_gather_mapping_groups): Initialise DELETED field for new groups. > (omp_index_mapping_groups): Notice DELETED groups when (re)indexing. > (insert_node_after, move_node_after, move_nodes_after, > move_concat_nodes_after): New helper functions. > (accumulate_sibling_list): New function to build up GOMP_MAP_STRUCT > node groups for sibling lists. Outlined from gimplify_scan_omp_clauses. > (omp_build_struct_sibling_lists): New function. > (gimplify_scan_omp_clauses): Remove struct_map_to_clause, > struct_seen_clause, struct_deref_set. Call > omp_build_struct_sibling_lists as pre-pass instead of handling sibling > lists in the function's main processing loop. > (gimplify_adjust_omp_clauses_1): Remove GOVD_MAP_HAS_ATTACHMENTS > handling, unused now. > * omp-low.cc (scan_sharing_clauses): Handle pointer-type indirect > struct references, and references to pointers to structs also. > > gcc/testsuite/ > * g++.dg/goacc/member-array-acc.C: New test. > * g++.dg/gomp/member-array-omp.C: New test. > * g++.dg/gomp/target-3.C: Update expected output. > * g++.dg/gomp/target-lambda-1.C: Likewise. > * g++.dg/gomp/target-this-2.C: Likewise. > * c-c++-common/goacc/deep-copy-arrayofstruct.c: Move test from here. > > libgomp/ > * testsuite/libgomp.oacc-c-c++-common/deep-copy-15.c: New test. > * testsuite/libgomp.oacc-c-c++-common/deep-copy-16.c: New test. > * testsuite/libgomp.oacc-c++/deep-copy-17.C: New test. > * testsuite/libgomp.oacc-c-c++-common/deep-copy-arrayofstruct.c: Move > test to here, make "run" test. > --- a/gcc/gimplify.cc > +++ b/gcc/gimplify.cc > @@ -125,10 +125,6 @@ enum gimplify_omp_var_data > /* Flag for GOVD_REDUCTION: inscan seen in {in,ex}clusive clause. */ > GOVD_REDUCTION_INSCAN = 0x2000000, > > - /* Flag for GOVD_MAP: (struct) vars that have pointer attachments for > - fields. */ > - GOVD_MAP_HAS_ATTACHMENTS = 0x4000000, > - > /* Flag for GOVD_FIRSTPRIVATE: OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT. */ > GOVD_FIRSTPRIVATE_IMPLICIT = 0x8000000, I'd renumber the GOVD_* constants after this, otherwise we won't remember we've left a gap. > + (or derived type, etc.) component, create an "alloc" or "release" node to > + insert into a list following a GOMP_MAP_STRUCT node. For some types of > + mapping (e.g. Fortran arrays with descriptors), an additional mapping may > + be created that is inserted into the list of mapping nodes attached to the > + directive being processed -- not part of the sorted list of nodes after > + GOMP_MAP_STRUCT. > + > + CODE is the code of the directive being processed. GRP_START and GRP_END > + are the first and last of two or three nodes representing this array section > + mapping (e.g. a data movement node like GOMP_MAP_{TO,FROM}, optionally a > + GOMP_MAP_TO_PSET, and finally a GOMP_MAP_ALWAYS_POINTER). EXTRA_NODE is > + filled with the additional node described above, if needed. > + > + This function does not add the new nodes to any lists itself. It is the > + responsibility of the caller to do that. */ > > static tree > -insert_struct_comp_map (enum tree_code code, tree c, tree struct_node, > - tree prev_node, tree *scp) > +build_struct_comp_nodes (enum tree_code code, tree grp_start, tree grp_end, > + tree *extra_node) I think it would be nice to use omp_ prefixes even for these static functions, this is all in the gimplifier, so it should be clear that it isn't some generic code but OpenMP specific gimplification code. Another variant would be to introduce omp-gimplify.cc and move lots of stuff there, but if we do that, best time might be during stage3 so that it doesn't collide with too many patches. > > +/* Link node NEWNODE so it is pointed to by chain INSERT_AT. NEWNODE's chain > + is linked to the previous node pointed to by INSERT_AT. */ > + > +static tree * > +insert_node_after (tree newnode, tree *insert_at) > +{ > + OMP_CLAUSE_CHAIN (newnode) = *insert_at; > + *insert_at = newnode; > + return &OMP_CLAUSE_CHAIN (newnode); > +} Including these... (etc.) The names are too generic for what they do. > +static bool > +omp_build_struct_sibling_lists (enum tree_code code, > + enum omp_region_type region_type, > + vec *groups, > + hash_map Perhaps better wrap on the , align omp_mapping_group * below tree_operand_hash and then **grpmap will fit. > + **grpmap) Otherwise LGTM. Jakub