Re: FINAL subroutines

[Paul Richard Thomas <paul.richard.thomas@gmail.com>]

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 <fortran@gcc.gnu.org>
>>>> 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