From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by sourceware.org (Postfix) with ESMTP id 43B6D3858D35 for ; Wed, 29 Jul 2020 08:23:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 43B6D3858D35 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-237-VxmRsVNAPBqHMZq_xY1h3g-1; Wed, 29 Jul 2020 04:23:07 -0400 X-MC-Unique: VxmRsVNAPBqHMZq_xY1h3g-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 0B71B8CAD92; Wed, 29 Jul 2020 08:23:06 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-218.ams2.redhat.com [10.36.112.218]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 91DCF5D9F7; Wed, 29 Jul 2020 08:23:05 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id 06T8N249002256; Wed, 29 Jul 2020 10:23:02 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id 06T8N0xq002255; Wed, 29 Jul 2020 10:23:00 +0200 Date: Wed, 29 Jul 2020 10:23:00 +0200 From: Jakub Jelinek To: Tobias Burnus Cc: gcc-patches , fortran Subject: Re: [Patch] OpenMP: Add 'omp requires' to Fortran (mostly parsing) Message-ID: <20200729082300.GK2363@tucnak> Reply-To: Jakub Jelinek References: <03601efc-b586-d58e-5950-3cfb40081d1a@codesourcery.com> <6b1143bd-4ca5-a392-966b-d6ca1ea7b198@codesourcery.com> <4aaad635-d902-f74f-0ca1-cea906edd5a9@codesourcery.com> MIME-Version: 1.0 In-Reply-To: <4aaad635-d902-f74f-0ca1-cea906edd5a9@codesourcery.com> User-Agent: Mutt/1.11.3 (2019-02-01) 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=-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_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 29 Jul 2020 08:23:11 -0000 On Wed, Jul 29, 2020 at 10:10:35AM +0200, Tobias Burnus wrote: > + case AB_OMP_REQ_REVERSE_OFFLOAD: > + gfc_omp_requires_add_clause (OMP_REQ_REVERSE_OFFLOAD, > + "reverse_offload", > + &gfc_current_locus, > + module_name); Even visually something is wrong with the indentation here and in a few others, where I'd expect all the arguments to be in the same column (even with the > + before it, but they are not. E.g. above I think the gfc_omp_... is indented by 3 instead of 2 columns from case, and "rev... and &gfc... match that, but module_name is probably correct. > + break; > + case AB_OMP_REQ_UNIFIED_ADDRESS: > + gfc_omp_requires_add_clause (OMP_REQ_UNIFIED_ADDRESS, > + "unified_address", > + &gfc_current_locus, And e.g. here the &gfc... is off. > --- a/gcc/fortran/openmp.c > +++ b/gcc/fortran/openmp.c > @@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see > #include "diagnostic.h" > #include "gomp-constants.h" > > + Unnecessary. > /* Match an end of OpenMP directive. End of OpenMP directive is optional > whitespace, followed by '\n' or comment '!'. */ > > @@ -3745,6 +3970,26 @@ gfc_match_omp_oacc_atomic (bool omp_p) > new_st.op = (omp_p ? EXEC_OMP_ATOMIC : EXEC_OACC_ATOMIC); > if (seq_cst) > op = (gfc_omp_atomic_op) (op | GFC_OMP_ATOMIC_SEQ_CST); > + else I wonder if this shouldn't be else if (omp_p), I'd think OpenACC atomics shouldn't be affected by OpenMP requires directive. > for (gfc_current_ns = gfc_global_ns_list; gfc_current_ns; > gfc_current_ns = gfc_current_ns->sibling) > - gfc_check_externals (gfc_current_ns); > + gfc_check_omp_requires (gfc_current_ns, omp_requires); Again indentation looks weird. Otherwise LGTM. Jakub