| Message ID | 7d28e6ef-ad7e-af12-5a25-bb92e0134b62@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Fortran] PR91640 – Fix call to contiguous dummy | expand |
Hi Tobias, > If one passes something which is not a variable as an argument, there is > no point to do the copy-out – the copy-in is sufficient. Note that a the > result of a pointer-returning function is regarded as variable. > > As cleanup, I also fixed the indentation (twice) and the pointless 'fsym > ?' check inside a condition body where the condition already checks this. > > Build on x86-64-gnu-linux. > OK for the trunk – and for GCC 9? [It's a 9/10 regression] The change to avoid the copy-out via INTENT(IN) makes sense. If you add this, it would be good to add a test (for example counting while statements in the *.original dump) that the copyback does not happen. Generally, if we are passing an expression, the call to gfc_conv_subref_array_arg is not needed - we will generate an array temporary for the expression anyway, and this will always be contiguous. Regards Thomas
Hi Tobias, > PS: I lost a bit the overview. Is there any patch pending review or > otherwise pending? From my side, there is the patch for PR 65428, https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00040.html Apart from that, I don't see any outstanding patches. Regards Thomas
Hi Thomas, On 1/3/20 7:18 PM, Thomas König wrote: >> Build on x86-64-gnu-linux. >> OK for the trunk – and for GCC 9? [It's a 9/10 regression] Is is now okay? > If you add this, it would be good to add a test (for example counting > while statements in the *.original dump) that the copyback does not > happen. I have now added something, which hopefully correctly captures this. I added 'xxx' to have some example how a copy out could look like. (That way, I also test whether a copy out happens.) That creating a well-working pattern is difficult as get_var() also uses atmp. – However, looking at the dump, I found that the copy out will call get_var again! That's now PR 93148. > Generally, if we are passing an expression, the call to > gfc_conv_subref_array_arg is not needed - we will generate an array > temporary for the expression anyway, and this will always be contiguous. True – but one needs to call some function. Whether gfc_conv_subref_array_arg with INTENT_IN or gfc_conv_array_parameter does not matter. As a variant, I now use the latter (via the else branch). Either variant produces the same original tree. One can argue which variant is clearer; I think both are fine – pick one. Tobias
Hi Tobias, > As a variant, I now use the latter (via the else branch). Either variant > produces the same original tree. One can argue which variant is clearer; > I think both are fine – pick one. I think the second one (the one you just attached) looks better. So, OK for trunk. Thanks for the patch! Regards Thomas
PR fortran/91640 * trans-expr.c (gfc_conv_procedure_call): Avoid copy-out for nonvariable arguments to contiguous dummy args. Avoid re-checking whether fsym is NULL. PR fortran/91640 * gfortran.dg/contiguous_10.f90: New. gcc/fortran/trans-expr.c | 19 +++++------ gcc/testsuite/gfortran.dg/contiguous_10.f90 | 49 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index f2fe538a511..471540453dc 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -6177,37 +6177,38 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, fsym ? fsym->attr.intent : INTENT_INOUT, fsym && fsym->attr.pointer); else if (gfc_is_class_array_ref (e, NULL) - && fsym && fsym->ts.type == BT_DERIVED) + && fsym && fsym->ts.type == BT_DERIVED) /* The actual argument is a component reference to an array of derived types. In this case, the argument is converted to a temporary, which is passed and then written back after the procedure call. OOP-TODO: Insert code so that if the dynamic type is the same as the declared type, copy-in/copy-out does not occur. */ gfc_conv_subref_array_arg (&parmse, e, nodesc_arg, - fsym ? fsym->attr.intent : INTENT_INOUT, - fsym && fsym->attr.pointer); + fsym->attr.intent, + fsym->attr.pointer); else if (gfc_is_class_array_function (e) - && fsym && fsym->ts.type == BT_DERIVED) + && fsym && fsym->ts.type == BT_DERIVED) /* See previous comment. For function actual argument, the write out is not needed so the intent is set as intent in. */ { e->must_finalize = 1; gfc_conv_subref_array_arg (&parmse, e, nodesc_arg, - INTENT_IN, - fsym && fsym->attr.pointer); + INTENT_IN, fsym->attr.pointer); } else if (fsym && fsym->attr.contiguous && !gfc_is_simply_contiguous (e, false, true)) { - gfc_conv_subref_array_arg (&parmse, e, nodesc_arg, - fsym ? fsym->attr.intent : INTENT_INOUT, - fsym && fsym->attr.pointer); + sym_intent intent = fsym->attr.intent; + if (!gfc_expr_is_variable (e)) + intent = INTENT_IN; + gfc_conv_subref_array_arg (&parmse, e, nodesc_arg, intent, + fsym->attr.pointer); } else gfc_conv_array_parameter (&parmse, e, nodesc_arg, fsym, sym->name, NULL); diff --git a/gcc/testsuite/gfortran.dg/contiguous_10.f90 b/gcc/testsuite/gfortran.dg/contiguous_10.f90 new file mode 100644 index 00000000000..46a497d6967 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/contiguous_10.f90 @@ -0,0 +1,49 @@ +! { dg-do run } +! +! PR fortran/91640 +! +! Based on G. Steinmetz's test case +! +program p + implicit none (type, external) + real, target :: z(3) = 1.0 + real :: res(3) + + res = 42.0 + call sub (-z, res) + if (any (abs (res - (-1.0)) > epsilon(res))) stop 1 + if (any (abs (z - 1.0) > epsilon(z))) stop 2 + + res = 43.0 + call sub (z*2.0, res) + if (any (abs (res - 2.0) > epsilon(res))) stop 3 + if (any (abs (z - 1.0) > epsilon(z))) stop 4 + + res = 44.0 + call sub(get_var(), res) + if (any (abs (res - 1.0) > epsilon(res))) stop 5 + if (any (abs (z - 1.0) > epsilon(z))) stop 6 + + call double(get_var()) + if (any (abs (z - 2.0) > epsilon(z))) stop 7 + + call double(get_var_cont()) + if (any (abs (z - 4.0) > epsilon(z))) stop 8 +contains + subroutine sub (x, res) + real, contiguous :: x(:), res(:) + res = x + end + subroutine double (x) + real, contiguous :: x(:) + x = x * 2.0 + end + function get_var() + real, pointer :: get_var(:) + get_var => z + end + function get_var_cont() + real, pointer, contiguous :: get_var_cont(:) + get_var_cont => z + end +end