Janus Weil wrote:
here is a patch for a pretty bad wrong-code regression, which affects
all maintained releases of gfortran. For discussion see bugzilla.
2012-12-15 Janus Weil<ja...@gcc.gnu.org>
PR fortran/55072
* trans-array.c (gfc_conv_array_parameter): No packing was done for
full arrays of derived type.
@@ -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)
Without experimenting more carefully, I have the gut feeling that there
are still cases where we might end up with invalid code and there is
missed optimization.
Regarding the latter: If the variable is simply contiguous, there is no
need to pack it. Hence, for
type(t), allocatable :: a(:)
...
call bar(a)
there is no need to pack the array. The problem with the original test
case is that one has a non-CONTIGUOUS pointer:
p => tgt(::2,::2)
call bar(p)
But that has in principle nothing to do with BT_DERIVED. In particular,
I would like to see an additional test case for the first example case
with "ptr" having the CONTIGUOUS attribute - and a check that then no
packing call is invoked.
For the second test case (comment 2, from GiBUU): Here, the problem is
that "full_array_var" is wrongly true:
call s1(OutPart(1,:))
I wonder whether some call to gfc_is_simply_contiguous could solve the
problem for both issues.
(For non-whole arrays one still have to ensure that one passes the
correct element: "call(a)" should pass a->data and not "&a" and "call
bar(a(:,2))" should neither pass "a->data" nor "&a" but "a->data + offset".)
Regarding BT_CLASS: BT_CLASS -> BT_TYPE (with same declared type) should
already be handled via gfc_conv_subref_array_arg, which takes care of
the actual type. Thus, the patched function should only be reachable for
BT_CLASS -> BT_CLASS. Here, packing is required for non-simply
contiguous actual arguments; but after the packing, a class container
has to be re-added. I think one should add a test case for this; testing
declared type == actual type and declared type != actual type - and
either one for both declared type being the same and for the dummy
having the declared type of the ancestor of the declared type of the
actual argument. And all cases for both simply contiguous arrays and
(simply - or better actually) noncontiguous arrays.
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 but I fear that code which lead to the original issue (e.g.
"full_array_var" is true although it shouldn't) is not solved via the patch.
Sorry for listing more my concerns that giving a proper review.
Tobias
