Hi Mikael,
> 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)
yes, I was also suspecting the same (but wasn't quite sure). I have
removed it now.
>> 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.
Good point. I have modified it accordingly, and this also means we can
fully remove the call to 'gfc_array_size' in
'gfc_calculate_transfer_sizes' (i.e. the code you commented on above),
since this is now done by 'gfc_target_expr_size'.
> The rest looks good.
Thanks a lot for the review! I will commit the updated patch in the
attachment after another regtest if there are no further comments.
Cheers,
Janus
pr57022_v3.diff
Description: Binary data
