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 D7E0F385AE75 for ; Mon, 13 Jun 2022 14:05:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D7E0F385AE75 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-641-A5YsxT9QMRuDZ0SMd3ZTCQ-1; Mon, 13 Jun 2022 10:05:05 -0400 X-MC-Unique: A5YsxT9QMRuDZ0SMd3ZTCQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CEC3C29AA3B9; Mon, 13 Jun 2022 14:05:03 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 872412026D2D; Mon, 13 Jun 2022 14:05:03 +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 25DE4xB63471893 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 13 Jun 2022 16:05:00 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 25DE4xQV3471892; Mon, 13 Jun 2022 16:04:59 +0200 Date: Mon, 13 Jun 2022 16:04:58 +0200 From: Jakub Jelinek To: Chung-Lin Tang Cc: Tobias Burnus , gcc-patches , Hafiz Abid Qadeer , Andrew Stubbs Subject: Re: [PATCH, OpenMP, v4] Implement uses_allocators clause for target regions Message-ID: Reply-To: Jakub Jelinek References: <46d77e14-080c-db6c-4032-e12899c5d059@codesourcery.com> <9c0945fa-1054-095e-86ae-a9d8dd1ab625@codesourcery.com> <075e0483-6d7b-3bbc-a718-b0872a870f85@codesourcery.com> MIME-Version: 1.0 In-Reply-To: <075e0483-6d7b-3bbc-a718-b0872a870f85@codesourcery.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 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.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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 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: Mon, 13 Jun 2022 14:05:12 -0000 On Mon, Jun 13, 2022 at 09:29:34PM +0800, Chung-Lin Tang wrote: > > I was hoping you'd drop all this. > > Seehttps://gcc.gnu.org/r13-1002 > > for implementation (both C and C++ FE) of something very similar, > > the only difference there is that in the case of linear clause, it is > > looking for > > val > > ref > > uval > > step ( whatever ) > > followed by , or ) > > (anod ref and uval not in C FE), > > while you are looking for > > memspace ( whatever ) > > traits ( whatever ) > > followed by : or by , (in case of , repeat). > > But in both cases you can actually use the same parser APIs > > for raw token pre-parsing to just compute if it is the modifier > > syntax or not, set bool has_modifiers based on that (when you > > come over probably valid syntax followed by CPP_COLON). > > The linear clause doesn't have the legacy 'allocator1(t1), allocator2(t2), ...' requirement, > and c_parser_omp_variable_list doesn't seem to support this pattern. True, but I don't see why it is relevant. > Also, the way c_parser_omp_clause_linear is implemented doesn't support the requirement > you mentioned earlier of allowing the use of "memspace", "traits" as the allocator name when > it's actually not a modifier. No, it is exactly the same thing. As you can see e.g. in the testsuite coverage I've committed in the linear patch, in the linear clause case after : either one uses a modifier syntax, or everything after the : is the step expression (assignment-expression in C/C++). There is parsing ambiguity and the spec says that it is resolved to the modifier syntax in that case. Say if I have: constexpr int step (int x) { return x; } and use linear (a, b, c : step (1)) then it is the modifier syntax (incompatible change from 5.1), while linear (a, b, c : step (1) + 0) linear (a, b, c : (step (1))) linear (a, b, c : 0 + step (1)) etc. have step expressions. The spec wording is such that it doesn't even have to be discovered by strictly tentative parsing (like in GCC the C++ and Fortran FEs do but C FE doesn't), modifier syntax wins if one sees the modifiers with balanced () after it if it is complex, followed by , or ) as a terminator of a single modifier. The first raw token walk in the patch is just a fast "tentative" parsing check whether it is modifier syntax or not, if it is, then we just parse it as modifiers, otherwise parse it as expression. The uses_allocator is similar, although in that case it actually isn't a parsing ambiguity, just that we need arbitrary number of tokens look-ahead to decide. We need to check if the tokens after uses_allocators ( look like one or more complex modifiers (with the 2 modifier names and just a balanced ()s after them - in the uses_allocators case currently all supported modifiers are complex), if yes and it is followed by : token, it is the modifiers syntax, otherwise it is not. So say: #include void foo (void) { omp_alloc_handle_t traits, x; const omp_alloctrait_t my_traits[] = { ... }; #pragma omp target uses_allocators (traits (my_traits) : x) ; #pragma omp target uses_allocators (traits (my_traits), x (my_traits)) ; #pragma omp target uses_allocators (traits (my_traits), omp_high_mem_bw_mem_alloc) ; #pragma omp target uses_allocators (traits (my_traits)) ; } All the clauses above start with the same tokens, but depending on what follows we need to either parse it as the modifier syntax (the first directive) or as the compatibility syntax (the rest). Which is why I was suggesting to do this quick raw token parsing check if it is the modifier syntax or not. If it is, parse modifiers and : and then you know to expect a single allocator without ()s after it (e.g. you could use c_parser_omp_variable_list etc. and just verify afterwards the list has a single entry in it). If it is not, it might still be old or new syntax, the latter only if the list contains a single var and not followed by ()s and sure, you need to write a parsing loop for that. It isn't the same thing as the modifier loop though, modifiers start with a keyword, the allocator list with a identifier for the variable. For uses_allocators, we can then even simplify when we almost finish OpenMP 6.0 support, if the old style syntax uses_allocators are gone by then, we could decide if it is a modifier syntax or not just by looking at first 2 tokens, whether the first token is allowed modifier keyword and whether it is followed by (, then we could commit to modifier parsing right away. And the loop to do the ()s parsing can go too... Jakub