This PR was posted to the list by Philippe Schaffnit http://gcc.gnu.org/ml/fortran/2006-04/msg00131.html The testcase provided reduces to: PROGRAM Test INTEGER :: Array(2, 3) = & reshape ((/1,4,2,5,3,6/),(/2,3/)), Brray(2, 3) = 0 Array(1,:) = Function_Test (Array(1,:)) print *, Array(1:1,:) print *, Function_test (Array(1,:) ) Brray(1,:) = Function_Test (Array(1,:)) print *, Brray(1:1,:) contains FUNCTION Function_Test (Input) USE Module_Test INTEGER, INTENT(IN) :: Input(1:3) INTEGER :: Function_Test(1:3) Function_Test = Input + 10 END FUNCTION Function_Test END PROGRAM Test which outputs 1 2 3 11 12 13 11 12 13 What is happening is all too apparent from the code for the first assignment from the function, which I have appended below. Being an array section, the actual argument is sent to be packed. At the same time, the assignment detects that the dependency between the lhs and the actual argument and so makes a temporary for the result. After the function call, the result is transferred from the temporary and the packed array section is unpacked on top of it. This latter ensures that the original overwrites the result because the actual argument is left untouched. The other two uses of the function give the correct result because there is no dependency. This can be fixed, I think, by suppressing the dependency temporary, on detecting that the array will be packed. Paul { int4 A.7[3]; struct array1_int4 atmp.6; int4[3] * ifm.5; void * D.928; void * D.927; struct array1_int4 parm.4; parm.4.dtype = 265; parm.4.dim[0].lbound = 1; parm.4.dim[0].ubound = 3; parm.4.dim[0].stride = 2; parm.4.data = (void *) (int4[0:] *) &array[0]; parm.4.offset = 0; D.927 = _gfortran_internal_pack (&parm.4); D.928 = D.927; ifm.5 = (int4[3] *) D.928; atmp.6.dtype = 265; atmp.6.dim[0].stride = 1; atmp.6.dim[0].lbound = 0; atmp.6.dim[0].ubound = 2; atmp.6.data = (void *) &A.7; atmp.6.offset = 0; atmp.6.dim[0].stride = 0; function_test (&atmp.6, D.928); { int4 S.8; S.8 = 0; while (1) { if (S.8 > 2) goto L.2; else (void) 0; array[(NON_LVALUE_EXPR <S.8> + 1) * 2 + -2] = (*(int4[0:] *) atmp.6.data)[NON_LVALUE_EXPR <S.8>]; S.8 = S.8 + 1; } L.2:; } if (D.927 != (int4[0:] *) parm.4.data) { _gfortran_internal_unpack (&parm.4, D.927); _gfortran_internal_free (D.927); } else { (void) 0; } }
Confirmed.
I have a fix for it, which is regtesting right now. Even if it fails in this form, it is along the right lines and there will be a version that is pukkah. I hope to submit the patch tonight. Quite simply, the fix consists of gathering up all the argument post_blocks, which contain the unpacking and freeing of argument temporaries, and putting them into a separate block. Once the function call is translated, it either goes in the se->pre block or becomes the se expression, depending on how the value is returned. Depending on this same choice, we now add the argument post block to se->pre or to se->post. This ensures that the results of byref calls that produce temporaries are transferred to the destination array AFTER the unpacking of the argument. The reduced testcase below now runs correctly. The patch and testcase appear below. Paul Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (révision 112853) +++ gcc/fortran/trans-expr.c (copie de travail) @@ -1832,6 +1832,7 @@ gfc_charlen cl; gfc_expr *e; gfc_symbol *fsym; + stmtblock_t post; arglist = NULL_TREE; retargs = NULL_TREE; @@ -1861,6 +1862,7 @@ else info = NULL; + gfc_init_block (&post); gfc_init_interface_mapping (&mapping); need_interface_mapping = ((sym->ts.type == BT_CHARACTER && sym->ts.cl->length @@ -1970,7 +1972,7 @@ gfc_add_interface_mapping (&mapping, fsym, &parmse); gfc_add_block_to_block (&se->pre, &parmse.pre); - gfc_add_block_to_block (&se->post, &parmse.post); + gfc_add_block_to_block (&post, &parmse.post); /* Character strings are passed as two parameters, a length and a pointer. */ @@ -2177,6 +2179,11 @@ } } + if (byref) + gfc_add_block_to_block (&se->pre, &post); + else + gfc_add_block_to_block (&se->post, &post); + return has_alternate_specifier; } ! { dg-do run } ! Tests the fix for PR27124 in which the unpacking of argument ! temporaries and of array result temporaries occurred in the ! incorrect order. ! ! Test is based on the original example, provided by ! Philippe Schaffnit <P.Schaffnit@access.rwth-aachen.de> ! PROGRAM Test INTEGER :: Array(2, 3) = reshape ((/1,4,2,5,3,6/),(/2,3/)) integer :: Brray(2, 3) = 0 Brray(1,:) = Function_Test (Array(1,:)) if (any(reshape (Brray, (/6/)) .ne. (/11, 0, 12, 0, 13, 0/))) call abort () Array(1,:) = Function_Test (Array(1,:)) if (any(reshape (Array, (/6/)) .ne. (/11, 4, 12, 5, 13, 6/))) call abort () contains FUNCTION Function_Test (Input) INTEGER, INTENT(IN) :: Input(1:3) INTEGER :: Function_Test(1:3) Function_Test = Input + 10 END FUNCTION Function_Test END PROGRAM Test
Subject: Re: Incorrect dependency for assignment from functionwith array section actual arg. Merci! Philippe paul dot richard dot thomas at cea dot fr wrote: > > ------- Comment #2 from paul dot richard dot thomas at cea dot fr 2006-04-12 14:34 ------- > I have a fix for it, which is regtesting right now. Even if it fails in this > form, it is along the right lines and there will be a version that is pukkah. I > hope to submit the patch tonight. > > Quite simply, the fix consists of gathering up all the argument post_blocks, > which contain the unpacking and freeing of argument temporaries, and putting > them into a separate block. Once the function call is translated, it either > goes in the se->pre block or becomes the se expression, depending on how the > value is returned. Depending on this same choice, we now add the argument post > block to se->pre or to se->post. This ensures that the results of byref calls > that produce temporaries are transferred to the destination array AFTER the > unpacking of the argument. The reduced testcase below now runs correctly. > > The patch and testcase appear below. > > Paul > > Index: gcc/fortran/trans-expr.c > =================================================================== > --- gcc/fortran/trans-expr.c (révision 112853) > +++ gcc/fortran/trans-expr.c (copie de travail) > @@ -1832,6 +1832,7 @@ > gfc_charlen cl; > gfc_expr *e; > gfc_symbol *fsym; > + stmtblock_t post; > > arglist = NULL_TREE; > retargs = NULL_TREE; > @@ -1861,6 +1862,7 @@ > else > info = NULL; > > + gfc_init_block (&post); > gfc_init_interface_mapping (&mapping); > need_interface_mapping = ((sym->ts.type == BT_CHARACTER > && sym->ts.cl->length > @@ -1970,7 +1972,7 @@ > gfc_add_interface_mapping (&mapping, fsym, &parmse); > > gfc_add_block_to_block (&se->pre, &parmse.pre); > - gfc_add_block_to_block (&se->post, &parmse.post); > + gfc_add_block_to_block (&post, &parmse.post); > > /* Character strings are passed as two parameters, a length and a > pointer. */ > @@ -2177,6 +2179,11 @@ > } > } > > + if (byref) > + gfc_add_block_to_block (&se->pre, &post); > + else > + gfc_add_block_to_block (&se->post, &post); > + > return has_alternate_specifier; > } > > ! { dg-do run } > ! Tests the fix for PR27124 in which the unpacking of argument > ! temporaries and of array result temporaries occurred in the > ! incorrect order. > ! > ! Test is based on the original example, provided by > ! Philippe Schaffnit <P.Schaffnit@access.rwth-aachen.de> > ! > PROGRAM Test > INTEGER :: Array(2, 3) = reshape ((/1,4,2,5,3,6/),(/2,3/)) > integer :: Brray(2, 3) = 0 > Brray(1,:) = Function_Test (Array(1,:)) > if (any(reshape (Brray, (/6/)) .ne. (/11, 0, 12, 0, 13, 0/))) call abort () > Array(1,:) = Function_Test (Array(1,:)) > if (any(reshape (Array, (/6/)) .ne. (/11, 4, 12, 5, 13, 6/))) call abort () > > contains > FUNCTION Function_Test (Input) > INTEGER, INTENT(IN) :: Input(1:3) > INTEGER :: Function_Test(1:3) > Function_Test = Input + 10 > END FUNCTION Function_Test > END PROGRAM Test > > -- > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124 > > ------- You are receiving this mail because: ------- > You are on the CC list for the bug, or are watching someone who is.
Subject: Bug number PR27124 A patch for this bug has been added to the patch tracker. The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00468.html
Subject: Bug 27124 Author: pault Date: Sun Apr 16 03:45:24 2006 New Revision: 112981 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112981 Log: 2006-04-16 Paul Thomas <pault@gcc.gnu.org> PR fortran/26822 * intrinsic.c (add_functions): Mark LOGICAL as elemental. PR fortran/26787 * expr.c (gfc_check_assign): Extend scope of error to include assignments to a procedure in the main program or, from a module or internal procedure that is not that represented by the lhs symbol. Use VARIABLE rather than l-value in message. PR fortran/27096 * trans-array.c (gfc_trans_deferred_array): If the backend_decl is not a descriptor, dereference and then test and use the type. PR fortran/25597 * trans-decl.c (gfc_trans_deferred_vars): Check if an array result, is also automatic character length. If so, process the character length. PR fortran/18803 PR fortran/25669 PR fortran/26834 * trans_intrinsic.c (gfc_walk_intrinsic_bound): Set data.info.dimen for bound intrinsics. * trans_array.c (gfc_conv_ss_startstride): Pick out LBOUND and UBOUND intrinsics and supply their shape information to the ss and the loop. PR fortran/27124 * trans_expr.c (gfc_trans_function_call): Add a new block, post, in to which all the argument post blocks are put. Add this block to se->pre after a byref call or to se->post, otherwise. 2006-04-16 Paul Thomas <pault@gcc.gnu.org> PR fortran/26787 * gfortran.dg/proc_assign_1.f90: New test. * gfortran.dg/procedure_lvalue.f90: Change message. * gfortran.dg/namelist_4.f90: Add new error. PR fortran/27096 * gfortran.dg/auto_pointer_array_result_1.f90 PR fortran/27089 * gfortran.dg/specification_type_resolution_1.f90 PR fortran/18803 PR fortran/25669 PR fortran/26834 * gfortran.dg/bounds_temporaries_1.f90: New test. PR fortran/27124 * gfortran.dg/array_return_value_1.f90: New test. Added: trunk/gcc/testsuite/gfortran.dg/array_return_value_1.f90 trunk/gcc/testsuite/gfortran.dg/auto_char_pointer_array_result_1.f90 trunk/gcc/testsuite/gfortran.dg/auto_pointer_array_result_1.f90 trunk/gcc/testsuite/gfortran.dg/bounds_temporaries_1.f90 trunk/gcc/testsuite/gfortran.dg/proc_assign_1.f90 trunk/gcc/testsuite/gfortran.dg/specification_type_resolution_1.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/expr.c trunk/gcc/fortran/intrinsic.c trunk/gcc/fortran/resolve.c trunk/gcc/fortran/trans-array.c trunk/gcc/fortran/trans-decl.c trunk/gcc/fortran/trans-expr.c trunk/gcc/fortran/trans-intrinsic.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gfortran.dg/namelist_4.f90 trunk/gcc/testsuite/gfortran.dg/procedure_lvalue.f90
Subject: Bug 27124 Author: pault Date: Sun Apr 23 05:33:16 2006 New Revision: 113191 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113191 Log: 2006-04-23 Paul Thomas <pault@gcc.gnu.org> PR fortran/27122 * resolve.c (resolve_function): Remove general restriction on auto character length function interfaces. (gfc_resolve_uops): Check restrictions on defined operator procedures. (resolve_types): Call the check for defined operators. PR fortran/27113 * trans-array.c (get_array_ctor_var_strlen): Remove typo in enum. Part of the fix in 4.2, which does not work in 4.1 because the divergence is now too great. PR fortran/26822 * intrinsic.c (add_functions): Mark LOGICAL as elemental. PR fortran/26787 * expr.c (gfc_check_assign): Extend scope of error to include assignments to a procedure in the main program or, from a module or internal procedure that is not that represented by the lhs symbol. Use VARIABLE rather than l-value in message. PR fortran/25597 * trans-decl.c (gfc_trans_deferred_vars): Check if an array result, is also automatic character length. If so, process the character length. Note that this fixes the bug in 4.2 but not here in 4.1 because the trees have diverged too much. Manifestly correct, so applied anyway. PR fortran/18803 PR fortran/25669 PR fortran/26834 * trans_intrinsic.c (gfc_walk_intrinsic_bound): Set data.info.dimen for bound intrinsics. * trans_array.c (gfc_conv_ss_startstride): Pick out LBOUND and UBOUND intrinsics and supply their shape information to the ss and the loop. PR fortran/27124 * trans_expr.c (gfc_trans_function_call): Add a new block, post, in to which all the argument post blocks are put. Add this block to se->pre after a byref call or to se->post, otherwise. 2006-04-23 Paul Thomas <pault@gcc.gnu.org> PR fortran/27122 * gfortran.dg/defined_operators_1.f90: New test. * gfortran.dg/assumed_charlen_function_1.f90: Add new error and remove old ones associated, incorrectly, with Note 5.46. PR fortran/26787 * gfortran.dg/proc_assign_1.f90: New test. * gfortran.dg/procedure_lvalue.f90: Change message. * gfortran.dg/namelist_4.f90: Add new error. PR fortran/27089 * gfortran.dg/specification_type_resolution_1.f90 PR fortran/18803 PR fortran/25669 PR fortran/26834 * gfortran.dg/bounds_temporaries_1.f90: New test. PR fortran/27124 * gfortran.dg/array_return_value_1.f90: New test. Added: branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/array_return_value_1.f90 branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/bounds_temporaries_1.f90 branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/defined_operators_1.f90 branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/proc_assign_1.f90 branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/specification_type_resolution_1.f90 Modified: branches/gcc-4_1-branch/gcc/fortran/ChangeLog branches/gcc-4_1-branch/gcc/fortran/expr.c branches/gcc-4_1-branch/gcc/fortran/intrinsic.c branches/gcc-4_1-branch/gcc/fortran/resolve.c branches/gcc-4_1-branch/gcc/fortran/trans-array.c branches/gcc-4_1-branch/gcc/fortran/trans-decl.c branches/gcc-4_1-branch/gcc/fortran/trans-expr.c branches/gcc-4_1-branch/gcc/fortran/trans-intrinsic.c branches/gcc-4_1-branch/gcc/testsuite/ChangeLog branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/assumed_charlen_function_1.f90 branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/namelist_4.f90 branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/procedure_lvalue.f90
Fixed on trunk and 4.1 Paul