[Bug fortran/27124] New: Incorrect dependency for assignment from function with array section actual arg.

[Bug fortran/27124] New: Incorrect dependency for assignment from function with array section actual arg.

[pault at gcc dot gnu dot org]

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;
      }
  }


-- 
           Summary: Incorrect dependency for assignment from function with
                    array section actual arg.
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: pault at gcc dot gnu dot org
        ReportedBy: pault at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2006-04-11 20:42 -------
Confirmed.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2006-04-11 20:42:42
               date|                            |
            Summary|Incorrect dependency for    |Incorrect dependency for
                   |assignment from function    |assignment from function
                   |with array section actual   |with array section actual
                   |arg.                        |arg.


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[paul dot richard dot thomas at cea dot fr]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]



------- 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

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[P dot Schaffnit at access dot rwth-aachen dot de]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3613 bytes --]



------- Comment #3 from P dot Schaffnit at access dot rwth-aachen dot de  2006-04-12 15:02 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[patchapp at dberlin dot org]

------- Comment #4 from patchapp at dberlin dot org  2006-04-12 19:30 -------
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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[pault at gcc dot gnu dot org]

------- Comment #5 from pault at gcc dot gnu dot org  2006-04-16 03:45 -------
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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[pault at gcc dot gnu dot org]

------- Comment #6 from pault at gcc dot gnu dot org  2006-04-23 05:33 -------
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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124

[Bug fortran/27124] Incorrect dependency for assignment from function with array section actual arg.

[pault at gcc dot gnu dot org]

------- Comment #7 from pault at gcc dot gnu dot org  2006-04-23 05:42 -------
Fixed on trunk and 4.1

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27124