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 28957383E690 for ; Fri, 27 May 2022 15:20:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 28957383E690 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-609-jStBWoKDP5m6DxmMHKT0Uw-1; Fri, 27 May 2022 11:20:10 -0400 X-MC-Unique: jStBWoKDP5m6DxmMHKT0Uw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 90291811E75; Fri, 27 May 2022 15:20:10 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.33.36.77]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 54C78170E8; Fri, 27 May 2022 15:20:10 +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 24RFK7Ru1984541 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 27 May 2022 17:20:07 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24RFK6S11984540; Fri, 27 May 2022 17:20:06 +0200 Date: Fri, 27 May 2022 17:20:06 +0200 From: Jakub Jelinek To: Tobias Burnus Cc: gcc-patches@gcc.gnu.org, fortran Subject: Re: [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target) Message-ID: Reply-To: Jakub Jelinek References: <651ae4c2-9440-b8df-e8af-b14491d45937@codesourcery.com> MIME-Version: 1.0 In-Reply-To: <651ae4c2-9440-b8df-e8af-b14491d45937@codesourcery.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.9 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=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: 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: Fri, 27 May 2022 15:20:15 -0000 On Fri, May 27, 2022 at 04:52:17PM +0200, Tobias Burnus wrote: > This patch adds the Fortran support to the just-committed C/C++ support for the 'enter' clause. > > The 'TO'/'ENTER' usage is first stored in a linked list – and > then as attribute to the symbol. I am not sure how to handle it best. > I did went for adding an ENTER_LIST but kept only using the single attribute. > > Result: In one diagnostic, I use 'TO or ENTER clause' in the other, > I can properly use 'TO' or 'ENTER' clause. > > This could be kept as is, but to save memory it would be possible > to drop the ENTER_LIST – or, to improve diagnostic, we could try harder > (e.g. by re-walking the list or by another gfc_attribute). Preferences? > If not, I would go for the attached middle way. > > Tobias > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 > OpenMP/Fortran: Add support for enter clause on declare target > > Fortran version to C/C++ commit r13-797-g0ccba4ed8571c18c7015413441e971 > > gcc/fortran/ChangeLog: > > * dump-parse-tree.cc (show_omp_clauses): Handle OMP_LIST_ENTER. > * gfortran.h: Add OMP_LIST_ENTER. > * openmp.cc (enum omp_mask2, OMP_DECLARE_TARGET_CLAUSES): Add > OMP_CLAUSE_ENTER. > (gfc_match_omp_clauses, gfc_match_omp_declare_target, > resolve_omp_clauses): Handle 'enter' clause. > > libgomp/ChangeLog: > > * libgomp.texi (OpenMP 5.2): Mark 'enter' clause as supported. > * testsuite/libgomp.fortran/declare-target-1.f90: Extend to test > explicit 'to' and 'enter' clause. > * testsuite/libgomp.fortran/declare-target-2.f90: Update accordingly. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/gomp/declare-target-2.f90: Add 'enter' clause test. > * gfortran.dg/gomp/declare-target-4.f90: Likewise. Mostly good, but see below. > - for (list = OMP_LIST_TO; list != OMP_LIST_NUM; > - list = (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM)) > + for (list = OMP_LIST_ENTER; list != OMP_LIST_NUM; > + list = (list == OMP_LIST_ENTER > + ? OMP_LIST_TO > + : (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM))) > for (n = c->lists[list]; n; n = n->next) > if (n->sym) > { > @@ -4564,14 +4584,14 @@ gfc_match_omp_declare_target (void) > && n->sym->attr.omp_declare_target_link > && list != OMP_LIST_LINK) > gfc_error_now ("OMP DECLARE TARGET variable at %L previously " > - "mentioned in LINK clause and later in TO clause", > - &n->where); > + "mentioned in LINK clause and later in %s clause", > + &n->where, list == OMP_LIST_TO ? "TO" : "ENTER"); > else if (n->sym->attr.omp_declare_target > && !n->sym->attr.omp_declare_target_link > && list == OMP_LIST_LINK) > gfc_error_now ("OMP DECLARE TARGET variable at %L previously " > - "mentioned in TO clause and later in LINK clause", > - &n->where); > + "mentioned in TO or ENTER clause and later in " > + "LINK clause", &n->where); The wording of the messages "previously mentioned in FOO and later in BAR clause" makes not much sense to me, because we in the Fortran FE don't remember which clause was specified earlier and which one was later. The loop is walking first all enter clauses, then all to clauses, then all link clauses. Now, if we change the wording so that it complains that a variable is "mentioned in both FOO and BAR clauses", we could avoid the TO or ENTER stuff even without some O(n^2) complexity or extra bit somewhere simply by walking the clauses in the order of TO, LINK, ENTER (or ENTER, LINK, TO) clauses, wer could be exact. Though, further thinking about it, this isn't just about the case where it is on the same declare target, but also on multiple and in that case previous/later makes sense. As we don't remember if it was previously TO or ENTER, I'd just suggest: 1) simplify the 2 for cycles, with 3 lists it is too unreadable, so use something like: static const int to_enter_link_lists[] = { OMP_LIST_TO, OMP_LIST_ENTER, OMP_LIST_LINK }; for (int listn = 0; listn < ARRAY_SIZE (to_enter_link_lists) && (list = to_enter_link_lists[listn], true); ++listn) 2) move the else if (n->sym->mark) gfc_error_now ("Variable at %L mentioned multiple times in " "clauses of the same OMP DECLARE TARGET directive", &n->where); diagnostics earlier, above else if (n->sym->attr.omp_declare_target && n->sym->attr.omp_declare_target_link && list != OMP_LIST_LINK) and adjust testsuite if needed Ok with those changes. On the C/C++ FE side, I'll need to change % to % or %. Jakub