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

[Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>]

Dear All,

Well, one day turned to thirteen! Sorry about the long delay - daytime
stuff intervened.

Committed as revision 195815.


I'll be very careful about 4.7 - there are a lot of differences in the
trans_array_constructor coding concerning the 'dynamic' flag.

Cheers

Paul


On 24 January 2013 21:03, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> 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



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