Bug 43180 - [4.5 Regression] Bad results without temporary copy of intent(in) argument
Summary: [4.5 Regression] Bad results without temporary copy of intent(in) argument
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P4 normal
Target Milestone: 4.5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2010-02-25 19:16 UTC by Harald Anlauf
Modified: 2010-03-02 12:11 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-02-25 23:38:17


Attachments
Testcase (3.12 KB, application/octet-stream)
2010-02-25 19:18 UTC, Harald Anlauf
Details
Stand-alone test (938 bytes, text/plain)
2010-02-25 22:26 UTC, Harald Anlauf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Anlauf 2010-02-25 19:16:28 UTC
Hi,

my large code suddenly fails with the current gfortran, while it still works
with gfortran from fortran-dev (version 20100127).  I traced the problem
to the following part of a module where the invocation of a subroutine
as

   call set_set_v (ru(i)% c, c)  ! (notemp) Fails with current gfortran

fails (i.e., the resulting c contains bad values), while

   tmp_c = ru(i)% c              ! ("temp") Works
   call set_set_v (tmp_c, c)

works.  For details see the attached code.

I have compared the output of -fdump-tree-original of the fortran-dev
and of the trunk versions of gfortran and found that they are identical
for the "temp" variant, while there is a significant different for the
notemp variant.  Maybe someone knowledgeable can identify the cause of
the problem.
Comment 1 Harald Anlauf 2010-02-25 19:18:33 UTC
Created attachment 19958 [details]
Testcase

