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 8BDAF385AE68 for ; Thu, 9 Jun 2022 14:46:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8BDAF385AE68 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-290-dyT3SmmcMhSZ6wCT6zjEdA-1; Thu, 09 Jun 2022 10:45:56 -0400 X-MC-Unique: dyT3SmmcMhSZ6wCT6zjEdA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 684FF804190; Thu, 9 Jun 2022 14:45:56 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 20ADB40885A1; Thu, 9 Jun 2022 14:45:55 +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 259EjrTJ1627784 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 9 Jun 2022 16:45:53 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 259Ejrrk1627783; Thu, 9 Jun 2022 16:45:53 +0200 Date: Thu, 9 Jun 2022 16:45:53 +0200 From: Jakub Jelinek To: Julian Brown Cc: fortran@gcc.gnu.org, Tobias Burnus , gcc-patches@gcc.gnu.org, Thomas Schwinge Subject: Re: [PATCH v2 01/11] OpenMP 5.0: Clause ordering for OpenMP 5.0 (topological sorting by base pointer) Message-ID: Reply-To: Jakub Jelinek References: <20220608160039.596be361@squid.athome> MIME-Version: 1.0 In-Reply-To: <20220608160039.596be361@squid.athome> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 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=-3.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, 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: Thu, 09 Jun 2022 14:46:04 -0000 On Wed, Jun 08, 2022 at 04:00:39PM +0100, Julian Brown wrote: > > I think big question is if we do want to do this map clause reordering > > before processing the omp target etc. clauses, or after (during > > gimplify_adjust_omp_clauses, when clauses from the implicit mappings > > are added too and especially with the declare mapper expansions), > > or both before and after. > > The existing code constrains us a bit here, unless we want to > completely rewrite it! > > We can only do sorting on clauses before gimplification, otherwise the > "structural" matching of the parsed syntax of base pointers inside other > clauses on the directive, etc. will certainly fail. > > (Semi-relatedly, I asked this on the omp-lang mailing list: > > "When we have mappings that represent base pointers, and other > mappings that use those base pointers, the former must be ordered to > take place before the latter -- but should we determine that relation > purely syntactically? How about if we write e.g. "p->" on one vs. > "(*p)." on the other?" > > but no reply...) > > So, this is fine for sorting explicit mapping clauses. When planning > the approach I've used for "declare mapper" support, I wrote this (in > an internal email): > > "At the moment, gimplifying OMP workshare regions proceeds in three > phases: > > 1. Clauses are processed (gimplify_scan_omp_clauses), creating > records of mapped variables in a splay tree, with associated flags. > > 2. The body of the workshare region is processed (gimplified), > augmenting the same splay tree with information about variables > which are used implicitly (and maybe also modifying the "explicit" > mappings from the first step). > > 3. The clauses are modified based on the results of the second stage > (gimplify_adjust_omp_clauses). E.g. clauses are removed that refer > to variables that aren't actually used in the region, or new > clauses created for implicitly-referenced variables without mapping > clauses on the construct. > > The problem with this with regards to mappers is that the "expanded" > mappers should undergo some of the processing we currently perform > during phase 1 (struct sibling list handling, and so on), but we don't > know which variables are implicitly referenced until phase 2. > > [description of a plan that didn't work removed] > > So the new plan is to do: > > phase 1 (scan original clauses) > phase 2 (scan workshare body) > phase 1 (use variables from "2" to instantiate mappers, and process > new clauses only. Prepend new list to original clauses) > phase 3 (as before) > > I was concerned that this would upset the sorting code -- but I think > actually, as long as implicitly-created clauses are inserted at the > front of the clause list, there can't be a case where a pointer base is > mapped after a use of that base. If that assumption turns out to be > wrong, then things might get a little more complicated." > > ...and so far, the plan seems to be working out. The assumption, to > state it in other words, is that an implicitly-added map clause *cannot* > have a dependency on an explicit map clause, in terms of relying on a > base pointer in that explicit clause, by construction. I don't think there is any need to add extra phases, but we can move some code from gimplify_scan_omp_clauses to gimplify_adjust_omp_clauses. What must be done in gimplify_scan_omp_clauses is stuff that will or could affect the gimplification of the region's body, in that phase 2 we want to know say that some variable was privatized explicitly or explicitly mapped or none of that, so we can based on that decide if we should note implicit data sharing or implicit mapping etc. But e.g. the sorting of the OMP_CLAUSE_MAP clauses is something that can IMHO be deferred until we have all those clauses, probably it is done in gimplify_scan_omp_clauses right now was just that the sorting at least initially was only needed for struct mapping (map (tofrom: a.b, a.c, a.d.e, a.d.f)) and that could appear only explicitly, not implicitly, implicit mapping would only map the whole var. But declare mapper changes this substantially, declare mapper can add similar mappings even from the implicit maps. So, I think we should keep in phase 1 for OMP_CLAUSE_MAP only the stuff that perhaps gimplifies some expressions used in those and puts records about them into splay trees and sorting and ideally some kind of merging of adjacent mappings can be done only when we have even the implicit mappings all collected (so that would be after splay_tree_foreach (ctx->variables, gimplify_adjust_omp_clauses_1, &data); finishes). Jakub