From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 2F0313858D1E; Mon, 20 Feb 2023 16:23:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2F0313858D1E 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.97,313,1669104000"; d="scan'208";a="97604888" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 20 Feb 2023 08:23:52 -0800 IronPort-SDR: 89Xm9eefzPbVhNvw8kDJo/2eTukuKOBqzAYEabh6reXpgumeqKC4S71w52g9kdg6MByp1fLy46 IR6lmpLLO3jxamnJhmOJxac3uKpOyPBUblRHQ+cOT/Onlx8DopL1ASbDrdLVj2Hlxj16SxHP11 AKKIB/VyWvEkxL/4C2mFMapuQIZVCglUld9JBZz4FA7BbT+un3SJi1603Ly2ZdqCUyp7V+sYxp GeBiWG4hJC2SGjyjAY8Lc7IWWO41L3VdRUJUb4cyyL2d/Fqf7zdc0r4ZfCsj3Ew872WKZdOOMe DTA= Message-ID: Date: Mon, 20 Feb 2023 17:23:44 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Subject: Re: [Patch] Fortran: Avoid SAVE_EXPR for deferred-len char types Content-Language: en-US To: Richard Biener , Jakub Jelinek CC: , gcc-patches , fortran , Paul Richard Thomas References: <27cd606a-f019-60b2-a9c8-0a570433b5eb@codesourcery.com> <34e0f9e6-ebb8-ce86-ea11-08b37e5be1f8@codesourcery.com> <8d3fae03-2638-9c6b-eccc-d0a31d1b9733@codesourcery.com> From: Tobias Burnus In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Richard, hi all, On 20.02.23 13:46, Richard Biener wrote: > + /* TODO: A more middle-end friendly alternative would be to use NU= LL_TREE > + as upper bound and store the value, e.g. as GFC_DECL_STRING_LEN. > + Caveat: this requires some cleanup throughout the code to consis= tently > + use some wrapper function. */ > + gcc_assert (TREE_CODE (TYPE_SIZE_UNIT (type)) =3D=3D SAVE_EXPR); > + tree tmp =3D TREE_TYPE (TYPE_SIZE (eltype)); > > ... > > you are probably breaking type sharing here. You could use > build_array_type_1 and pass false for 'shared' to get around that. Note > that there's also canonical type building done in case 'eltype' is not > canonical itself. My feeling is that this is already somewhat broken. Currently, there is one type per decl as each has its own artificial length variable. I have no idea how this will be handled in the ME in terms of alias analysis. And whether shared=3Dfalse makes sense here and what effect is has. (Probably yes.) In principle, integer(kind=3D8) .str., .str2; character(kind=3D1)[1:.str] * str; character(kind=3D1)[1:.str2] * str2; have the same type and iff .str =3D=3D .str at runtime, they can alias. Example: str2 =3D str; .str2 =3D .str; I have no idea how the type analysis currently works (with or without SAVE_= EXPR) nor what effect shared=3Dfalse has in this case. > The solution to the actual problem is a hack - you are relying on > re-evaluation of TYPE_SIZE, and for that, only from within accesses > from inside the frontend? I think this mostly helps with access inside the FE of the type 'size =3D TYPE_SIZE_UNIT(type)', which is used surprisingly often and is often directly evaluated (i.e. assigned to a temporary). > Since gimplification will produce the result into a single temporary agai= n, re-storing the "breakage". > So, does it_really_ fix things? It does seem to fix cases which do 'size =3D TYPE_SIZE_UNIT (type);' in the front end and then uses this size expression. Thus, there are fixed. However, there are many cases where things go wrong - with and without the patch. I keep discovering more and more :-( * * * I still think that the proper way is to have NULL_TREE as upper value would be better in several ways, except that there is (too) much code which relies on TYPE_UNIT_SIZE to work. (There are 117 occurrences). Additionally, there is more code doing assumptions in this area. Thus, the question is whether it makes sense as hackish partial solution or whether it should remain in the current broken stage until it is fixed properly. Tobias, who would like to have more time for fixing such issues. ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955