Hello,
Le 30/04/2015 15:07, Andre Vehreschild a écrit :
> Hi all,
>
> this is just a service release. I encountered that the new testcase in the
> previous release included the testcase of the initial patch, that is
> already on trunk. I therefore replaced the testcase allocate_with_source_5.f90
> by allocate_with_source_6.f90 (the extended testcase). Besides this there is
> no
> difference inbetween this and the patch in:
>
> https://gcc.gnu.org/ml/fortran/2015-04/msg00121.html
>
> Sorry for the mess. For a description of the original patches scope see below.
>
> Bootstraps and regtests ok on x86_64-linux-gnu/F21.
>
> Ok for trunk?
>
> Regards,
> Andre
>
> On Wed, 29 Apr 2015 14:31:01 +0200
> Andre Vehreschild <ve...@gmx.de> wrote:
>
>> Hi all,
>>
>> after the first patch to fix the issue reported in the pr, some more issues
>> were reported, which are now fixed by this new patch, aka the 2nd take.
>>
>> The patch modifies the gfc_trans_allocate() in order to pre-evaluate all
>> source= expressions. It no longer rejects array valued source= expressions,
>> but just uses gfc_conv_expr_descriptor () for most of them. Furthermore, is
>> the allocate now again able to allocate arrays of strings. This feature
>> previously slipped my attention.
>>
>> Although the reporter has not yet reported, that the patch fixes his issue, I
>> like to post it for review, because there are more patches in my pipeline,
>> that depend on this one.
>>
>> Bootstraps and regtests ok on x86_64-linux-gnu/F21.
>>
>> Ok, for trunk?
>>
questions below
>
>
> *** trans-stmt.c 2015-05-12 14:42:17.882108651 +0200
> --- trans-stmt.c.modif 2015-05-12 14:42:11.300108561 +0200
> ***************
> *** 5205,5213 ****
> /* In all other cases evaluate the expr3 and create a
> temporary. */
> gfc_init_se (&se, NULL);
> if (code->expr3->rank != 0
> ! && code->expr3->expr_type == EXPR_FUNCTION
> ! && code->expr3->value.function.isym)
> gfc_conv_expr_descriptor (&se, code->expr3);
> else
> gfc_conv_expr_reference (&se, code->expr3);
> --- 5198,5222 ----
> /* In all other cases evaluate the expr3 and create a
> temporary. */
> gfc_init_se (&se, NULL);
> + /* For more complicated expression, the decision when to get the
> + descriptor and when to get a reference is depending on more
> + conditions. The descriptor is only retrieved for functions
> + that are intrinsic, elemental user-defined and known, or neither
> + of the two, or are a class or type, that has a not deferred type
> + array_spec. */
> if (code->expr3->rank != 0
> ! && (code->expr3->expr_type != EXPR_FUNCTION
> ! || code->expr3->value.function.isym
> ! || (code->expr3->value.function.esym &&
> ! code->expr3->value.function.esym->attr.elemental)
> ! || (!code->expr3->value.function.isym
> ! && !code->expr3->value.function.esym)
> ! || (code->expr3->ts.type == BT_DERIVED
> ! && code->expr3->ts.u.derived->as
> ! && code->expr3->ts.u.derived->as->type != AS_DEFERRED)
> ! || (code->expr3->ts.type == BT_CLASS
> ! && CLASS_DATA (code->expr3)->as
> ! && CLASS_DATA (code->expr3)->as->type != AS_DEFERRED)))
> gfc_conv_expr_descriptor (&se, code->expr3);
> else
> gfc_conv_expr_reference (&se, code->expr3);
What is the rationale for choosing between gfc_conv_expr_descriptor and
gfc_conv_expr_reference?
Is it contiguous array vs non-contiguous or needing an evaluation?
For example why not use gfc_conv_expr_descriptor for derived type arrays?
> ***************
> *** 5646,5659 ****
> }
> else if (code->expr3->ts.type == BT_CHARACTER)
> {
> ! tmp = INDIRECT_REF_P (se.expr) ?
> se.expr :
> build_fold_indirect_ref_loc (input_location,
> se.expr);
> ! gfc_trans_string_copy (&block, al_len, tmp,
> ! code->expr3->ts.kind,
> ! expr3_len, expr3,
> ! code->expr3->ts.kind);
> tmp = NULL_TREE;
> }
> else if (al->expr->ts.type == BT_CLASS)
> --- 5658,5707 ----
> }
> else if (code->expr3->ts.type == BT_CHARACTER)
> {
> ! tree dst, src, dlen, slen;
> ! /* For arrays of char arrays, a ref to the data component still
> ! needs to be added, because se.expr upto now only contains the
> ! descritor. */
> ! if (expr->ref && se.expr && TREE_TYPE (se.expr) != NULL_TREE
> ! && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se.expr)))
> ! {
> ! dst = gfc_conv_array_data (se.expr);
> ! src = gfc_conv_array_data (expr3);
> ! /* For CHARACTER (len=string_length), dimension (nelems)
> ! compute the total length of the string to copy. */
> ! if (nelems)
> ! {
> ! dlen = fold_build2_loc (input_location, MULT_EXPR,
> ! size_type_node,
> ! fold_convert (size_type_node,
> ! se.string_length),
> ! fold_convert (size_type_node,
> ! nelems));
> ! slen = fold_build2_loc (input_location, MULT_EXPR,
> ! size_type_node,
> ! fold_convert (size_type_node,
> ! expr3_len),
> ! fold_convert (size_type_node,
> ! nelems));
> ! }
> ! else
> ! {
> ! dlen = se.string_length;
> ! slen = expr3_len;
> ! }
> ! }
> ! else
> ! {
> ! dst = INDIRECT_REF_P (se.expr) ?
> se.expr :
> build_fold_indirect_ref_loc (input_location,
> se.expr);
> ! src = expr3;
> ! dlen = al_len;
> ! slen = expr3_len;
> ! }
> ! gfc_trans_string_copy (&block, dlen, dst, code->expr3->ts.kind,
> ! slen, src, code->expr3->ts.kind);
> tmp = NULL_TREE;
> }
> else if (al->expr->ts.type == BT_CLASS)
This seems to assume that the array is contiguous.
Can't we just fall back to the default case for characters?
The rest looks good.
Mikael
