Re: *Ping* Re: [Patch, fortran] PR61831 side-effect deallocation of variable components

Re: *Ping* Re: [Patch, fortran] PR61831 side-effect deallocation of variable components

[Dominique d'Humières]

See https://gcc.gnu.org/ml/gcc-bugs/2015-07/msg01789.html

Dominique

Re: *Ping* Re: [Patch, fortran] PR61831 side-effect deallocation of variable components

[Uros Bizjak]

Hello!

> On Fri, Jul 10, 2015 at 06:35:30PM +0200, Mikael Morin wrote:
>> Ping: https://gcc.gnu.org/ml/fortran/2015-06/msg00075.html
>>
>
> Patch looks ok to me.

The new test introduced several testsuite failures, please see e.g. [1]:

UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O0
scan-tree-dump-times original "__builtin_free" 33
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O0
scan-tree-dump-times original "__builtin_malloc" 15
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O1
scan-tree-dump-times original "__builtin_free" 33
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O1
scan-tree-dump-times original "__builtin_malloc" 15
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O2
scan-tree-dump-times original "__builtin_free" 33
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O2
scan-tree-dump-times original "__builtin_malloc" 15
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O3
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer
-finline-functions   scan-tree-dump-times original "__builtin_free" 33
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O3
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer
-finline-functions   scan-tree-dump-times original "__builtin_malloc"
15
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O3 -g
scan-tree-dump-times original "__builtin_free" 33
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -O3 -g
scan-tree-dump-times original "__builtin_malloc" 15
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -Os
scan-tree-dump-times original "__builtin_free" 33
UNRESOLVED: gfortran.dg/derived_constructor_comps_6.f90   -Os
scan-tree-dump-times original "__builtin_malloc" 15

(Also, the name of the test is not correctly stated in the ChangeLog entry).

Runtime tests can't scan their dumpfiles.

[1] https://gcc.gnu.org/ml/gcc-testresults/2015-07/msg02059.html

Uros.

Re: *Ping* Re: [Patch, fortran] PR61831 side-effect deallocation of variable components

[Steve Kargl]

On Fri, Jul 10, 2015 at 06:35:30PM +0200, Mikael Morin wrote:
> Ping: https://gcc.gnu.org/ml/fortran/2015-06/msg00075.html
> 

Patch looks ok to me.

-- 
Steve

*Ping* Re: [Patch, fortran] PR61831 side-effect deallocation of variable components

[Mikael Morin]

Ping: https://gcc.gnu.org/ml/fortran/2015-06/msg00075.html

Le 21/06/2015 11:48, Mikael Morin a écrit :
> Le 16/05/2015 18:43, Mikael Morin a écrit :
>> Hello,
>>
>> this is about PR61831 where in code like:
>> 	
>> 	type :: string_t
>> 	   character(LEN=1), dimension(:), allocatable :: chars
>> 	end type string_t
>> 	type(string_t) :: prt_in
>> 	(...)
>> 	tmp = new_prt_spec ([prt_in])
>> 	
>> the deallocation of the argument's allocatable components after the
>> procedure call (to new_prt_spec) has the side effect of freeing prt_in's
>> allocatable components, as the array constructor temporary for [prt_in]
>> is a shallow copy of prt_in.
>>
>> This bug is a regression caused by the Dominique's PR41936 memory leak
>> fix, itself based on a patch originally from me.
>>
>> The attached patch is basically a revert of that fix.  It avoids the
>> problem by not deallocating allocatable components in the problematic
>> case, at the price of a (possible) memory leak.  A new function is
>> introduced telling whether there is aliasing, so that we don't regress
>> on PR41936's memory leak when there is no aliasing, and we don't free
>> components when there is aliasing.
>> The possible remaining memory leak case is the case of a "mixed" array
>> constructor with some parts aliasing variables, and some non-aliasing parts.
>>
>> The patch takes also the opportunity to reassemble the scattered
>> procedure argument deallocation code into a single place.
>>
>> The test needs pr65792's fix (thanks Paul), so for the 4.9 branch I
>> propose commenting the parts that depend on PR65792 in the test.
>>
>> Regression tested on x86_64-linux. OK for 6/5/4.9 ?
> 
> Hello,
> 
> I would like to come back to the patch:
> https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01491.html
> 
> PR66082 made me notice one bug in that patch:
> for descriptorless arrays, the patch was deallocating only the first
> element's allocatable components.
> As gfc_conv_array_parameter returns the array data only, the bounds are
> lost and it is not possible to loop through all the elements.
> 
> With the attached patch, the deallocation code is kept in
> gfc_conv_array_parameter where the bounds of descriptorless arrays are
> known.
> To test this fixes the bug, I have added a count of while (1) loops in
> the dump of pr66082's test.  I'm open to better ideas to properly test this.
> 
> For arrays with descriptors, I have decided to not handle them in
> gfc_conv_array_parameter, because in some cases gfc_conv_expr_descriptor
> is called directly from gfc_conv_procedure_call, without passing through
> gfc_conv_array_parameter.
> To be able to select everything but descriptorless arrays for
> allocatable component deallocation, the flags are moved around somewhat.
> The rest is as in the previous patch.
> 
> The test provided is basically unchanged, thus not suitable for the
> branches without pr65792.
> We can decide what to do with it later (backport pr65792 or disable
> parts of the test), I would like to have the fix in mainline first.
> It has been fortran-tested on x86_64-linux, OK for trunk?
> 
> Mikael
> 
> 
>