From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) by sourceware.org (Postfix) with ESMTPS id 8E6C73858D1E for ; Fri, 28 Jan 2022 09:01:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8E6C73858D1E Received: by mail-qt1-x835.google.com with SMTP id v5so4624076qto.7 for ; Fri, 28 Jan 2022 01:01:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=v90ejowMI0mfLZjgdCNcyWwjxTku8Jy5tuclEu3LIP0=; b=IfHYnb1huqSAdZHocf5tTJbvvq1+47fgePnqzu8UfkLkrMqjBdNX9lC7wACVMSkSMS sGVJUVATYJ9ZAaKxLMUbrEPhL8lY/DiBgFC+KU+51y4kSEpWxLpyXXX69vbF1rNZZWRO kafA4+AWr7PGFNGoIIL9ZHL4SoGbwE7jhl0I9UcMKC1oaoITOj9F6ZFaXfwFfHI3p69x dH6IDctIsjNxB9A4nwHkkKlXSraC+t68FeqK2MpZoU+dt5SZ+ebGz9IxdRgQW44arwoI YTAbyQY6xXPY+jfRCf4XDtdKZbfH/NQGvLaf88+uCbD3kibUQ9fpQBzeN//dn1rIgM9i o5Cw== X-Gm-Message-State: AOAM531hsg3IfOl3b+pcDpRgGx3uQQJ6nMM1qOo7IMdGswftV+sOaGk/ Lo9NqXag201ScGgZ2psGIV80BEQvMwDL2wHj1Hs= X-Google-Smtp-Source: ABdhPJz5K/LrSRpAydZynsJsQnqruFwcpDXktxspcZS6TvbYRn92hgf1jbpLZi1ptJTWIXbHK4/4bqZNCXv9zelEKeo= X-Received: by 2002:ac8:5d94:: with SMTP id d20mr5404142qtx.240.1643360472799; Fri, 28 Jan 2022 01:01:12 -0800 (PST) MIME-Version: 1.0 References: <4449972.oeqMtR4stF@abensonca-precision-7540> <6d037812-1395-3bf6-3de9-5ba331cdaf14@gmail.com> In-Reply-To: From: Paul Richard Thomas Date: Fri, 28 Jan 2022 09:01:01 +0000 Message-ID: Subject: Re: FINAL subroutines To: Salvatore Filippone Cc: Jerry D , Andrew Benson , Damian Rouson , Fortran List X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, 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 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Jan 2022 09:01:21 -0000 Hi Salvatore, That's correct. It's what ifort does too. Thus far I have found only one or two minor niggles with the Intel implementation of finalization. Since, as often or not, the two compilers are frequently used by the same users, I will attempt to make them compliant with one another. I will have a conversation with the Intel developers sometime next week. BTW I noticed a small error in the patch that I attached last night. I introduced a kludge to fix your testcase in the call to gfc_copy_alloc_comp in finalize_function_result. The the expression rank is needed and has been introduced as a new argument to gfc_copy_alloc_comp. Ciao Paul On Fri, 28 Jan 2022 at 08:05, Salvatore Filippone < filippone.salvatore@gmail.com> wrote: > So, you are saying that three calls is the correct number, one for the > LHS, one for the RHS, and one for the deallocation in the main program. > I have (re)-read the standard, I would concur with your interpretation. > > On Thu, Jan 27, 2022 at 11:10 PM Paul Richard Thomas < > paul.richard.thomas@gmail.com> wrote: > >> Hi Salvatore, >> >> >> My reading of F2018: 7.5.6.3 "When finalization occurs" is that three >> calls is correct. (i) Paragraph 1 stipulates that the 'var' expression be >> evaluated before the reallocation and intrinsic assignment; (ii)stipulates >> that the function result be finalised "the result is finalized after >> execution of the innermost >> executable construct containing the reference." ; and (iii) Finalisation >> occurs on going out of scope. >> >> That is what is implemented in the attached. I am working my way through >> the testcase dependencies of PR37336 making sure that finalizat >> ion occurs as required by 7.5.6.3 and in the order defined in the >> previous section. It will all be done in the next few days. >> >> What will remain is finalization of function results within array >> constructors and one or two other corner cases. Following that, the >> existing finalization calls will be brought into the framework as the new >> calls. >> >> Best regards >> >> Paul >> >> >> On Thu, 27 Jan 2022 at 07:17, Salvatore Filippone < >> filippone.salvatore@gmail.com> wrote: >> >>> One more data point: Cray FTN issues TWO calls to the FINAL. >>> Which begs the question: what is the correct number of calls one, two or >>> three? >>> Salvatore >>> >>> fsalvato@daint102:/project/prce01/fsalvato/NUMERICAL/PSBLAS/V4/psblas4/test/newstuff> >>> ftn --version >>> Cray Fortran : Version 11.0.0 >>> fsalvato@daint102:/project/prce01/fsalvato/NUMERICAL/PSBLAS/V4/psblas4/test/newstuff> >>> ftn -o testfinal testfinal.f90 >>> fsalvato@daint102:/project/prce01/fsalvato/NUMERICAL/PSBLAS/V4/psblas4/test/newstuff> >>> ./testfinal >>> Allocating wrapper >>> Calling new_outer_type >>> Assigning outer%test_item >>> Called delete_test_type >>> End of new_outer_type >>> DeAllocating wrapper >>> Called delete_test_type >>> >>> On Wed, Jan 26, 2022 at 11:59 PM Paul Richard Thomas < >>> paul.richard.thomas@gmail.com> wrote: >>> >>>> Hi Jerry, >>>> >>>> I am trying to fix the failure of my latest patch with this very test >>>> case. Otherwise it fixes most of the remaining dependencies in PR37336. >>>> >>>> At a pinch, I could submit the earlier patch that Andrew mentions and >>>> work from there. However, you will note that it does miss one of the >>>> finalizations. This is critical because function results, which should be >>>> finalized, are not. >>>> >>>> I'll keep an eye on the state of the branch. By and large, release >>>> occurs 3-4 months after the start of stage 4. I will leave 2 months maximum. >>>> >>>> Best regards >>>> >>>> Paul >>>> >>>> >>>> On Wed, 26 Jan 2022 at 21:29, Jerry D via Fortran >>>> wrote: >>>> >>>>> Is there any reason these patches can not be applied and use this test >>>>> as a test case? >>>>> >>>>> Regards, >>>>> >>>>> Jerry >>>>> >>>>> On 1/24/22 8:11 AM, Salvatore Filippone via Fortran wrote: >>>>> > Thanks a lot >>>>> > (yes, I suspected both gfortran and intel were wrong, precisely >>>>> because I >>>>> > could see why you'd need two FINAL calls, but not three). >>>>> > >>>>> > Salvatore >>>>> > >>>>> > On Mon, Jan 24, 2022 at 4:45 PM Andrew Benson < >>>>> abenson@carnegiescience.edu> >>>>> > wrote: >>>>> > >>>>> >> Hi Salvatore, >>>>> >> >>>>> >> This looks like it's related to some of the missing finalization >>>>> >> functionality >>>>> >> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37336). Paul has some >>>>> >> patches >>>>> >> (e.g. >>>>> https://gcc.gnu.org/pipermail/fortran/2022-January/057415.html) >>>>> >> which >>>>> >> implement most of the missing functionality. With those patches >>>>> >> incorporated >>>>> >> your code gives the following output with gfortran: >>>>> >> >>>>> >> $ ./testfinal >>>>> >> Allocating wrapper >>>>> >> Calling new_outer_type >>>>> >> Assigning outer%test_item >>>>> >> Called delete_test_type >>>>> >> End of new_outer_type >>>>> >> DeAllocating wrapper >>>>> >> Called delete_test_type >>>>> >> >>>>> >> So there is one more call to the finalizer than you found - I >>>>> haven't >>>>> >> checked >>>>> >> carefully but I would guess this is a deallocation of LHS on >>>>> assignment. >>>>> >> >>>>> >> In testing these patches using the Intel compiler we found that it >>>>> seems >>>>> >> to >>>>> >> call the finalization wrapper more than it should, sometimes on >>>>> objects >>>>> >> that >>>>> >> have already been deallocated. Your code, compiled with the Intel >>>>> compiler >>>>> >> and >>>>> >> then run under valgrind shows the following: >>>>> >> >>>>> >> $ valgrind ./testfinal >>>>> >> ==7340== Memcheck, a memory error detector >>>>> >> ==7340== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward >>>>> et al. >>>>> >> ==7340== Using Valgrind-3.13.0 and LibVEX; rerun with -h for >>>>> copyright info >>>>> >> ==7340== Command: ./testfinal >>>>> >> ==7340== >>>>> >> ==7340== Conditional jump or move depends on uninitialised value(s) >>>>> >> ==7340== at 0x493A51: __intel_sse2_strcpy (in >>>>> /home/abensonca/Scratch/ >>>>> >> ifortTests/testfinal) >>>>> >> ==7340== by 0x45D70E: for__add_to_lf_table (in >>>>> /home/abensonca/Scratch/ >>>>> >> ifortTests/testfinal) >>>>> >> ==7340== by 0x4410CB: for__open_proc (in /home/abensonca/Scratch/ >>>>> >> ifortTests/testfinal) >>>>> >> ==7340== by 0x423A64: for__open_default (in >>>>> /home/abensonca/Scratch/ >>>>> >> ifortTests/testfinal) >>>>> >> ==7340== by 0x4305A9: for_write_seq_lis (in >>>>> /home/abensonca/Scratch/ >>>>> >> ifortTests/testfinal) >>>>> >> ==7340== by 0x4047E1: MAIN__ (testfinal.f90:62) >>>>> >> ==7340== by 0x403CE1: main (in >>>>> /home/abensonca/Scratch/ifortTests/ >>>>> >> testfinal) >>>>> >> ==7340== >>>>> >> Allocating wrapper >>>>> >> Calling new_outer_type >>>>> >> Assigning outer%test_item >>>>> >> Called delete_test_type >>>>> >> ==7340== Conditional jump or move depends on uninitialised value(s) >>>>> >> ==7340== at 0x40572A: do_alloc_copy (in >>>>> >> /home/abensonca/Scratch/ifortTests/ >>>>> >> testfinal) >>>>> >> ==7340== by 0x406B9A: do_alloc_copy (in >>>>> >> /home/abensonca/Scratch/ifortTests/ >>>>> >> testfinal) >>>>> >> ==7340== by 0x4084ED: for_alloc_assign_v2 (in >>>>> /home/abensonca/Scratch/ >>>>> >> ifortTests/testfinal) >>>>> >> ==7340== by 0x404474: target_mod_mp_new_outer_type_ >>>>> (testfinal.f90:48) >>>>> >> ==7340== by 0x40485E: MAIN__ (testfinal.f90:65) >>>>> >> ==7340== by 0x403CE1: main (in >>>>> /home/abensonca/Scratch/ifortTests/ >>>>> >> testfinal) >>>>> >> ==7340== >>>>> >> Called delete_test_type >>>>> >> End of new_outer_type >>>>> >> DeAllocating wrapper >>>>> >> Called delete_test_type >>>>> >> ==7340== >>>>> >> ==7340== HEAP SUMMARY: >>>>> >> ==7340== in use at exit: 48 bytes in 1 blocks >>>>> >> ==7340== total heap usage: 14 allocs, 13 frees, 12,879 bytes >>>>> allocated >>>>> >> ==7340== >>>>> >> ==7340== LEAK SUMMARY: >>>>> >> ==7340== definitely lost: 48 bytes in 1 blocks >>>>> >> ==7340== indirectly lost: 0 bytes in 0 blocks >>>>> >> ==7340== possibly lost: 0 bytes in 0 blocks >>>>> >> ==7340== still reachable: 0 bytes in 0 blocks >>>>> >> ==7340== suppressed: 0 bytes in 0 blocks >>>>> >> ==7340== Rerun with --leak-check=full to see details of leaked >>>>> memory >>>>> >> ==7340== >>>>> >> ==7340== For counts of detected and suppressed errors, rerun with: >>>>> -v >>>>> >> ==7340== Use --track-origins=yes to see where uninitialised values >>>>> come >>>>> >> from >>>>> >> ==7340== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 >>>>> from 0) >>>>> >> >>>>> >> so there are some cases of what look like incorrect accesses (and >>>>> some >>>>> >> leaked >>>>> >> memory). >>>>> >> >>>>> >> Your code compiled with gfortran (with Paul's patches in place) >>>>> shows no >>>>> >> errors or leaks from valgrind. >>>>> >> >>>>> >> So, in summary, in this case I think the current gfortran is >>>>> missing some >>>>> >> finalizations (which are fixed by Paul's patches), and ifort is >>>>> likely >>>>> >> doing >>>>> >> something wrong and probably calling the finalizer more times than >>>>> it >>>>> >> should. >>>>> >> >>>>> >> -Andrew >>>>> >> >>>>> >> On Monday, January 24, 2022 6:49:23 AM PST Salvatore Filippone via >>>>> Fortran >>>>> >> wrote: >>>>> >>> And here is the code embedded as text............ sorry about >>>>> sending an >>>>> >>> attachment that was purged >>>>> >>> ------------------------- testfinal.f90 --------------------- >>>>> >>> module test_type_mod >>>>> >>> >>>>> >>> type :: my_test_type >>>>> >>> integer, allocatable :: i >>>>> >>> contains >>>>> >>> final :: delete_test_type >>>>> >>> end type my_test_type >>>>> >>> >>>>> >>> interface my_test_type >>>>> >>> module procedure new_test_type_object >>>>> >>> end interface my_test_type >>>>> >>> >>>>> >>> contains >>>>> >>> >>>>> >>> subroutine delete_test_type(this) >>>>> >>> type(my_test_type) :: this >>>>> >>> >>>>> >>> write(*,*) 'Called delete_test_type' >>>>> >>> if (allocated(this%i)) deallocate(this%i) >>>>> >>> >>>>> >>> end subroutine delete_test_type >>>>> >>> >>>>> >>> >>>>> >>> function new_test_type_object(item) result(res) >>>>> >>> type(my_test_type) :: res >>>>> >>> integer, intent(in) :: item >>>>> >>> !Allocation on assignment >>>>> >>> res%i=item >>>>> >>> end function new_test_type_object >>>>> >>> >>>>> >>> >>>>> >>> end module test_type_mod >>>>> >>> >>>>> >>> module target_mod >>>>> >>> use test_type_mod >>>>> >>> type :: outer_type >>>>> >>> type(my_test_type), allocatable :: test_item >>>>> >>> end type outer_type >>>>> >>> >>>>> >>> contains >>>>> >>> >>>>> >>> subroutine new_outer_type(outer,item) >>>>> >>> type(outer_type), intent(out) :: outer >>>>> >>> integer :: item >>>>> >>> >>>>> >>> allocate(outer%test_item) >>>>> >>> write(*,*) 'Assigning outer%test_item' >>>>> >>> outer%test_item = my_test_type(itemi) >>>>> >>> write(*,*) 'End of new_outer_type' >>>>> >>> end subroutine new_outer_type >>>>> >>> >>>>> >>> end module target_mod >>>>> >>> >>>>> >>> program testfinal >>>>> >>> use target_mod >>>>> >>> >>>>> >>> implicit none >>>>> >>> >>>>> >>> integer :: i=10 >>>>> >>> type(outer_type), allocatable :: wrapper >>>>> >>> >>>>> >>> write(*,*) 'Allocating wrapper ' >>>>> >>> allocate(wrapper) >>>>> >>> write(*,*) 'Calling new_outer_type ' >>>>> >>> call new_outer_type(wrapper,i) >>>>> >>> write(*,*) 'DeAllocating wrapper ' >>>>> >>> deallocate(wrapper) >>>>> >>> >>>>> >>> end program testfinal >>>>> >>> >>>>> >>> On Mon, Jan 24, 2022 at 2:50 PM Salvatore Filippone < >>>>> >>> >>>>> >>> filippone.salvatore@gmail.com> wrote: >>>>> >>>> Hi all >>>>> >>>> The attached code compiles and runs fine under both GNU and >>>>> Intel, but >>>>> >> it >>>>> >>>> produces different results, in particular the FINAL subroutine is >>>>> >> invoked >>>>> >>>> just once with GNU, three times with Intel. >>>>> >>>> >>>>> >>>> It seems to me that they cannot both be right; I am not sure what >>>>> the >>>>> >>>> standard is mandating in this case. >>>>> >>>> Any ideas? >>>>> >>>> Salvatore >>>>> >>>> --------------- Intel >>>>> >>>> [pr1eio03@login1: newstuff]$ ifort -v >>>>> >>>> ifort version 19.1.1.217 >>>>> >>>> [pr1eio03@login1: newstuff]$ ifort -o testfinal testfinal.f90 >>>>> >>>> [pr1eio03@login1: newstuff]$ ./testfinal >>>>> >>>> >>>>> >>>> Allocating wrapper >>>>> >>>> Calling new_outer_type >>>>> >>>> Assigning outer%test_item >>>>> >>>> Called delete_test_type >>>>> >>>> Called delete_test_type >>>>> >>>> End of new_outer_type >>>>> >>>> DeAllocating wrapper >>>>> >>>> Called delete_test_type >>>>> >>>> >>>>> >>>> ----------------------------- GNU >>>>> >>>> sfilippo@lagrange newstuff]$ gfortran -v >>>>> >>>> Using built-in specs. >>>>> >>>> COLLECT_GCC=gfortran >>>>> >>>> >>>>> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/11/lto-wrapper >>>>> >>>> OFFLOAD_TARGET_NAMES=nvptx-none >>>>> >>>> OFFLOAD_TARGET_DEFAULT=1 >>>>> >>>> Target: x86_64-redhat-linux >>>>> >>>> Configured with: ../configure --enable-bootstrap >>>>> >>>> --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,lto >>>>> >> --prefix=/usr >>>>> >>>> --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl= >>>>> >>>> http://bugzilla.redhat.com/bugzilla --enable-shared >>>>> >>>> --enable-threads=posix --enable-checking=release --enable-multilib >>>>> >>>> --with-system-zlib --enable-__cxa_atexit >>>>> --disable-libunwind-exceptions >>>>> >>>> --enable-gnu-unique-object --enable-linker-build-id >>>>> >>>> --with-gcc-major-version-only --with-linker-hash-style=gnu >>>>> >> --enable-plugin >>>>> >>>> --enable-initfini-array >>>>> >>>> >>>>> >> >>>>> --with-isl=/builddir/build/BUILD/gcc-11.2.1-20210728/obj-x86_64-redhat-lin >>>>> >>>> ux/isl-install --enable-offload-targets=nvptx-none >>>>> >> --without-cuda-driver >>>>> >>>> --enable-gnu-indirect-function --enable-cet --with-tune=generic >>>>> >>>> --with-arch_32=i686 --build=x86_64-redhat-linux >>>>> >>>> Thread model: posix >>>>> >>>> Supported LTO compression algorithms: zlib zstd >>>>> >>>> gcc version 11.2.1 20210728 (Red Hat 11.2.1-1) (GCC) >>>>> >>>> [sfilippo@lagrange newstuff]$ gfortran -o testfinal testfinal.f90 >>>>> >>>> [sfilippo@lagrange newstuff]$ ./testfinal >>>>> >>>> >>>>> >>>> Allocating wrapper >>>>> >>>> Calling new_outer_type >>>>> >>>> Assigning outer%test_item >>>>> >>>> End of new_outer_type >>>>> >>>> DeAllocating wrapper >>>>> >>>> Called delete_test_type >>>>> >>>> >>>>> >>>> --------------------- >>>>> >> >>>>> >> -- >>>>> >> >>>>> >> * Andrew Benson: https://abensonca.github.io >>>>> >> >>>>> >> * Galacticus: https://github.com/galacticusorg/galacticus >>>>> >> >>>>> >> >>>>> >> >>>>> >> >>>>> >>>>> >>>> >>>> -- >>>> "If you can't explain it simply, you don't understand it well enough" - >>>> Albert Einstein >>>> >>> >> >> -- >> "If you can't explain it simply, you don't understand it well enough" - >> Albert Einstein >> > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein