From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.126.135]) by sourceware.org (Postfix) with ESMTPS id 5F7EF38582B3; Wed, 10 Jan 2024 21:31:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5F7EF38582B3 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=net-b.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=net-b.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5F7EF38582B3 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=212.227.126.135 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704922303; cv=none; b=A8SWiEzaJ+9bUKKXsaD2JGZPPtsL3+Rgz4mTuaooP6i+pmAlsJw6hfZjDxSUOetGepfW/l5bT3TfLj1FkJ++9oT6j8L54gRuF1B/TcOQstC3oHP9fpYwWp1Lo5YjApoEqObCqyBo5/UxCV6toBga/BiDAkRG5UuMa74wHFqpUpw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704922303; c=relaxed/simple; bh=CE2y8V7eC04ftZlkz6AziWfS5hZWfMWaz4cdjfK4cMQ=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=HZcKwAgXaGHJV0BfpKtXAilfL1P+3z6FV7plmdfILlTN34LvAGez17oJumhz8aNsF7oZU7Vn6+0QRl3fT3YUHwFZRe7ZmiZR1CReM9NBvKGtN+kzMwZdA0pviziQg58HsAhvxsuG4wuN8tGdN9eeoVDxiByauqFE0bz8xWmZXDc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from [192.168.198.216] ([79.153.32.189]) by mrelayeu.kundenserver.de (mreue011 [213.165.67.97]) with ESMTPSA (Nemesis) id 1MkpKR-1qtJl31G63-00mO0Q; Wed, 10 Jan 2024 22:31:34 +0100 Message-ID: <34099853-299a-415e-a762-c84a39f3e334@net-b.de> Date: Wed, 10 Jan 2024 22:31:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/8] OpenMP: lvalue parsing for map/to/from clauses (C) Content-Language: en-US To: Julian Brown , gcc-patches@gcc.gnu.org Cc: fortran@gcc.gnu.org, jakub@redhat.com, tobias@codesourcery.com References: <78fa6c4dae60578a8feffe204bfe24d85d19520c.1693941293.git.julian@codesourcery.com> From: Tobias Burnus In-Reply-To: <78fa6c4dae60578a8feffe204bfe24d85d19520c.1693941293.git.julian@codesourcery.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K1:PRWFn5MLk7q4eGdv8ljMdzLqpgtNpx4tYS1kKqra4KYLOUUL1QG gopLlWlGr4EdyHBnkVx8t7vKKj0bp/FfsYZXxDYf5eprTz5JrXvNbgW41A5Sqz0G03hrCRM /w3PisBsxIStM6Kbl8Wcyhvkq05S8klsr00zV3Xar9jOrrSZ6SjUHDnIDbbcnWrQCbZAMOC /CWbU5uUWIWZd2MstdM8A== UI-OutboundReport: notjunk:1;M01:P0:qK9P/+P7zrY=;spcWIdK9x0qKk4k+RLlSfR3PrkD PyzBEuhqPp2tTTkTK2IWbeMAY3dmtf+4QrTBKJIinSa1KcC22tH/T5XCh0D9MD0QQhTQFJWK3 Qkg08TfK3LSxxmook9UEB9RJstsAEJAy+4BkJ9gQbNmOoXvO/d5GTSIvgBnDRV+5PNwLJGU9e kH/EbrP7Zvit/9PCXr8SnKxedfk+4N5Gba1pWBBXd903PzESqr5Ab+fJ/ExiQeY1ClJSpbDct slURwweNJmXOz0cwPvxffnQyrwvAmYhyiKhmSKPoAvEMp5Hfd05cebty9sc6x5/snkYXK9R0+ LPbs/ft6YAWFi5n8Qz3GHybfQl3BYFpiB3CShB+T0vpvJpkZfWlsdn4t3vLC+4uxw5XeFZGfk a2KHdd1G5bQ5Bch41deUYrIEMTOXEUWLyePKk4C4mVMOowMBc2+MxuAI4uDn+9sxCTy2jVChI 6/1d5fcNWyc8WCGaUfRPbAOYdeGFYUNdgMR4ZZP2fEcRUzQZHTPyvn0c/FJ7Vq409Z62qHCXm LgncFO/pjxyaO6KHGvU2pFTWw3kM/HcO8wwXHBCvFpcTKVeJU/UFu9webaREpwrrbrbIzuLZ1 P92WzgAfBl9TC2j2zHB8i5GJtOfr8ot9AveggiRnY1La9FX75/uwL3GdVCrzXW0+YmKhmZGtT T4sBlmb+wYhkTTiJZUDHcTY097FbKK2piA6GvnbTf0fAYsNTS1N5i4CXGTLDxm5lorAbpzITT T+FOptpc7+WjLZfU5zeHxLVNIKPLUHYKfISMem8Dlme08hWXmu8CfVwCyzp6gueMj62BLPGCj TNAhwTfacGjVI9ycd9HCa1V5Y0SHJ8FOEnVkQcIXh205EeuzkdiRnAmnqyC0ETUNAjkvrYKA4 jreTAjLiCQ12zJw== X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,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 List-Id: Julian Brown wrote: > This patch adds support for parsing general lvalues ("locator list item > types") for OpenMP "map", "to" and "from" clauses to the C front-end, > similar to the previously-posted patch for C++. Such syntax is permitted > for OpenMP 5.0 and above. It was previously posted for mainline here ... In libgomp/libgomp.texi, the following can now be set to 'Y': @item C/C++'s lvalue expressions in @code{to}, @code{from} and @code{map} clauses @tab N @tab > @@ -11253,16 +11263,41 @@ c_parser_postfix_expression_after_primary (c_parser *parser, > case CPP_OPEN_SQUARE: > /* Array reference. */ > c_parser_consume_token (parser); > - idx = c_parser_expression (parser).value; > - c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, > - "expected %<]%>"); > - start = expr.get_start (); > - finish = parser->tokens_buf[0].location; > - expr.value = build_array_ref (op_loc, expr.value, idx); > - set_c_expr_source_range (&expr, start, finish); > - expr.original_code = ERROR_MARK; > - expr.original_type = NULL; > - expr.m_decimal = 0; > + idx = len = NULL_TREE; > + if (!c_omp_array_section_p > + || c_parser_next_token_is_not (parser, CPP_COLON)) > + idx = c_parser_expression (parser).value; > + > + if (c_omp_array_section_p > + && c_parser_next_token_is (parser, CPP_COLON)) > + { > + c_parser_consume_token (parser); > + if (c_parser_next_token_is_not (parser, CPP_CLOSE_SQUARE)) > + len = c_parser_expression (parser).value; > + > + c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, > + "expected %<]%>"); > + > + start = expr.get_start (); > + finish = parser->tokens_buf[0].location; > + expr.value = build_omp_array_section (op_loc, expr.value, idx, > + len); > + set_c_expr_source_range (&expr, start, finish); > + expr.original_code = ERROR_MARK; > + expr.original_type = NULL; > + } > + else > + { > + c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, > + "expected %<]%>"); > + start = expr.get_start (); > + finish = parser->tokens_buf[0].location; > + expr.value = build_array_ref (op_loc, expr.value, idx); > + set_c_expr_source_range (&expr, start, finish); > + expr.original_code = ERROR_MARK; > + expr.original_type = NULL; > + expr.m_decimal = 0; > + } I think that's more readable when moving everything but the expr.value assignment after the if/else (obviously for "if" also everything until "len =" has to remain). That also adds the missing "m_decimal = 0" for the "if" case. > @@ -13915,8 +13955,97 @@ c_parser_omp_variable_list (c_parser *parser, ... > + else if (TREE_CODE (decl) == INDIRECT_REF) > + { > + /* Turn *foo into the representation previously used for > + foo[0]. */ > + decl = TREE_OPERAND (decl, 0); > + STRIP_NOPS (decl); > + > + decl = build_omp_array_section (loc, decl, integer_zero_node, > + integer_one_node); > + } I wonder whether we shouldn't use the C++ wording, i.e. /* If we have "*foo" and - it's an indirection of a reference, "unconvert" it, i.e. strip the indirection (to just "foo"). - it's an indirection of a pointer, turn it into "foo[0:1]". */ * * * As remarked for cp/typecheck.cc's build_omp_array_section: > +tree > +build_omp_array_section (location_t loc, tree array, tree index, tree length) > +{ > + tree idxtype; > + > + if (index != NULL_TREE > + && length != NULL_TREE > + && INTEGRAL_TYPE_P (TREE_TYPE (index)) > + && INTEGRAL_TYPE_P (TREE_TYPE (length))) > + { > + tree low = fold_convert (sizetype, index); > + tree high = fold_convert (sizetype, length); > + high = size_binop (PLUS_EXPR, low, high); > + high = size_binop (MINUS_EXPR, high, size_one_node); > + idxtype = build_range_type (sizetype, low, high); > + } > + else if ((index == NULL_TREE || integer_zerop (index)) > + && length != NULL_TREE > + && INTEGRAL_TYPE_P (TREE_TYPE (length))) > + idxtype = build_index_type (length); > + else > + idxtype = NULL_TREE; > + > + tree type = TREE_TYPE (array); > + gcc_assert (type); > + > + tree sectype, eltype = TREE_TYPE (type); > + > + /* It's not an array or pointer type. Just reuse the type of the original > + expression as the type of the array section (an error will be raised > + anyway, later). */ > + if (eltype == NULL_TREE > + || error_operand_p (eltype) > + || error_operand_p (idxtype)) > + sectype = TREE_TYPE (array); > + else > + sectype = build_array_type (eltype, idxtype); > + > + return build3_loc (loc, OMP_ARRAY_SECTION, sectype, array, index, length); > +} I think that's more readable when having that if condition at the top and moving everything before into the else branch. Otherwise, LGTM. Thanks for the patch! Tobias