Testcase (module) and output of -fdump-tree-original for gfortran
from fortran-dev and trunk (4.5).
Comment 2 Steven Bosscher 2010-02-25 19:51:38 UTC
Paul, looks like one of yours.
Comment 3 Joost VandeVondele 2010-02-25 20:45:09 UTC
(In reply to comment #1)
> Created an attachment (id=19958) [edit]
> Testcase

I'm wondering if you have a testcase that is 'fully functional' i.e. that one can run & debug. My first guess is that a temporary there is not needed, unless there is an aliasing problem.

Comment 4 Harald Anlauf 2010-02-25 20:59:35 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > Created an attachment (id=19958) [edit]
> > Testcase
> 
> I'm wondering if you have a testcase that is 'fully functional' i.e. that one
> can run & debug.

It will take some time as I need to dump a snapshot of the table
of rules and an excerpt of the input data that are processed.
Will do, but don't hold your breath.

> My first guess is that a temporary there is not needed, unless
> there is an aliasing problem.

There is no real need for a temporary copy of ru(i)%c, but as I wrote
without the copy I get wrong results.  Maybe it's as simple as an
improper offset and the like.
Comment 5 Paul Thomas 2010-02-25 21:42:55 UTC
I cannot persuade trunk to produce different output to 4.4.

I am not saying that you are wrong but I can only reiterate Joost's request for a testcase in which the result changes for the lack of a temporary.

Paul
Comment 6 Harald Anlauf 2010-02-25 22:12:30 UTC
(In reply to comment #5)
> I cannot persuade trunk to produce different output to 4.4.
> 
> I am not saying that you are wrong but I can only reiterate Joost's request for
> a testcase in which the result changes for the lack of a temporary.

Printing the argument src in the contained subroutine set_set_v
shows that it contains junk on ia32 if there is no temporary.

I'm working on a self-contained test case.
Comment 7 Dominique d'Humieres 2010-02-25 22:16:24 UTC
Comparing the original dumps for revisions 156618 and 157068, I see:

<                     parm.19.dim[0].ubound = 50;
<                     parm.19.dim[0].stride = 1;
<                     parm.19.data = (void *) &ru[(integer(kind=8)) i + -1].c[0];
<                     parm.19.offset = -1;
<                     D.1619 = _gfortran_internal_pack (&parm.19);
...
>                     parm.19.dim[0].ubound = D.1620;
>                     parm.19.dim[0].stride = NON_LVALUE_EXPR <D.1625>;
>                     parm.19.data = (void *) &(*c.0)[0];
>                     parm.19.offset = NON_LVALUE_EXPR <D.1624>;

Are the NON_LVALUE_EXPR expected?
Comment 8 Harald Anlauf 2010-02-25 22:26:56 UTC
Created attachment 19961 [details]
Stand-alone test

I get:

 get_rule: ru(i)% c(1) =
 -1000000000 -1000000000 -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000 -127
 Using temporary:
 set_set_v: src(1) =
 -1000000000 -1000000000 -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000 -127
 No temporary:
 set_set_v: src(1) =
   538976288   538976288  6.01347001699906849E-154  6.01347001699906849E-154  6.01347001699906849E-154  6.01347001699906849E-154 -3.13018361251940323E+021 -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000      -1000000000.00000000                0  101

No problem with older gfortrans.
Comment 9 Tobias Burnus 2010-02-25 23:38:16 UTC
Reduced test case below. The problem is the call. On the trunk the call looks as follows:
  set_set_v (&ru, D.1578);
which is complete nonesense. It should be:
  &ru.data[0].c.use
or something like that. If one removes the "comment" component, it works - but only because then "&ru" and "&ru.data[0].c.use" point to the same memory - otherwise it is as wrong as before.

! ------------------------
module mo_obs_rules
  type t_set
     integer :: use = 42
  end type t_set
  type t_rules
     character(len=40) :: comment
     type(t_set)       :: c (1)
  end type t_rules
  type (t_rules), save :: ru (1)
contains
  subroutine get_rule (c)
    type(t_set) :: c (:)
!    print *, ru(1)%c(1)%use ! 42 -> OK
!    print *,       c(1)%use ! 42 -> OK
    call set_set_v (ru(1)%c, c)
  contains
    subroutine set_set_v (src, dst)
      type(t_set), intent(in)    :: src(1)
      type(t_set), intent(inout) :: dst(1)
!      print *, src(1)%use ! garbage (should be 42)
!      print *, dst(1)%use ! 42 -> OK
    end subroutine set_set_v
  end subroutine get_rule
end module mo_obs_rules

program test
  use mo_obs_rules
  type(t_set) :: c (1)
  call get_rule (c)
end program test
Comment 10 Paul Thomas 2010-02-28 13:46:34 UTC
(In reply to comment #9)
> Reduced test case below. The problem is the call. On the trunk the call looks
> as follows:
>   set_set_v (&ru, D.1578);
> which is complete nonesense. It should be:
>   &ru.data[0].c.use
> or something like that. If one removes the "comment" component, it works - but
> only because then "&ru" and "&ru.data[0].c.use" point to the same memory -
> otherwise it is as wrong as before.

Yes, indeed - that is the problem.  Since the need for a temporary has been removed, the code now runs straight into line trans-array.c:5515.  The condition !sym->as causes tmp to be used as the actual, rather than calling gfc_conv_expr_descriptor.

Just as soon as todayś clean build stops, I will remove the condition and will try a regtest.

Paul
Comment 11 Paul Thomas 2010-03-02 11:58:38 UTC
Subject: Bug 43180

Author: pault
Date: Tue Mar  2 11:58:02 2010
New Revision: 157163

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157163
Log:
2010-03-02  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/43180
	* trans-array.c (gfc_conv_array_parameter): A full array of
	derived type need not be restricted to a symbol without an
	array spec to use the call to gfc_conv_expr_descriptor.

	PR fortran/43173
	* trans-array.c (gfc_conv_array_parameter): Contiguous refs to
	allocatable arrays do not need temporaries.

2010-03-02  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/43180
	* gfortran.dg/internal_pack_10.f90: New test.

	PR fortran/43173
	* gfortran.dg/internal_pack_11.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/internal_pack_10.f90
    trunk/gcc/testsuite/gfortran.dg/internal_pack_11.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/testsuite/ChangeLog

Comment 12 Paul Thomas 2010-03-02 12:11:20 UTC
Fixed on trunk, thanks for the report!

Paul
Comment 13 hjl@gcc.gnu.org 2010-03-13 16:59:32 UTC
Subject: Bug 43180

Author: hjl
Date: Sat Mar 13 16:58:19 2010
New Revision: 157426

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157426
Log:
Backport testcases from mainline.

2010-03-13  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline:
	2010-03-11  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/43257
	* g++.dg/torture/pr43257.C: New test.

	2010-03-11  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43255
	* gcc.c-torture/compile/pr43255.c: New testcase.

	2010-03-11  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>

	* gcc.dg/pr43280.c: New testcase.

	2010-03-10  Jan Hubicka   <jh@suse.cz>

	* gcc.c-torture/compile/pr43288.c: New test.

	2010-03-10  Andrey Belevantsev  <abel@ispras.ru>

	PR middle-end/42859
	* g++.dg/eh/pr42859.C: New test.

	2010-03-09  Jakub Jelinek  <jakub@redhat.com>

	PR debug/43299
	* gcc.dg/pr43299.c: New test.

	2010-03-08  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43269
	* gcc.c-torture/execute/pr43269.c: New testcase.

	2010-03-04  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/43164
	PR tree-optimization/43191
	* gcc.c-torture/compile/pr43164.c: New test.
	* gcc.c-torture/compile/pr43191.c: Likewise.

	2010-03-04  Changpeng Fang  <changpeng.fang@amd.com>

	PR middle-end/43209
	* gcc.dg/tree-ssa/ivopts-4.c: New.

	2010-03-03  Jakub Jelinek  <jakub@redhat.com>

	PR debug/43229
	* gfortran.dg/pr43229.f90: New test.

	PR debug/43237
	* gcc.dg/debug/dwarf2/pr43237.c: New test.

	2010-03-02  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/43180
	* gfortran.dg/internal_pack_10.f90: New test.

	2010-02-26  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43188
	* gcc.c-torture/compile/pr43188.c: New testcase.

	2010-02-25  Jakub Jelinek  <jakub@redhat.com>

	PR debug/43166
	* gfortran.dg/debug/pr43166.f: New test.

	PR debug/43165
	* gcc.dg/torture/pr43165.c: New test.

	2010-02-23  Jakub Jelinek  <jakub@redhat.com>

	PR target/43107
	* gcc.target/i386/pr43107.c: New test.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/eh/pr42859.C
      - copied unchanged from r157425, trunk/gcc/testsuite/g++.dg/eh/pr42859.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr43257.C
      - copied unchanged from r157425, trunk/gcc/testsuite/g++.dg/torture/pr43257.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43164.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.c-torture/compile/pr43164.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43188.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.c-torture/compile/pr43188.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43191.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.c-torture/compile/pr43191.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43255.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.c-torture/compile/pr43255.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43288.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.c-torture/compile/pr43288.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr43269.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.c-torture/execute/pr43269.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/debug/dwarf2/pr43237.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr43237.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr43280.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.dg/pr43280.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr43299.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.dg/pr43299.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/torture/pr43165.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.dg/torture/pr43165.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/tree-ssa/ivopts-4.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.dg/tree-ssa/ivopts-4.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.target/i386/pr43107.c
      - copied unchanged from r157425, trunk/gcc/testsuite/gcc.target/i386/pr43107.c
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/debug/pr43166.f
      - copied unchanged from r157425, trunk/gcc/testsuite/gfortran.dg/debug/pr43166.f
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/internal_pack_10.f90
      - copied unchanged from r157425, trunk/gcc/testsuite/gfortran.dg/internal_pack_10.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr43229.f90
      - copied unchanged from r157425, trunk/gcc/testsuite/gfortran.dg/pr43229.f90
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog