From patchwork Thu Nov 23 14:21:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 1867795 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4SbgKm09hzz1yRg for ; Fri, 24 Nov 2023 01:22:03 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9AB513857C62 for ; Thu, 23 Nov 2023 14:22:00 +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 8C2E23858C53; Thu, 23 Nov 2023 14:21:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8C2E23858C53 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8C2E23858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=68.232.141.98 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700749309; cv=none; b=lRKLfikDliM1cqRXN01ZRrdV2AuMZRGZ8Yda07G9+pEF99B4lmLqNo2hOT5I/pFOiT+F+WD3n+MtLQVOkrEZk+fTlshcmeCA1XioZZSPfxlKAQyhGOX9d3nayV81EHb7IWcQPrY5AW55FOsQYZECj5ccRSJaVBZUMy/Psy1adDg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700749309; c=relaxed/simple; bh=9BcLhv2xdN/QhPi9SK+ilKAwPTLeifzX2YKBE2lSfLg=; h=Message-ID:Date:MIME-Version:To:From:Subject; b=Xkc5dHrVF1LeRe1SCEixLY2rycB9TQDUSHnblXGRDYtFU7n+ufX5CzNoZbFd5SPmxPJVH8ExgR15V8wVZbRIhN4kqGWEuT78MHKV03kd6dKH+TDil2rtxxSoOFsr5myOoPyzVuEJap3j+fG6YZojEF4HyiG/qCycYgZJfyl2i3s= ARC-Authentication-Results: i=1; server2.sourceware.org X-CSE-ConnectionGUID: D8SAHOUXQu+OHK4bzIA62A== X-CSE-MsgGUID: tK9Y3fgvRvi3odpCXfizRg== X-IronPort-AV: E=Sophos;i="6.04,221,1695715200"; d="diff'?scan'208";a="26349591" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa2.mentor.iphmx.com with ESMTP; 23 Nov 2023 06:21:45 -0800 IronPort-SDR: peUI9rR8yoHcB1jsnia71xA82ULtRhBXVHGJ3KCHQlzuXM2BCTLZ854EwPftZOkBIUGfKYhd6/ 5wmlH7WX9R+sHDbXsox271sAPVnVv0EwtVTOOjsta4+o1hmrxKdRKnhc9SXMJxCeSlyyiYNk0g nYwHrAxbe5RJRH2ViRfT0D0kbUOsa/4wSCivBPRn8yZQnrughK3lK6vJbwodARAR2DCel5l4V1 1SdxeOgEIgKVnYmZhoBO1c63++MxMP8u8nTB6QsV8Q2ZkkBVbCBjKwribv6EtmFHnodAzSIHHA 7UQ= Message-ID: <43e9e456-5114-468b-95a1-b22d8abea73c@codesourcery.com> Date: Thu, 23 Nov 2023 15:21:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: gcc-patches , fortran From: Tobias Burnus Subject: [Patch] OpenMP: Accept argument to depobj's destroy clause X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) To svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org I stumbled over this trivial omission which blocks some testcases. I am not sure whether I have solved the is-same-expr most elegantly, but I did loosely follow the duplicated-entry check for 'map'. As that's a restriction to the user, we don't have to catch all and I hope the code catches the most important violations, doesn't ICE and does not reject valid code. At least for all real-world code it should™ work, but I guess for lvalue expressions involving function calls it probably doesn't. Thoughts, comments? Tobias PS: GCC accepts an lvalue expression in C/C++ and only a identifier for a scalar variable in Fortran, i.e. neither array elements nor structure components. Which variant is right depends whether one reads OpenMP 5.1 (lvalue expr, scalar variable) or 5.2 (variable without permitting array sections or structure components) - whereas TR12 has the same but talks about locator list items in one restriction. For the OpenMP mess, see spec issue #3739. ----------------- 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: Accept argument to depobj's destroy clause Since OpenMP 5.2, the destroy clause takes an depend argument as argument; for the depobj directive, it the new argument is optional but, if present, it must be identical to the directive's argument. gcc/c/ChangeLog: * c-parser.cc (c_parser_omp_depobj): Accept optionally an argument to the destroy clause. gcc/cp/ChangeLog: * parser.cc (cp_parser_omp_depobj): Accept optionally an argument to the destroy clause. gcc/fortran/ChangeLog: * openmp.cc (gfc_match_omp_depobj): Accept optionally an argument to the destroy clause. libgomp/ChangeLog: * libgomp.texi (5.2 Impl. Status): An argument to the destroy clause is now supported. gcc/testsuite/ChangeLog: * c-c++-common/gomp/depobj-3.c: New test. * gfortran.dg/gomp/depobj-3.f90: New test. gcc/c/c-parser.cc | 57 ++++++++++++++++++++++++++- gcc/cp/parser.cc | 60 ++++++++++++++++++++++++++++- gcc/fortran/openmp.cc | 15 +++++++- gcc/testsuite/c-c++-common/gomp/depobj-3.c | 40 +++++++++++++++++++ gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 | 18 +++++++++ libgomp/libgomp.texi | 2 +- 6 files changed, 188 insertions(+), 4 deletions(-) diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 371dd29557b..378647c1a67 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -21605,6 +21605,9 @@ c_parser_omp_critical (location_t loc, c_parser *parser, bool *if_p) destroy update (dependence-type) + OpenMP 5.2 additionally: + destroy ( depobj ) + dependence-type: in out @@ -21663,7 +21666,59 @@ c_parser_omp_depobj (c_parser *parser) clause = error_mark_node; } else if (!strcmp ("destroy", p)) - kind = OMP_CLAUSE_DEPEND_LAST; + { + matching_parens c_parens; + kind = OMP_CLAUSE_DEPEND_LAST; + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN) + && c_parens.require_open (parser)) + { + tree destobj = c_parser_expr_no_commas (parser, NULL).value; + /* OpenMP requires that the two expressions are identical; catch + the most common mismatches. */ + if (!lvalue_p (destobj)) + error_at (EXPR_LOC_OR_LOC (destobj, c_loc), + "% expression is not lvalue expression"); + else if (depobj != error_mark_node) + { + tree t = depobj; + tree t2 = build_unary_op (EXPR_LOC_OR_LOC (destobj, c_loc), + ADDR_EXPR, destobj, false); + if (t2 != error_mark_node) + t2 = build_indirect_ref (EXPR_LOC_OR_LOC (t2, c_loc), + t2, RO_UNARY_STAR); + while (TREE_CODE (t) == COMPONENT_REF + || TREE_CODE (t) == ARRAY_REF) + { + t = TREE_OPERAND (t, 0); + if (TREE_CODE (t) == MEM_REF || INDIRECT_REF_P (t)) + { + t = TREE_OPERAND (t, 0); + STRIP_NOPS (t); + if (TREE_CODE (t) == POINTER_PLUS_EXPR) + t = TREE_OPERAND (t, 0); + } + } + while (TREE_CODE (t2) == COMPONENT_REF + || TREE_CODE (t2) == ARRAY_REF) + { + t2 = TREE_OPERAND (t2, 0); + if (TREE_CODE (t2) == MEM_REF || INDIRECT_REF_P (t2)) + { + t2 = TREE_OPERAND (t2, 0); + STRIP_NOPS (t2); + if (TREE_CODE (t2) == POINTER_PLUS_EXPR) + t2 = TREE_OPERAND (t2, 0); + } + } + if (DECL_UID (t) != DECL_UID (t2)) + error_at (EXPR_LOC_OR_LOC (destobj, c_loc), + "the % expression %qE must be the same " + "as the % argument %qE", + destobj, depobj); + } + c_parens.skip_until_found_close (parser); + } + } else if (!strcmp ("update", p)) { matching_parens c_parens; diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index f6d088bc73f..0fff66555bf 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -43173,6 +43173,9 @@ cp_parser_omp_critical (cp_parser *parser, cp_token *pragma_tok, bool *if_p) destroy update (dependence-type) + OpenMP 5.2 additionally: + destroy ( depobj ) + dependence-type: in out @@ -43219,7 +43222,62 @@ cp_parser_omp_depobj (cp_parser *parser, cp_token *pragma_tok) clause = error_mark_node; } else if (!strcmp ("destroy", p)) - kind = OMP_CLAUSE_DEPEND_LAST; + { + kind = OMP_CLAUSE_DEPEND_LAST; + matching_parens c_parens; + if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN) + && c_parens.require_open (parser)) + { + tree destobj = cp_parser_assignment_expression (parser); + /* OpenMP requires that the two expressions are identical; catch + the most common mismatches. */ + if (depobj != error_mark_node && destobj != error_mark_node) + { + tree t = depobj; + tree t2 = destobj; + while (TREE_CODE (t) == COMPONENT_REF + || TREE_CODE (t) == ARRAY_REF + || TREE_CODE (t) == VIEW_CONVERT_EXPR) + { + t = TREE_OPERAND (t, 0); + if (REFERENCE_REF_P (t)) + t = TREE_OPERAND (t, 0); + if (TREE_CODE (t) == MEM_REF || INDIRECT_REF_P (t)) + { + t = TREE_OPERAND (t, 0); + STRIP_NOPS (t); + if (TREE_CODE (t) == POINTER_PLUS_EXPR) + t = TREE_OPERAND (t, 0); + } + } + while (TREE_CODE (t2) == COMPONENT_REF + || TREE_CODE (t2) == ARRAY_REF + || TREE_CODE (t2) == VIEW_CONVERT_EXPR) + { + t2 = TREE_OPERAND (t2, 0); + if (REFERENCE_REF_P (t2)) + t2 = TREE_OPERAND (t2, 0); + if (TREE_CODE (t2) == MEM_REF || INDIRECT_REF_P (t2)) + { + t2 = TREE_OPERAND (t2, 0); + STRIP_NOPS (t2); + if (TREE_CODE (t2) == POINTER_PLUS_EXPR) + t2 = TREE_OPERAND (t2, 0); + } + } + if (t != t2) + error_at (EXPR_LOC_OR_LOC (destobj, c_loc), + "the % expression %qE must be the same " + "as the % argument %qE", + destobj, depobj); + } + if (!c_parens.require_close (parser)) + cp_parser_skip_to_closing_parenthesis (parser, + /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); + } + } else if (!strcmp ("update", p)) { matching_parens c_parens; diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index 2e2e23d567b..5f37f3a2586 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -4731,10 +4731,23 @@ gfc_match_omp_depobj (void) goto error; } } - else if (gfc_match ("destroy") == MATCH_YES) + else if (gfc_match ("destroy ") == MATCH_YES) { + gfc_expr *destroyobj = NULL; c = gfc_get_omp_clauses (); c->destroy = true; + + if (gfc_match (" ( %v ) ", &destroyobj) == MATCH_YES) + { + if (destroyobj->symtree != depobj->symtree) + { + gfc_error ("The same depend object must be used as DEPOBJ argument at %L" + " and as DESTROY argument at %L", &depobj->where, + &destroyobj->where); + return MATCH_ERROR; + } + gfc_free_expr (destroyobj); + } } else if (gfc_match_omp_clauses (&c, omp_mask (OMP_CLAUSE_DEPEND), true, false) != MATCH_YES) diff --git a/gcc/testsuite/c-c++-common/gomp/depobj-3.c b/gcc/testsuite/c-c++-common/gomp/depobj-3.c new file mode 100644 index 00000000000..27c66ed5019 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/depobj-3.c @@ -0,0 +1,40 @@ +typedef struct __attribute__((__aligned__ (sizeof (void *)))) omp_depend_t { + char __omp_depend_t__[2 * sizeof (void *)]; +} omp_depend_t; + +void +f () +{ + omp_depend_t obj2; + struct { omp_depend_t c; } s; + float a; + #pragma omp depobj(s.c) depend(inout: a) + + #pragma omp depobj(s.c) destroy(s.c) /* OK */ + + #pragma omp depobj(s.c) destroy(obj2) +/* { dg-error "the 'destroy' expression 'obj2' must be the same as the 'depobj' argument 's.c'" "" { target c } .-1 } */ +/* { dg-error "the 'destroy' expression 'obj2' must be the same as the 'depobj' argument 's.f\\(\\)::::c'" "" { target c++ } .-2 } */ +} + +int +main () +{ + float a; + omp_depend_t obj; + + #pragma omp depobj(obj) depend(inout: a) + + #pragma omp depobj(obj) destroy(obj) /* OK */ + + #pragma omp depobj(obj) destroy(a + 5) +/* { dg-error "'destrory' expression is not lvalue expression" "" { target c } .-1 } */ +/* { dg-error "the 'destroy' expression '\\(a \\+ \\(float\\)5\\)' must be the same as the 'depobj' argument 'obj'" "" { target c++ } .-2 } */ + + #pragma omp depobj(obj+5) destroy(a) +/* { dg-error "invalid operands to binary \\+ \\(have 'omp_depend_t' and 'int'\\)" "" { target c } .-1 } */ +/* { dg-error "no match for 'operator\\+' in 'obj \\+ 5' \\(operand types are 'omp_depend_t' and 'int'\\)" "" { target c++ } .-2 } */ + + #pragma omp depobj(obj) destroy(a) /* { dg-error "the 'destroy' expression 'a' must be the same as the 'depobj' argument 'obj'" } */ + return 0; +} diff --git a/gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 b/gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 new file mode 100644 index 00000000000..a0020014f9e --- /dev/null +++ b/gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 @@ -0,0 +1,18 @@ +! { dg-do compile { target { fortran_integer_16 || ilp32 } } } +! omp_depend_kind = 2*intptr_t --> 16 (128 bit) on 64bit-pointer systems +! --> 8 (128 bit) on 32bit-pointer systems +subroutine f1 + !use omp_lib ! N/A in gcc/testsuite + use iso_c_binding, only: c_intptr_t + implicit none + integer, parameter :: omp_depend_kind = 2*c_intptr_t + integer :: a, b + integer(kind=omp_depend_kind) :: depobj, depobj1(5) + + !$omp depobj(depobj) destroy + + !$omp depobj(depobj) destroy( depobj) + + !$omp depobj(depobj) destroy( depobj2) ! { dg-error "The same depend object must be used as DEPOBJ argument at .1. and as DESTROY argument at .2." } + !$omp depobj(depobj) destroy( a) ! { dg-error "The same depend object must be used as DEPOBJ argument at .1. and as DESTROY argument at .2." } +end diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi index 2f6227c94b2..e5fe7af76af 100644 --- a/libgomp/libgomp.texi +++ b/libgomp/libgomp.texi @@ -387,7 +387,7 @@ to address of matching mapped list item per 5.1, Sect. 2.21.7.2 @tab N @tab @code{-Wall}). Unknown clauses are always rejected with an error.} @item Clauses on @code{end} directive can be on directive @tab Y @tab @item @code{destroy} clause with destroy-var argument on @code{depobj} - @tab N @tab + @tab Y @tab @item Deprecation of no-argument @code{destroy} clause on @code{depobj} @tab N @tab @item @code{linear} clause syntax changes and @code{step} modifier @tab Y @tab