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 0BE5338485AD for ; Tue, 24 May 2022 14:48:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0BE5338485AD 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-640-g3uOTNFcOU-3qapossp7Rg-1; Tue, 24 May 2022 10:48:21 -0400 X-MC-Unique: g3uOTNFcOU-3qapossp7Rg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5412F811E75; Tue, 24 May 2022 14:48:21 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0F97740CFD05; Tue, 24 May 2022 14:48:20 +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 24OEmCRe2107220 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 24 May 2022 16:48:13 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24OEmCLf2107219; Tue, 24 May 2022 16:48:12 +0200 Date: Tue, 24 May 2022 16:48:12 +0200 From: Jakub Jelinek To: Julian Brown Cc: gcc-patches@gcc.gnu.org, Thomas Schwinge , Tobias Burnus , Fortran List Subject: Re: [PATCH v2 09/11] OpenMP 5.0 "declare mapper" support for C++ Message-ID: Reply-To: Jakub Jelinek References: <446929ccca41031c43ca38c6f5ce2141422a2a0d.1647619145.git.julian@codesourcery.com> MIME-Version: 1.0 In-Reply-To: <446929ccca41031c43ca38c6f5ce2141422a2a0d.1647619145.git.julian@codesourcery.com> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 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 14:48:26 -0000 On Fri, Mar 18, 2022 at 09:26:50AM -0700, Julian Brown wrote: > This patch implements OpenMP 5.0 "declare mapper" support for C++ -- > except for arrays of structs with mappers, which are TBD. I've taken cues > from the existing "declare reduction" support where appropriate, though > obviously the details of implementation differ somewhat (in particular, > "declare mappers" must survive longer, until gimplification time). > > Both named and unnamed (default) mappers are supported, and both > explicitly-mapped structures and implicitly-mapped struct-typed variables > used within an offload region are supported. For the latter, we need a > way to communicate to the middle end which mapper (visible, in-scope) is > the right one to use -- for that, we scan the target body in the front > end for uses of structure (etc.) types, and create artificial "mapper > binding" clauses to associate types with visible mappers. (It doesn't > matter too much if we create these mapper bindings a bit over-eagerly, > since they will only be used if needed later during gimplification.) > > Another difficulty concerns the order in which an OpenMP offload region > body's clauses are processed relative to its body: in order to add > clauses for instantiated mappers, we need to have processed the body > already in order to find which variables have been used, but at present > the region's clauses are processed strictly before the body. So, this > patch adds a second clause-processing step after gimplification of the > body in order to add clauses resulting from instantiated mappers. > > This version of the patch improves detection of explicitly-mapped struct > accesses which inhibit implicitly-triggered user-defined mappers for a > target region. Will start with a general comment, from looking at the dumps it seems handling the mappers in the FE right away for explicit mapping clauses and attaching mapper binding clauses for types that are (or could conservatively be, including from the recursive mappers themselves) be used in the target body and letting gimplification find those var in detail and use mapper binding clauses to actually expand it looks like the right approach to me. As I raised in an earlier patch, a big question is if we should do map clause sorting on gimplify_scan_omp_clauses or gimplify_adjust_omp_clauses or both... The conservative discovery of what types we might need to create mapper binding clauses for should be probably done only if !processing_template_decl. One question is though if DECL_OMP_DECLARE_MAPPER_P should be a magic FUNCTION_DECL or a magic TREE_STATIC VAR_DECL or say CONST_DECLs. The reason for the choice of FUNCTION_DECLs for UDRs is that they actually contain code, but for UDMs we don't need any code, all we need is some decl to which we can somehow attach list of clauses and a placeholder decl used in them. Perhaps a magic VAR_DECL or CONST_DECL would be cheaper than a FUNCTION_DECL... > @@ -32193,6 +32197,16 @@ cp_parser_late_parsing_for_member (cp_parser* parser, tree member_function) > finish_function (/*inline_p=*/true); > cp_check_omp_declare_reduction (member_function); > } > + else if (DECL_OMP_DECLARE_MAPPER_P (member_function)) > + { > + parser->lexer->in_pragma = true; > + cp_parser_omp_declare_mapper_maplist (member_function, parser); > + finish_function (/*inline_p=*/true); > + cp_check_omp_declare_mapper (member_function); > + /* If this is a template class, this forces the body of the mapper > + to be instantiated. */ > + DECL_PRESERVE_P (member_function) = 1; UDRs don't do this. Why aren't the clauses instantiated when we actually need such a template? > @@ -39509,11 +39522,27 @@ cp_parser_omp_clause_map (cp_parser *parser, tree list) > > if (cp_lexer_peek_nth_token (parser->lexer, pos + 1)->type == CPP_COMMA) > pos++; > + else if ((cp_lexer_peek_nth_token (parser->lexer, pos + 1)->type > + == CPP_OPEN_PAREN) > + && ((cp_lexer_peek_nth_token (parser->lexer, pos + 2)->type > + == CPP_NAME) > + || ((cp_lexer_peek_nth_token (parser->lexer, pos + 2)->type > + == CPP_KEYWORD) > + && (cp_lexer_peek_nth_token (parser->lexer, > + pos + 2)->keyword > + == RID_DEFAULT))) > + && (cp_lexer_peek_nth_token (parser->lexer, pos + 3)->type > + == CPP_CLOSE_PAREN) > + && (cp_lexer_peek_nth_token (parser->lexer, pos + 4)->type > + == CPP_COMMA)) In this loop we don't need to be exact, all we want is find out if the mapper-mdifier candidates are followed by : or not, the actual parsing is done only later. So, can't we just use for CPP_OPEN_PAREN cp_parser_skip_balanced_tokens to move over all the modifier's arguments? Jakub