Hello,
Le 21/04/2013 23:04, Janus Weil a écrit :
> Hi all,
>
> the attached patch fixes an regression with TRANSFER, which was just
> reported today. The problem was that array-valued SOURCE arguments
> were not treated correctly.
>
> To fix it properly, I had to make 'gfc_target_expr_size' behave
> correctly for arrays and introduced an new function 'gfc_element_size'
> (which, contrary to the former, returns the size of one array element
> instead of the whole expression).
>
> In the process I could also remove the small function 'size_array' and
> had to make some more adjustments in simlify.c, in order to account
> for the slightly modified (and now more consistent) behavior of
> 'gfc_target_expr_size'.
>
Two comments below:
> Index: gcc/fortran/check.c
> ===================================================================
> --- gcc/fortran/check.c (revision 198108)
> +++ gcc/fortran/check.c (working copy)
> @@ -4456,7 +4455,7 @@ gfc_calculate_transfer_sizes (gfc_expr *source, gf
> return false;
>
> /* Calculate the size of the source. */
> - if (source->expr_type == EXPR_ARRAY
> + if ((source->expr_type == EXPR_ARRAY || source->rank > 0)
>
Minor: we can probably assume that rank > 0 if expr_type == EXPR_ARRAY,
which makes the first condition unnecessary.
(There is another instance of this later)
> Index: gcc/fortran/target-memory.c
> ===================================================================
> --- gcc/fortran/target-memory.c (revision 198108)
> +++ gcc/fortran/target-memory.c (working copy)
> +/* Return the size of an expression in its target representation. */
> +
> +size_t
> +gfc_target_expr_size (gfc_expr *e)
> +{
> + mpz_t tmp;
> + size_t asz;
> +
> + gcc_assert (e != NULL);
> +
> + if ((e->expr_type == EXPR_ARRAY || e->rank > 0) && gfc_array_size (e,
> &tmp))
> + asz = mpz_get_ui (tmp);
> + else
> + asz = 1;
> +
> + return asz * gfc_element_size (e);
> +}
>
If gfc_array_size returns false, the function return
gfc_element_size(e), which feels wrong (or confusing at least).
The original gfc_target_expr_size function returned 0 if it couldn't
determine the size (in the CHARACTER case); I think we should stick to
that. And then, the callers should handle that case.
The rest looks good.
Mikael
