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.133.124]) by sourceware.org (Postfix) with ESMTPS id D4E693858427 for ; Fri, 3 Dec 2021 16:47:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D4E693858427 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-210-GyiMckxHNbWXvuywYZqouQ-1; Fri, 03 Dec 2021 11:47:12 -0500 X-MC-Unique: GyiMckxHNbWXvuywYZqouQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 11499835E20; Fri, 3 Dec 2021 16:47:11 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.194.188]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9F3A95DF21; Fri, 3 Dec 2021 16:47:10 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 1B3Gl7QH3062110 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 3 Dec 2021 17:47:07 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 1B3Gl6OB3062109; Fri, 3 Dec 2021 17:47:06 +0100 Date: Fri, 3 Dec 2021 17:47:06 +0100 From: Jakub Jelinek To: Chung-Lin Tang Cc: gcc-patches , Catherine Moore Subject: Re: [PATCH, v5, OpenMP 5.0] Improve OpenMP target support for C++ [PR92120 v5] Message-ID: <20211203164706.GT2646553@tucnak> Reply-To: Jakub Jelinek References: <20210624131551.GR7746@tucnak> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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=-5.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Dec 2021 16:47:16 -0000 On Tue, Nov 16, 2021 at 08:43:27PM +0800, Chung-Lin Tang wrote: > 2021-11-16 Chung-Lin Tang > > PR middle-end/92120 > > gcc/cp/ChangeLog: > > * cp-tree.h (finish_omp_target): New declaration. > (finish_omp_target_clauses): Likewise. > * parser.c (cp_parser_omp_clause_map): Adjust call to > cp_parser_omp_var_list_no_open to set 'allow_deref' argument to true. > (cp_parser_omp_target): Factor out code, adjust into calls to new > function finish_omp_target. > * pt.c (tsubst_expr): Add call to finish_omp_target_clauses for > OMP_TARGET case. > * semantics.c (handle_omp_array_sections_1): Add handling to create > 'this->member' from 'member' FIELD_DECL. Remove case of rejecting > 'this' when not in declare simd. > (handle_omp_array_sections): Likewise. > (finish_omp_clauses): Likewise. Adjust to allow 'this[]' in OpenMP > map clauses. Handle 'A->member' case in map clauses. Remove case of > rejecting 'this' when not in declare simd. > (struct omp_target_walk_data): New struct for walking over > target-directive tree body. > (finish_omp_target_clauses_r): New function for tree walk. > (finish_omp_target_clauses): New function. > (finish_omp_target): New function. > > gcc/c/ChangeLog: > > * c-parser.c (c_parser_omp_clause_map): Set 'allow_deref' argument in > call to c_parser_omp_variable_list to 'true'. > * c-typeck.c (handle_omp_array_sections_1): Add strip of MEM_REF in > array base handling. > (c_finish_omp_clauses): Handle 'A->member' case in map clauses. > > gcc/ChangeLog: > > * gimplify.c ("tree-hash-traits.h"): Add include. > (gimplify_scan_omp_clauses): Change struct_map_to_clause to type > hash_map *. Adjust struct map handling to handle > cases of *A and A->B expressions. Under !DECL_P case of > GOMP_CLAUSE_MAP handling, add STRIP_NOPS for indir_p case, add to > struct_deref_set for map(*ptr_to_struct) cases. Add MEM_REF case when > handling component_ref_p case. Add unshare_expr and gimplification > when created GOMP_MAP_STRUCT is not a DECL. Add code to add > firstprivate pointer for *pointer-to-struct case. > (gimplify_adjust_omp_clauses): Move GOMP_MAP_STRUCT removal code for > exit data directives code to earlier position. > * omp-low.c (lower_omp_target): > Handle GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION, and > GOMP_MAP_POINTER_TO_ZERO_LENGTH_ARRAY_SECTION map kinds. > * tree-pretty-print.c (dump_omp_clause): Likewise. > > gcc/testsuite/ChangeLog: > > * gcc.dg/gomp/target-3.c: New testcase. > * g++.dg/gomp/target-3.C: New testcase. > * g++.dg/gomp/target-lambda-1.C: New testcase. > * g++.dg/gomp/target-lambda-2.C: New testcase. > * g++.dg/gomp/target-this-1.C: New testcase. > * g++.dg/gomp/target-this-2.C: New testcase. > * g++.dg/gomp/target-this-3.C: New testcase. > * g++.dg/gomp/target-this-4.C: New testcase. > * g++.dg/gomp/target-this-5.C: New testcase. > * g++.dg/gomp/this-2.C: Adjust testcase. > > include/ChangeLog: > > * gomp-constants.h (enum gomp_map_kind): > Add GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION, and > GOMP_MAP_POINTER_TO_ZERO_LENGTH_ARRAY_SECTION map kinds. > (GOMP_MAP_POINTER_P): > Include GOMP_MAP_POINTER_TO_ZERO_LENGTH_ARRAY_SECTION. > > libgomp/ChangeLog: > > * libgomp.h (gomp_attach_pointer): Add bool parameter. > * oacc-mem.c (acc_attach_async): Update call to gomp_attach_pointer. > (goacc_enter_data_internal): Likewise. > * target.c (gomp_map_vars_existing): Update assert condition to > include GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION. > (gomp_map_pointer): Add 'bool allow_zero_length_array_sections' > parameter, add support for mapping a pointer with NULL target. > (gomp_attach_pointer): Add 'bool allow_zero_length_array_sections' > parameter, add support for attaching a pointer with NULL target. > (gomp_map_vars_internal): Update calls to gomp_map_pointer and > gomp_attach_pointer, add handling for > GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION, and > GOMP_MAP_POINTER_TO_ZERO_LENGTH_ARRAY_SECTION cases. > * testsuite/libgomp.c++/target-23.C: New testcase. > * testsuite/libgomp.c++/target-lambda-1.C: New testcase. > * testsuite/libgomp.c++/target-lambda-2.C: New testcase. > * testsuite/libgomp.c++/target-this-1.C: New testcase. > * testsuite/libgomp.c++/target-this-2.C: New testcase. > * testsuite/libgomp.c++/target-this-3.C: New testcase. > * testsuite/libgomp.c++/target-this-4.C: New testcase. > * testsuite/libgomp.c++/target-this-5.C: New testcase. > +/* Used to walk OpenMP target directive body. */ > + > +struct omp_target_walk_data > +{ > + tree current_object; > + bool this_expr_accessed; > + > + hash_map ptr_members_accessed; > + hash_set lambda_objects_accessed; > + > + tree current_closure; > + hash_set closure_vars_accessed; > + > + hash_set local_decls; Can you please add short comments above the members describing what they are for? > +}; > + > +static tree > +finish_omp_target_clauses_r (tree *tp, int *walk_subtrees, void *ptr) And add function comments above here. > + > +void > +finish_omp_target_clauses (location_t loc, tree body, tree *clauses_ptr) And here. > + > +tree > +finish_omp_target (location_t loc, tree clauses, tree body, bool combined_p) And here? > + if (allow_zero_length_array_sections) > + { > + /* When allowing attachment to zero-length array sections, we > + allow attaching to NULL pointers when the target region is not > + mapped. */ > + data = 0; > + } No {}s around single statement if body. Otherwise LGTM. Jakub