* Re: Possible patch for fortran/57910
@ 2016-10-06 17:04 Dominique d'Humières
2016-10-06 17:19 ` Steve Kargl
2016-10-06 17:36 ` Louis Krupp
0 siblings, 2 replies; 6+ messages in thread
From: Dominique d'Humières @ 2016-10-06 17:04 UTC (permalink / raw)
To: Louis Krupp; +Cc: fortran, gcc-patches List
Dear Louis,
> PR fortran/57910
> * trans-expr.c (gfc_add_interface_mapping): Don't try to
> dereference call-by-value scalar argument.
>
> The patch seems to work without breaking other tests.
From the patch, I think the PR number is wrong and should be 69955.
The test fails on darwin with
At line 71 of file /opt/gcc/work/gcc/testsuite/gfortran.dg/pr69955.f90 (unit = 21)
Fortran runtime error: Cannot open file '/proc/974/statm': No such file or directory
Thanks for working on this issue,
Dominique
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible patch for fortran/57910
2016-10-06 17:04 Possible patch for fortran/57910 Dominique d'Humières
@ 2016-10-06 17:19 ` Steve Kargl
2016-10-06 17:36 ` Louis Krupp
1 sibling, 0 replies; 6+ messages in thread
From: Steve Kargl @ 2016-10-06 17:19 UTC (permalink / raw)
To: Dominique d'Humières; +Cc: Louis Krupp, fortran, gcc-patches List
On Thu, Oct 06, 2016 at 07:04:36PM +0200, Dominique d'Humières wrote:
> Dear Louis,
>
> > PR fortran/57910
> > * trans-expr.c (gfc_add_interface_mapping): Don't try to
> > dereference call-by-value scalar argument.
> >
> > The patch seems to work without breaking other tests.
> >From the patch, I think the PR number is wrong and should be 69955.
>
> The test fails on darwin with
>
Why is darwin executing the test? The dejagnu directive
should restrict it to linux systems.
--
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible patch for fortran/57910
2016-10-06 17:04 Possible patch for fortran/57910 Dominique d'Humières
2016-10-06 17:19 ` Steve Kargl
@ 2016-10-06 17:36 ` Louis Krupp
2016-10-06 21:30 ` Dominique d'Humières
1 sibling, 1 reply; 6+ messages in thread
From: Louis Krupp @ 2016-10-06 17:36 UTC (permalink / raw)
To: "Dominique d'Humières"; +Cc: fortran, gcc-patches List
Dominique,
Vous avez raison. I attached the wrong patch. I've resent the message with the correct patch.
I tried to make pr69955.f90 run only on 64-bit Linux:
! { dg-do run { target x86_64-*-linux* } }
I'm not sure there's a portable way to query virtual memory usage, and testing this on one platform seemed to be better than nothing.
Did I get the target wrong? Or do I need to figure out how to make this work on Darwin?
Louis
---- On Thu, 06 Oct 2016 10:04:36 -0700 Dominique d'Humières <dominiq@lps.ens.fr> wrote ----
> Dear Louis,
>
> > PR fortran/57910
> > * trans-expr.c (gfc_add_interface_mapping): Don't try to
> > dereference call-by-value scalar argument.
> >
> > The patch seems to work without breaking other tests.
> From the patch, I think the PR number is wrong and should be 69955.
>
> The test fails on darwin with
>
> At line 71 of file /opt/gcc/work/gcc/testsuite/gfortran.dg/pr69955.f90 (unit = 21)
> Fortran runtime error: Cannot open file '/proc/974/statm': No such file or directory
>
> Thanks for working on this issue,
>
> Dominique
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible patch for fortran/57910
2016-10-06 17:36 ` Louis Krupp
@ 2016-10-06 21:30 ` Dominique d'Humières
2016-10-06 22:52 ` Louis Krupp
0 siblings, 1 reply; 6+ messages in thread
From: Dominique d'Humières @ 2016-10-06 21:30 UTC (permalink / raw)
To: Louis Krupp; +Cc: fortran, gcc-patches List
> Le 6 oct. 2016 à 19:35, Louis Krupp <louis.krupp@zoho.com> a écrit :
>
> Dominique,
>
> Vous avez raison. I attached the wrong patch. I've resent the message with the correct patch.
Which works as expected. Thanks
>
> I tried to make pr69955.f90 run only on 64-bit Linux:
>
> ! { dg-do run { target x86_64-*-linux* } }
>
> I'm not sure there's a portable way to query virtual memory usage, and testing this on one platform seemed to be better than nothing.
>
> Did I get the target wrong? Or do I need to figure out how to make this work on Darwin?
I did miss the the target restriction and I tried to run the test manually with the result I reported. Running it through dejagnu show it as UNSUPPORTED.
Now my general concern is that restricting any test to linux may hide problems with target A or B (darwin for me, but AIX or BSD for others). AFAICT the standard way to check fixes for memory leaks is to use either
! { dg-final { scan-tree-dump-times "__builtin_malloc" xx "original" } }
see, e.g., gfortran.dg/move_alloc_15.f90
and/or
! { dg-final { scan-tree-dump-times "__builtin_free" xx "original" } }
see, e.g., gfortran.dg/transfer_intrinsic_6.f90.
For the original test in pr69955, there are 4 "__builtin_free" and 5 "__builtin_malloc" before your patch, i.e., a memory leak, but only 4 after the patch.
I know that scan-tree-dump-times are quite fragile, but at least they are portable from a target to another one.
I hope it helps.
Dominique
>
> Louis
>
>
> ---- On Thu, 06 Oct 2016 10:04:36 -0700 Dominique d'Humières <dominiq@lps.ens.fr> wrote ----
>> Dear Louis,
>>
>>> PR fortran/57910
>>> * trans-expr.c (gfc_add_interface_mapping): Don't try to
>>> dereference call-by-value scalar argument.
>>>
>>> The patch seems to work without breaking other tests.
>> From the patch, I think the PR number is wrong and should be 69955.
>>
>> The test fails on darwin with
>>
>> At line 71 of file /opt/gcc/work/gcc/testsuite/gfortran.dg/pr69955.f90 (unit = 21)
>> Fortran runtime error: Cannot open file '/proc/974/statm': No such file or directory
>>
>> Thanks for working on this issue,
>>
>> Dominique
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible patch for fortran/57910
2016-10-06 21:30 ` Dominique d'Humières
@ 2016-10-06 22:52 ` Louis Krupp
2016-10-07 0:26 ` Jerry DeLisle
0 siblings, 1 reply; 6+ messages in thread
From: Louis Krupp @ 2016-10-06 22:52 UTC (permalink / raw)
To: "Dominique d'Humières"; +Cc: fortran, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 2765 bytes --]
I've attached an updated patch for pr69955. It works just as you said.
Please let me know if this or my patch for pr57910 is OK to check in.
Louis
---- On Thu, 06 Oct 2016 14:30:29 -0700 Dominique d'Humières <dominiq@lps.ens.fr> wrote ----
>
> > Le 6 oct. 2016 à 19:35, Louis Krupp <louis.krupp@zoho.com> a écrit :
> >
> > Dominique,
> >
> > Vous avez raison. I attached the wrong patch. I've resent the message with the correct patch.
>
> Which works as expected. Thanks
>
> >
> > I tried to make pr69955.f90 run only on 64-bit Linux:
> >
> > ! { dg-do run { target x86_64-*-linux* } }
> >
> > I'm not sure there's a portable way to query virtual memory usage, and testing this on one platform seemed to be better than nothing.
> >
> > Did I get the target wrong? Or do I need to figure out how to make this work on Darwin?
>
> I did miss the the target restriction and I tried to run the test manually with the result I reported. Running it through dejagnu show it as UNSUPPORTED.
>
> Now my general concern is that restricting any test to linux may hide problems with target A or B (darwin for me, but AIX or BSD for others). AFAICT the standard way to check fixes for memory leaks is to use either
>
> ! { dg-final { scan-tree-dump-times "__builtin_malloc" xx "original" } }
>
> see, e.g., gfortran.dg/move_alloc_15.f90
>
> and/or
>
> ! { dg-final { scan-tree-dump-times "__builtin_free" xx "original" } }
>
> see, e.g., gfortran.dg/transfer_intrinsic_6.f90.
>
> For the original test in pr69955, there are 4 "__builtin_free" and 5 "__builtin_malloc" before your patch, i.e., a memory leak, but only 4 after the patch.
>
> I know that scan-tree-dump-times are quite fragile, but at least they are portable from a target to another one.
>
> I hope it helps.
>
> Dominique
>
> >
> > Louis
> >
> >
> > ---- On Thu, 06 Oct 2016 10:04:36 -0700 Dominique d'Humières <dominiq@lps.ens.fr> wrote ----
> >> Dear Louis,
> >>
> >>> PR fortran/57910
> >>> * trans-expr.c (gfc_add_interface_mapping): Don't try to
> >>> dereference call-by-value scalar argument.
> >>>
> >>> The patch seems to work without breaking other tests.
> >> From the patch, I think the PR number is wrong and should be 69955.
> >>
> >> The test fails on darwin with
> >>
> >> At line 71 of file /opt/gcc/work/gcc/testsuite/gfortran.dg/pr69955.f90 (unit = 21)
> >> Fortran runtime error: Cannot open file '/proc/974/statm': No such file or directory
> >>
> >> Thanks for working on this issue,
> >>
> >> Dominique
> >>
> >>
> >
>
>
[-- Attachment #2: pr69955.f90 --]
[-- Type: application/octet-stream, Size: 928 bytes --]
! { dg-do run }
! { dg-options "-fdump-tree-original" }
program p
implicit none
type :: t1
integer, allocatable :: t(:)
end type t1
type :: t2
type(t1), allocatable :: x1(:)
end type t2
type(t2) :: var(10)
integer :: i
integer :: vm_after_short_run, vm_after_long_run
do i= 1, 10
allocate(var(i)%x1(100))
allocate(var(i)%x1(1)%t(100))
enddo
open(unit = 37, file = "/dev/null", status = "old")
call s(1)
close(unit = 37)
do i=1,10
deallocate(var(i)%x1)
enddo
contains
subroutine s(counter)
implicit none
integer, intent(in) :: counter
integer :: i, j, n
do j=1, counter
n = size( [ ( var(i)%x1 , i = 1, size(var) ) ] )
write(unit = 37, fmt = '(i5)') n
enddo
end subroutine
end program p
! { dg-final { scan-tree-dump-times "__builtin_malloc" 4 "original" } }
! { dg-final { scan-tree-dump-times "__builtin_free" 4 "original" } }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible patch for fortran/57910
2016-10-06 22:52 ` Louis Krupp
@ 2016-10-07 0:26 ` Jerry DeLisle
0 siblings, 0 replies; 6+ messages in thread
From: Jerry DeLisle @ 2016-10-07 0:26 UTC (permalink / raw)
To: Louis Krupp, Dominique d'Humières; +Cc: fortran, gcc-patches List
On 10/06/2016 03:52 PM, Louis Krupp wrote:
> I've attached an updated patch for pr69955. It works just as you said.
>
> Please let me know if this or my patch for pr57910 is OK to check in.
>
> Louis
Both are OK. Thanks.
Jerry
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-07 0:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-06 17:04 Possible patch for fortran/57910 Dominique d'Humières
2016-10-06 17:19 ` Steve Kargl
2016-10-06 17:36 ` Louis Krupp
2016-10-06 21:30 ` Dominique d'Humières
2016-10-06 22:52 ` Louis Krupp
2016-10-07 0:26 ` Jerry DeLisle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).