PR55789 - [4.6/4.7/4.8 Regression] Needless realloc with array constructor

[Paul Richard Thomas]

Hi Mikael,

I am sorry to take so long to reply - I have been frying other fish!

I was aware of the structure and the function of the various functions
that translate array constructors.  My question was rather more
specific to the conditional setting of 'dynamic' that the patch
eliminates.  I have been unable to find any test that triggers it
where 'dynamic' must be set.  I was worried that I was missing
something.

If I do not hear to the contrary over the next 24 hours, I will commit
on the basis of Tobias's OK.

Cheers

Paul

On 12 January 2013 22:07, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 12/01/2013 16:33, Paul Richard Thomas a écrit :
>>
>> Dear All,
>>
>> I think that there must be something that I am missing with this PR.
>>
>> Index: gcc/fortran/trans-array.c
>> ===================================================================
>> *** gcc/fortran/trans-array.c    (revision 195123)
>> --- gcc/fortran/trans-array.c    (working copy)
>> *************** trans_array_constructor (gfc_ss * ss, lo
>> *** 2307,2315 ****
>>        }
>>        }
>>
>> -   if (TREE_CODE (*loop_ubound0) == VAR_DECL)
>> -     dynamic = true;
>> -
>>      gfc_trans_create_temp_array (&outer_loop->pre,&outer_loop->post, ss,
>> type,
>>
>>                       NULL_TREE, dynamic, true, false, where);
>>
>> --- 2307,2312 ----
>>
>> fixes it and regtests OK.  I just do not understand why the loop
>> ubound being a variable should ever result in dynamic being true:
>>
>> According to 4.8 Construction of array values
>> 7 For an ac-implied-do, the loop initialization and execution is the
>> same as for a DO construct.
>>
>> Thus, the initial, terminal and incrementation parameters are
>> evaluated before the loop is initiated and so the loop upper bound
>> being a variable can have no bearing on whether or not dynamic should
>> be true.
>>
> I think the reason is just array constructors are poorly handled.
> - dynamic is set using gfc_get_array_constructor_size which sets the known
> (at compile time) minimal size of the array constructor and returns whether
> there are parts of unknown (IOW non-constant) size.
> - gfc_trans_create_temp_array allocates the temporary with the minimal size.
> - gfc_trans_array_constructor_value initializes the temporary, extending the
> allocated storage on the fly if needed (for the parts of non-constant size).
>
> gfc_get_array_constructor_size should really work with trees instead of mpz,
> so that the initial storage is of adequate size.  Not sure I would try
> though.  It has huge potential for blowing up in one's face.
>
> Regarding your patch, I think it is OK.  dynamic should have been set above
> independently.
>
> Mikael



-- 
The knack of flying is learning how to throw yourself at the ground and miss.
       --Hitchhikers Guide to the Galaxy