From patchwork Wed Mar 2 17:22:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kwok Cheung Yeung X-Patchwork-Id: 51507 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6BC9D3857C7A for ; Wed, 2 Mar 2022 17:22:55 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id 4C6C03858D37; Wed, 2 Mar 2022 17:22:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4C6C03858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.90,149,1643702400"; d="scan'208,223";a="72648289" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa2.mentor.iphmx.com with ESMTP; 02 Mar 2022 09:22:29 -0800 IronPort-SDR: /UQTxlx7pfU8IX46d7DIw4POsXL3QtrQXP/ZveDkdvcdJYJ3ArBDFwTUhAs8a15BqBUKfcWvND ObxUaz9gMRmmXxK58TcmkeEc6CoCfbezlwlw16kbDglnBtnQnUw1ZlutcB6+EyznYAuyLWALwJ jg2DPSuieGGZK9+Ho7v0BLV3xuc/Wwv5UfOm2hvZxB6JlbpAkMXmhaIwh7h07jw1ySVxS9XVQL n95nl3mEAlcNz3w2Dzq6XoxfGoETsCcLrDsrmuLp55z51mkx2unNOS/fXhFA08Gu7NVQbbFneZ N7Y= Message-ID: <262035c6-7039-84c5-0b61-b9ee98f24b23@codesourcery.com> Date: Wed, 2 Mar 2022 17:22:21 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 From: Kwok Cheung Yeung Subject: [PATCH][v2] openmp, fortran: Check that the type of an event handle in a detach clause is suitable [PR104131] To: Mikael Morin , Tobias Burnus , Jakub Jelinek References: <383792f1-a5c7-af10-c603-2f1332336648@codesourcery.com> <3c2d161f-481a-b223-a855-cadee01a5679@orange.fr> <8981ebb8-7b8e-19d7-a3b3-ef5fce239c53@orange.fr> <907e1f89-4534-75e4-fdb8-a11da280e3d1@orange.fr> <65456330-dc81-0647-3bdb-6d409554136d@codesourcery.com> <14be1163-51e6-19bd-0032-c18a53ffe254@codesourcery.com> <6f75ce81-648d-a2e5-b9f4-b9aaa4a19775@orange.fr> In-Reply-To: <6f75ce81-648d-a2e5-b9f4-b9aaa4a19775@orange.fr> X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-13.mgc.mentorg.com (139.181.222.13) To SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: , Cc: gscfq@t-online.de, gcc-patches , fortran Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hello I have updated the patch to catch array elements and structure components as additional checks, in addition to checking that the variable is a scalar. The check has been moved to the end of resolve_omp_clauses as it is more appropriate there. This gets rid of the additional 'Unexpected !$OMP END TASK statement' error, since the type error is now caught after the matching phase. Coarrays (with the testcases in pr104131-2.f90) can be dealt with in a separate patch. Is this part okay for trunk? Thanks Kwok On 01/03/2022 3:37 pm, Mikael Morin wrote: > So, if I try to sum up what has been gathered in this thread: > >  - pr104131.f90 is invalid, as x is not scalar. >    Checks are better done in resolve_omp_clauses after a call >    to gfc_resolve_expr. >    Checking expr->sym->attr.dimension seems to cover more cases than >    expr->rank > 0. > >  - pr104131-2.f90 is valid and should be accepted. > >  - Some other cases should be rejected, including x[1] (coindexed >    variable), x(1) (array element), x%comp (structure component). > > Is that correct? Anything else? > > Regarding the expr->rank vs expr->sym->attr.dimension controversy, my > take is that it should stick to the error message.  Use expr->rank is > the error is about scalar vs array, use expr->sym->attr.dimension if > it’s about subobject-ness of an array variable. > > Coming back to the PR, the ICE backtraces for pr104131.f90 and > pr104131-2.f90 are different and should probably be treated separatedly. > I don’t know how difficult the bullet 2 above would be, but bullet 1 and > 3 seem quite doable. From 3ed6eb1e38ad2a25c6eca18f9ff4d05d3f227db3 Mon Sep 17 00:00:00 2001 From: Kwok Cheung Yeung Date: Wed, 2 Mar 2022 17:09:45 +0000 Subject: [PATCH] openmp, fortran: Check that the type of an event handle in a detach clause is suitable [PR104131] This rejects variables that are array types, array elements or derived type members when used as the event handle inside a detach clause (in accordance with the OpenMP specification). This would previously lead to an ICE. 2022-03-02 Kwok Cheung Yeung gcc/fortran/ PR fortran/104131 * openmp.cc (gfc_match_omp_detach): Move check for type of event handle to... (resolve_omp_clauses) ...here. Also check that the event handle is not an array, or an array access or structure element access. gcc/testsuite/ PR fortran/104131 * gfortran.dg/gomp/pr104131.f90: New. * gfortran.dg/gomp/task-detach-1.f90: Update expected error message. --- gcc/fortran/openmp.cc | 34 +++++++++++++------ gcc/testsuite/gfortran.dg/gomp/pr104131.f90 | 26 ++++++++++++++ .../gfortran.dg/gomp/task-detach-1.f90 | 4 +-- 3 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr104131.f90 diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index 19142c4d8d0..16cd03a3d67 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -531,14 +531,6 @@ gfc_match_omp_detach (gfc_expr **expr) if (gfc_match_variable (expr, 0) != MATCH_YES) goto syntax_error; - if ((*expr)->ts.type != BT_INTEGER || (*expr)->ts.kind != gfc_c_intptr_kind) - { - gfc_error ("%qs at %L should be of type " - "integer(kind=omp_event_handle_kind)", - (*expr)->symtree->n.sym->name, &(*expr)->where); - return MATCH_ERROR; - } - if (gfc_match_char (')') != MATCH_YES) goto syntax_error; @@ -7581,9 +7573,29 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, gfc_error ("%s must contain at least one MAP clause at %L", p, &code->loc); } - if (!openacc && omp_clauses->mergeable && omp_clauses->detach) - gfc_error ("% clause at %L must not be used together with " - "% clause", &omp_clauses->detach->where); + + if (!openacc && omp_clauses->detach) + { + if (!gfc_resolve_expr (omp_clauses->detach) + || omp_clauses->detach->ts.type != BT_INTEGER + || omp_clauses->detach->ts.kind != gfc_c_intptr_kind + || omp_clauses->detach->rank != 0) + gfc_error ("%qs at %L should be a scalar of type " + "integer(kind=omp_event_handle_kind)", + omp_clauses->detach->symtree->n.sym->name, + &omp_clauses->detach->where); + else if (omp_clauses->detach->symtree->n.sym->attr.dimension > 0) + gfc_error ("The event handle at %L must not be an array element", + &omp_clauses->detach->where); + else if (omp_clauses->detach->symtree->n.sym->ts.type == BT_DERIVED + || omp_clauses->detach->symtree->n.sym->ts.type == BT_CLASS) + gfc_error ("The event handle at %L must not be part of " + "a derived type or class", &omp_clauses->detach->where); + + if (omp_clauses->mergeable) + gfc_error ("% clause at %L must not be used together with " + "% clause", &omp_clauses->detach->where); + } } diff --git a/gcc/testsuite/gfortran.dg/gomp/pr104131.f90 b/gcc/testsuite/gfortran.dg/gomp/pr104131.f90 new file mode 100644 index 00000000000..472d19dd753 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/gomp/pr104131.f90 @@ -0,0 +1,26 @@ +! { dg-do compile } +! { dg-options "-fopenmp" } + +program p + use iso_c_binding, only: c_intptr_t + implicit none + integer, parameter :: omp_event_handle_kind = c_intptr_t + + type dt + integer(omp_event_handle_kind) :: f + end type + integer(omp_event_handle_kind) :: x(1) + type(dt) :: y + + !$omp task detach(x) ! { dg-error "'x' at \\\(1\\\) should be a scalar of type integer\\\(kind=omp_event_handle_kind\\\)" } + !$omp end task + + !$omp task detach(x(1)) ! { dg-error "The event handle at \\\(1\\\) must not be an array element" } + !$omp end task + + !$omp task detach(y) ! { dg-error "'y' at \\\(1\\\) should be a scalar of type integer\\\(kind=omp_event_handle_kind\\\)" } + !$omp end task + + !$omp task detach(y%f) ! { dg-error "The event handle at \\\(1\\\) must not be part of a derived type or class" } + !$omp end task +end program diff --git a/gcc/testsuite/gfortran.dg/gomp/task-detach-1.f90 b/gcc/testsuite/gfortran.dg/gomp/task-detach-1.f90 index 020be13a8b6..2e77aea0549 100644 --- a/gcc/testsuite/gfortran.dg/gomp/task-detach-1.f90 +++ b/gcc/testsuite/gfortran.dg/gomp/task-detach-1.f90 @@ -18,8 +18,8 @@ program task_detach_1 !$omp task detach(x) mergeable ! { dg-error "'DETACH' clause at \\\(1\\\) must not be used together with 'MERGEABLE' clause" } !$omp end task - !$omp task detach(z) ! { dg-error "'z' at \\\(1\\\) should be of type integer\\\(kind=omp_event_handle_kind\\\)" } - !$omp end task ! { dg-error "Unexpected !\\\$OMP END TASK statement at \\\(1\\\)" } + !$omp task detach(z) ! { dg-error "'z' at \\\(1\\\) should be a scalar of type integer\\\(kind=omp_event_handle_kind\\\)" } + !$omp end task !$omp task detach (x) firstprivate (x) ! { dg-error "DETACH event handle 'x' in FIRSTPRIVATE clause at \\\(1\\\)" } !$omp end task