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
