From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) by sourceware.org (Postfix) with ESMTPS id 6CFBC3858D35; Tue, 4 Jul 2023 19:00:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6CFBC3858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.de; s=s31663417; t=1688497224; x=1689102024; i=anlauf@gmx.de; bh=2X11HCNBWgaR5JRNNyqZ+twyn53n0Rs7fUHbFTlnRp8=; h=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To; b=eRnfb3DwuiJJ4jcNAWKwT0ztgDb+xxvhbx0fxyyorKm4ljYOnO14PhoOxVlAzePXljzahAC qDQ1dnNLRTDpUz0ZKvnFeoaMQVp76DL13s9+LUUN0M9X0ipccdYuwGC2Zey6R9vzq4pqBGjWF 2bhhJJIeVy2XBJKF1jQep7+3/RT5VxIiiw2lLPSOj6yLEHO+xLSkDVN9bxupbVGUQVO7QGOEg 8ATwMUNO4kPbOmxbAuVcS/Cz2FBA/fexk8ZzrwbsR9oEdlu4gCmlyl/PtAPH/k8xRXBm6VrtS hYU/2ObyE+6PNMpvB1qoi1iYft1taLaCAq+cw/4juo8urTKB6dZQ== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from [192.168.178.29] ([79.251.14.91]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1M59C2-1qFenc2ASa-001CQq; Tue, 04 Jul 2023 21:00:24 +0200 Content-Type: multipart/mixed; boundary="------------s8o5nY6ocPM2ejf4Ta0VcmX3" Message-ID: Date: Tue, 4 Jul 2023 21:00:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178] To: Mikael Morin , fortran , gcc-patches Newsgroups: gmane.comp.gcc.patches,gmane.comp.gcc.fortran References: <5a5306ae-0db1-c7e2-e744-a3beced17636@orange.fr> <3adc2904-9876-74d6-2b5d-3cc1896866c3@gmx.de> Content-Language: en-US From: Harald Anlauf In-Reply-To: X-Provags-ID: V03:K1:SXOxlWTCFMLtWvuZ+z/8vJ+DRbw3NJN42rOwLLjFo0rbwVF3qvr dxPkYFxAPVJpKUs0wZxTgej0ssHgbPK9OZRYtbyWAbsAJf9jXiolFvNn9NOco1aazlkTNS4 Pv9gdcfqm0UcvsUorZF7wmCSDrFlzSC6xfn25kBN4tQMfKrXV8NiCZyPoE5Ntx+tMdmk5Ag SPK0a6WEFTZIVH1HRCvqw== UI-OutboundReport: notjunk:1;M01:P0:wqBLHabAM4E=;8pgpeQNKRj+jE76PCQD2H5EFrqK uOhh9gQ4wmiysRIUTusd0iZHMkis8W0VkYdtz1Prr+q15ofurZMiDoxPVq7B2Lxy4ldEmI0PR /D9v2DPRcZvrhoq4Yq1OLwLlLOuBsgyaK//3/8cnb880SQXjJuFQ740KOnx4cuQZJa89TJ0Zx XGL7DACQmUaSP8Cchgg1bis08fXhKliGJEoIUvCD4d4HrKYZYvOGznbtivj4QEr1lUHBUwW8N Msz3eWLXub5QRFI88p/AGlaOXEHan9bSDki3esKS8TFZ6VC6Vq9Vv2vmjZb1ZLr8OowCzcFec 4ppgzk+FrdwhaauJejNqKpA9vwF+wBrbwR+rGFdozRFK6JupoV5N18SLzeJKPIq41zxStqDGN 2V4+XsuKaL+/M4n8HWXvcNaEKYYAyfDqXvVyozsymtCCxco2H/HGYLjTGDszDBs0dZc2KUQpr f6/z08oyaU4pFubA/hv+uPF+sGP6vmbQp/Ke/V1F1QTywioT/gvKnVIp543j2rV2/zA8YLsE4 5KBtR65owdYqZUvWvCJeEdwXUPzkHTlCgtU9UYTYHc9AdQPdm/piY3iClrLdJ6UjHJO7krtjK QguAtEclNIozj0I7HH+MoRftvTd7t1yuOg9X0uiRyHJWX2ZzTx9N8tmy1yVSBD0It7nKPX8wl z86cDUyn8SDCAIl26AlgauIpDt8eFiQrrZVoLircChvsVQdl3kzUwxpGZBBv/g8pKCRjaLDs9 cmAmPgyFQfH8Qslv4HO8AYlxGoqAWXS2uhwgt/Xv80GcTQDvnrguHyNrTCkMSDgXNuL4KJ6ci xY+enZmRN7kfIjyhDukfW5lwIZQny7WxSWXnQriDQQ8TovFAzQgrgGcklSGIMQrXNLr3jxOin YU2yIvfBDuY9mXGIYInb2OU0Lv1KWA2JdfCnLsOUX7gAV//1dvcSEnyfU7W5DWltEKl/Svr9u Gjpxb0nTcBseKnPJ+X8F/LnghjA= X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_LOW,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: This is a multi-part message in MIME format. --------------s8o5nY6ocPM2ejf4Ta0VcmX3 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hi Mikael, all, I think I've found it: there is a call to gfc_conv_class_to_class that - according to a comment - does a repackaging to a class array. Deferring that repackaging along with the deallocation not only fixes the regression, but also the cases I tested. Attached is a "sneak preview", hoping that the experts (Paul, Mikael, ...) can tell if I am going down the wrong road. I'll wrap up all pieces and resubmit when the dust settles. We can then address the other findings later. Harald Am 04.07.23 um 15:35 schrieb Mikael Morin: > Le 03/07/2023 =C3=A0 22:49, Harald Anlauf a =C3=A9crit=C2=A0: >> Hi Mikael, >> >> Am 03.07.23 um 13:46 schrieb Mikael Morin: >>> These look good, but I'm surprised that there is no similar change at >>> the 6819 line. >>> This is the class array actual vs class array dummy case. >>> It seems to be checked by the "bar" subroutine in your testcase, excep= t >>> that the intent(out) argument comes last there, whereas it was coming >>> first with the original testcases in the PR. >>> Can you double check? >> >> I believe I tried that before and encountered regressions. >> The change >> >> diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc >> index 16e8f037cfc..43e013fa720 100644 >> --- a/gcc/fortran/trans-expr.cc >> +++ b/gcc/fortran/trans-expr.cc >> @@ -6844,7 +6844,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol = * >> sym, >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tmp =3D gfc_finish_blo= ck (&block); >> >> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 gfc_add_expr_to_block (&se->pre, tmp); >> +//=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 gfc_add_expr_to_block (&se->pre, tmp); >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 gfc_add_expr_to_block (&dealloc_blk, tmp); >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 /* The conversion does not repackage the reference to a >> class >> >> regresses on: >> gfortran.dg/class_array_16.f90 >> gfortran.dg/finalize_12.f90 >> gfortran.dg/optional_class_1.f90 >> >> A simplified testcase for further study: >> >> program p >> =C2=A0=C2=A0 implicit none >> =C2=A0=C2=A0 class(*),=C2=A0 allocatable :: c(:) >> =C2=A0=C2=A0 c =3D [3, 4] >> =C2=A0=C2=A0 call bar (allocated (c), c, allocated (c)) >> =C2=A0=C2=A0 if (allocated (c)) stop 14 >> contains >> =C2=A0=C2=A0 subroutine bar (alloc, x, alloc2) >> =C2=A0=C2=A0=C2=A0=C2=A0 logical :: alloc, alloc2 >> =C2=A0=C2=A0=C2=A0=C2=A0 class(*), allocatable, intent(out) :: x(:) >> =C2=A0=C2=A0=C2=A0=C2=A0 if (allocated (x)) stop 5 >> =C2=A0=C2=A0=C2=A0=C2=A0 if (.not. alloc)=C2=A0=C2=A0 stop 6 >> =C2=A0=C2=A0=C2=A0=C2=A0 if (.not. alloc2)=C2=A0 stop 16 >> =C2=A0=C2=A0 end subroutine bar >> end >> >> (This fails in a different place for the posted patch and for >> the above trial change.=C2=A0 Need to go to the drawing board...) >> > I've had a quick look. > > The code originally generated looks like: > > =C2=A0=C2=A0=C2=A0 D.4343 =3D (void *[0:] * restrict) c._data.data !=3D= 0B; > =C2=A0=C2=A0=C2=A0 if (c._data.data !=3D 0B) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // free c._data.data > =C2=A0=C2=A0=C2=A0 c._data.data =3D 0B; > =C2=A0=C2=A0=C2=A0 ... > =C2=A0=C2=A0=C2=A0 class.3._data =3D c._data; > =C2=A0=C2=A0=C2=A0 ... > =C2=A0=C2=A0=C2=A0 D.4345 =3D (void *[0:] * restrict) c._data.data !=3D= 0B; > =C2=A0=C2=A0=C2=A0 bar (&D.4343, &class.3, &D.4345); > > this fails because D.4345 has the wrong value. > With your change, it becomes: > > =C2=A0=C2=A0=C2=A0 D.4343 =3D (void *[0:] * restrict) c._data.data !=3D= 0B; > =C2=A0=C2=A0=C2=A0 ... > =C2=A0=C2=A0=C2=A0 class.3._data =3D c._data; > =C2=A0=C2=A0=C2=A0 ... > =C2=A0=C2=A0=C2=A0 D.4345 =3D (void *[0:] * restrict) c._data.data !=3D= 0B; > =C2=A0=C2=A0=C2=A0 if (c._data.data !=3D 0B) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // free c._data.data > =C2=A0=C2=A0=C2=A0 c._data.data =3D 0B; > =C2=A0=C2=A0=C2=A0 bar (&D.4343, &class.3, &D.4345); > > and then it is class.3._data that has the wrong value. > So basically the initialization of class.3 should move with the > deallocation. > > I can reproduce a similar problem with your unmodified patch on the > following variant: > > program p > =C2=A0 implicit none > =C2=A0 class(*),=C2=A0 allocatable :: c > =C2=A0 c =3D 3 > =C2=A0 call bar (c, allocated (c)) > =C2=A0 if (allocated (c)) stop 14 > contains > =C2=A0 subroutine bar (x, alloc2) > =C2=A0=C2=A0=C2=A0 logical :: alloc, alloc2 > =C2=A0=C2=A0=C2=A0 class(*), allocatable, intent(out) :: x(..) > =C2=A0=C2=A0=C2=A0 if (allocated (x)) stop 5 > =C2=A0=C2=A0=C2=A0 if (.not. alloc)=C2=A0=C2=A0 stop 6 > =C2=A0=C2=A0=C2=A0 if (.not. alloc2)=C2=A0 stop 16 > =C2=A0 end subroutine bar > end > > > --------------s8o5nY6ocPM2ejf4Ta0VcmX3 Content-Type: text/x-patch; charset=UTF-8; name="pr92178-v2-partial.patch" Content-Disposition: attachment; filename="pr92178-v2-partial.patch" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2djYy9mb3J0cmFuL3RyYW5zLWV4cHIuY2MgYi9nY2MvZm9ydHJhbi90 cmFucy1leHByLmNjCmluZGV4IDE2ZThmMDM3Y2ZjLi5hNjhjOGQzM2FjYyAxMDA2NDQKLS0t IGEvZ2NjL2ZvcnRyYW4vdHJhbnMtZXhwci5jYworKysgYi9nY2MvZm9ydHJhbi90cmFucy1l eHByLmNjCkBAIC02ODA0LDYgKzY4MDQsNyBAQCBnZmNfY29udl9wcm9jZWR1cmVfY2FsbCAo Z2ZjX3NlICogc2UsIGdmY19zeW1ib2wgKiBzeW0sCiAJICAgICAgLyogUGFzcyBhIGNsYXNz IGFycmF5LiAgKi8KIAkgICAgICBwYXJtc2UudXNlX29mZnNldCA9IDE7CiAJICAgICAgZ2Zj X2NvbnZfZXhwcl9kZXNjcmlwdG9yICgmcGFybXNlLCBlKTsKKwkgICAgICBib29sIGRlZmVy X3JlcGFja2FnZSA9IGZhbHNlOwogCiAJICAgICAgLyogSWYgYW4gQUxMT0NBVEFCTEUgZHVt bXkgYXJndW1lbnQgaGFzIElOVEVOVChPVVQpIGFuZCBpcwogCQkgYWxsb2NhdGVkIG9uIGVu dHJ5LCBpdCBtdXN0IGJlIGRlYWxsb2NhdGVkLiAgKi8KQEAgLTY4NDQsNyArNjg0NSw4IEBA IGdmY19jb252X3Byb2NlZHVyZV9jYWxsIChnZmNfc2UgKiBzZSwgZ2ZjX3N5bWJvbCAqIHN5 bSwKIAkJICBlbHNlCiAJCSAgICB0bXAgPSBnZmNfZmluaXNoX2Jsb2NrICgmYmxvY2spOwog Ci0JCSAgZ2ZjX2FkZF9leHByX3RvX2Jsb2NrICgmc2UtPnByZSwgdG1wKTsKKwkJICBnZmNf YWRkX2V4cHJfdG9fYmxvY2sgKCZkZWFsbG9jX2JsaywgdG1wKTsKKwkJICBkZWZlcl9yZXBh Y2thZ2UgPSB0cnVlOwogCQl9CiAKIAkgICAgICAvKiBUaGUgY29udmVyc2lvbiBkb2VzIG5v dCByZXBhY2thZ2UgdGhlIHJlZmVyZW5jZSB0byBhIGNsYXNzCkBAIC02ODU4LDYgKzY4NjAs MTAgQEAgZ2ZjX2NvbnZfcHJvY2VkdXJlX2NhbGwgKGdmY19zZSAqIHNlLCBnZmNfc3ltYm9s ICogc3ltLAogCQkJCSAgICAgJiYgZS0+c3ltdHJlZS0+bi5zeW0tPmF0dHIub3B0aW9uYWws CiAJCQkJICAgICBDTEFTU19EQVRBIChmc3ltKS0+YXR0ci5jbGFzc19wb2ludGVyCiAJCQkJ ICAgICB8fCBDTEFTU19EQVRBIChmc3ltKS0+YXR0ci5hbGxvY2F0YWJsZSk7CisKKwkgICAg ICAvKiBEZWZlciByZXBhY2thZ2luZyBhZnRlciBkZWFsbG9jYXRpb24uICAqLworCSAgICAg IGlmIChkZWZlcl9yZXBhY2thZ2UpCisJCWdmY19hZGRfYmxvY2tfdG9fYmxvY2sgKCZkZWFs bG9jX2JsaywgJnBhcm1zZS5wcmUpOwogCSAgICB9CiAJICBlbHNlCiAJICAgIHsKQEAgLTcx MzEsMTcgKzcxMzcsMTIgQEAgZ2ZjX2NvbnZfcHJvY2VkdXJlX2NhbGwgKGdmY19zZSAqIHNl LCBnZmNfc3ltYm9sICogc3ltLAogCiAgICAgICAvKiBJZiBhbnkgYWN0dWFsIGFyZ3VtZW50 IG9mIHRoZSBwcm9jZWR1cmUgaXMgYWxsb2NhdGFibGUgYW5kIHBhc3NlZAogCSB0byBhbiBh bGxvY2F0YWJsZSBkdW1teSB3aXRoIElOVEVOVChPVVQpLCB3ZSBjb25zZXJ2YXRpdmVseQot CSBldmFsdWF0ZSBhbGwgYWN0dWFsIGFyZ3VtZW50IGV4cHJlc3Npb25zIGJlZm9yZSBkZWFs bG9jYXRpb25zIGFyZQorCSBldmFsdWF0ZSBhY3R1YWwgYXJndW1lbnQgZXhwcmVzc2lvbnMg YmVmb3JlIGRlYWxsb2NhdGlvbnMgYXJlCiAJIHBlcmZvcm1lZCBhbmQgdGhlIHByb2NlZHVy ZSBpcyBleGVjdXRlZC4gIFRoaXMgZW5zdXJlcyB3ZSBjb25mb3JtCi0JIHRvIEYyMDIzOjE1 LjUuMywgMTUuNS40LiAgQ3JlYXRlIHRlbXBvcmFyaWVzIGV4Y2VwdCBmb3IgY29uc3RhbnRz LAotCSB2YXJpYWJsZXMsIGFuZCBmdW5jdGlvbnMgcmV0dXJuaW5nIHBvaW50ZXJzIHRoYXQg Y2FuIGFwcGVhciBpbiBhCi0JIHZhcmlhYmxlIGRlZmluaXRpb24gY29udGV4dC4gICovCisJ IHRvIEYyMDIzOjE1LjUuMywgMTUuNS40LiAgTWF5IGNyZWF0ZSB0ZW1wb3JhcmllcyB3aGVu IG5lZWRlZC4gICovCiAgICAgICBpZiAoZSAmJiBmc3ltICYmIGZvcmNlX2V2YWxfYXJncwot CSAgJiYgZS0+ZXhwcl90eXBlICE9IEVYUFJfVkFSSUFCTEUKLQkgICYmICFnZmNfaXNfY29u c3RhbnRfZXhwciAoZSkKLQkgICYmIChlLT5leHByX3R5cGUgIT0gRVhQUl9GVU5DVElPTgot CSAgICAgIHx8ICEoZ2ZjX2V4cHJfYXR0ciAoZSkucG9pbnRlcgotCQkgICB8fCBnZmNfZXhw cl9hdHRyIChlKS5wcm9jX3BvaW50ZXIpKSkKKwkgICYmIGZzeW0tPmF0dHIuaW50ZW50ICE9 IElOVEVOVF9PVVQKKwkgICYmICFnZmNfaXNfY29uc3RhbnRfZXhwciAoZSkpCiAJcGFybXNl LmV4cHIgPSBnZmNfZXZhbHVhdGVfbm93IChwYXJtc2UuZXhwciwgJnBhcm1zZS5wcmUpOwog CiAgICAgICBpZiAoZnN5bSAmJiBuZWVkX2ludGVyZmFjZV9tYXBwaW5nICYmIGUpCg== --------------s8o5nY6ocPM2ejf4Ta0VcmX3--