Re: Possible patch for fortran/57910

[Jerry DeLisle]

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

Re: Possible patch for fortran/57910

[Louis Krupp]

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 
 wrote  
 >  
 > > Le 6 oct. 2016 à 19:35, Louis Krupp  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 
 > >  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  
 > >>  
 > >>  
 > >  
 >  
 > 


pr69955.f90
Description: Binary data

Re: Possible patch for fortran/57910

[Dominique d'Humières]

> Le 6 oct. 2016 à 19:35, Louis Krupp  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 
>  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 
>> 
>> 
> 

Re: Possible patch for fortran/57910

[Louis Krupp]

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 
 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 
 >  
 > 

Re: Possible patch for fortran/57910

[Steve Kargl]

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

Re: Possible patch for fortran/57910

[Dominique d'Humières]

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