[Patch, Fortran] PR 55072: [4.6/4.7/4.8 Regression] Missing internal_pack leads to wrong code with derived type

[Janus Weil]

Hi Mikael,

>> Ping! (What do we do with this little bugger?)
>>
>> @Paul: Was your comment 19 in the PR meant as an OK for my patch
>> (submitted here: http://gcc.gnu.org/ml/fortran/2012-12/msg00097.html),
>> or was it just a general agreement with the previous comments?
>>
> FWIW, I regard the patch as a (conservative) improvement, thus certainly
> acceptable.

To be conservative was exactly my intention, since
a) trunk is in stage 4 and
b) I wanted something that is safe for backporting to 4.6 and 4.7
(where we certainly can not afford to introduce any new wrong-code
regressions).


>>>> @@ -6995,20 +6995,14 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr
>>>> *
>>>>        this_array_result = false;
>>>>        /* Passing address of the array if it is not pointer or
>>>> assumed-shape.  */
>>>> -  if (full_array_var&&  g77&&  !this_array_result)
>>>> +  if (full_array_var&&  g77&&  !this_array_result
>>>> +&&  sym->ts.type != BT_DERIVED&&  sym->ts.type != BT_CLASS)
>
> I would have used instead:
>  && expr->expr_type == EXPR_VARIABLE && ref == NULL)
>
> to make the optimization available to variables of derived type.
> As we are now in stage4, your version should probably be preferred though.

Ok, I will leave it as it is then.


> Regarding:
>
>>>> Regarding the wrong code: I fear that some code involving non-BT_DERIVED
>>>> could lead to wrong code, e.g. "a(:)%x". I don't have an example for
>>>> that
>>>
> I don't think this can happen as the test for non-derived type is on the
> root symbol (`a' in your example).  For other cases, to be honest, I can't
> make any sense of all the booleans interacting with each other in that code
> area (this_array_result, g77, contiguous vs. gfc_is_simply_contiguous, ...).
>
> Regarding the missed optimization, bah, maybe we can defer to 4.9+?

Yes, certainly the upcoming release should better produce code that is
fully correct, rather than "fast but wrong" ;)


Thanks for your review (which I read as an OK for all branches,
right?). Will commit soon.

Cheers,
Janus