Bug 43243 - [4.4 Regression ?] Wrong-code due to missing array temp for DT with pointer component
Summary: [4.4 Regression ?] Wrong-code due to missing array temp for DT with pointer c...
Status: RESOLVED INVALID
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: missed-optimization, wrong-code
Depends on:
Blocks: 42361
  Show dependency treegraph
 
Reported: 2010-03-03 08:51 UTC by Tobias Burnus
Modified: 2010-03-15 17:23 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.5 4.4.3
Known to fail: 4.5.0
Last reconfirmed:


Attachments
submit.diff (1.50 KB, patch)
2010-03-03 15:53 UTC, paul.richard.thomas@gmail.com
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2010-03-03 08:51:28 UTC
Follow up to PR 43173 - but this time for derived types.

In the following calls, if the ultimate component is a non-pointer and all previous part-refs have no vector subscripts, there is no need for packing the argument (i.e. creating a temporary).

However, if the ultimate component is a pointer, one needs to pack as the test case also shows.

Correctness test fails with
  4.5.0 20100218 (experimental) [trunk revision 156860]
  4.5.0 20100303 (experimental)
but works with
  4.4.2 [gcc-4_4-branch revision 155966]
  4.3.4 [gcc-4_3-branch revision 152973]



module m
  type t
    integer, allocatable :: a(:)
    integer, pointer :: b(:)
    integer :: c(5)
  end type t
end module m

subroutine foo(a,d,e,n)
  use m
  implicit none
  integer :: n
  type(t) :: a
  type(t), allocatable :: d(:)
  type(t), pointer :: e(:)
  call bar(   a%a) ! OK - no array temp needed
  call bar(   a%c) ! OK - no array temp needed

  call bar(   a%a(1:n)) ! Missed: No pack needed
  call bar(   a%b(1:n)) ! OK: pack needed
  call bar(   a%c(1:n)) ! Missed: No pack needed

  call bar(d(1)%a(1:n)) ! Missed: No pack needed
  call bar(d(1)%b(1:n)) ! OK: pack needed
  call bar(d(1)%c(1:n)) ! Missed: No pack needed

  call bar(e(1)%a(1:n)) ! Missed: No pack needed
  call bar(e(1)%b(1:n)) ! OK: pack needed
  call bar(e(1)%c(1:n)) ! Missed: No pack needed
end subroutine foo

use m
implicit none
integer :: i
integer, target :: z(6)
type(t) :: y

z = [(i, i=1,6)]
y%b => z(::2)
call bar(y%b)
end

subroutine bar(x)
  integer :: x(1:*)
  print *, x(1:3)
  if (any (x(1:3) /= [1,3,5])) call abort ()
end subroutine bar
Comment 1 paul.richard.thomas@gmail.com 2010-03-03 15:53:27 UTC
Subject: Re:  [4.5 Regression] Missing array temp for DT 
	with pointer component

Yet another in the series :-)   I hope that it is the last...

This bootstraps and regtests on RHEL5.2/x86_64 - OK for trunk?

Paul

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

	PR fortran/43243
	* trans-array.c (gfc_conv_array_parameter): Contiguous refs to
	allocatable ultimate components do not need temporaries, whilst
	ultimate pointer components do.

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

	PR fortran/43243
	* gfortran.dg/internal_pack_12.f90: New test.
Comment 2 paul.richard.thomas@gmail.com 2010-03-03 15:53:27 UTC
Created attachment 20010 [details]
submit.diff
Comment 3 Tobias Burnus 2010-03-03 15:53:56 UTC
As it might have not been completely clear from comment 0:

  type t
    [...]
    integer, pointer :: b(:)
  end type
  type(t) :: y
  y%b => z(::2)
  call bar(y%b) ! <<< WRONG-CODE: Here a temporary is missing

Without the temporary, [1,2,3] instead of [1,3,5] is passed to "bar" and thus the example fails with a call to ABORT. (The rest of example illustrates a missed optimization, which is not a regression.)
Comment 4 paul.richard.thomas@gmail.com 2010-03-03 16:15:45 UTC
Subject: Re:  [4.5 Regression] Wrong-code due to missing 
	array temp for DT with pointer component

Tobias,

Don't worry, I got it:-)

The patch fixes both problems.

Cheers

Paul

On Wed, Mar 3, 2010 at 4:53 PM, burnus at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #3 from burnus at gcc dot gnu dot org  2010-03-03 15:53 -------
> As it might have not been completely clear from comment 0:
>
>  type t
>    [...]
>    integer, pointer :: b(:)
>  end type
>  type(t) :: y
>  y%b => z(::2)
>  call bar(y%b) ! <<< WRONG-CODE: Here a temporary is missing
>
> Without the temporary, [1,2,3] instead of [1,3,5] is passed to "bar" and thus
> the example fails with a call to ABORT. (The rest of example illustrates a
> missed optimization, which is not a regression.)
>
>
> --
>
> burnus at gcc dot gnu dot org changed:
>
>           What    |Removed                     |Added
> ----------------------------------------------------------------------------
>      Known to fail|                            |4.5.0
>      Known to work|                            |4.3.5 4.4.3
>           Priority|P3                          |P4
>            Summary|[4.5 Regression] Missing    |[4.5 Regression] Wrong-code
>                   |array temp for DT with      |due to missing array temp
>                   |pointer component           |for DT with pointer
>                   |                            |component
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43243
>
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
>



Comment 5 burnus@net-b.de 2010-03-03 16:59:30 UTC
Subject: Re:  [4.5 Regression] Missing array temp for DT
  	with pointer component

On 03/03/2010 04:53 PM, Paul Richard Thomas wrote:
> Yet another in the series :-)   I hope that it is the last...
>   

So do I - especially with regards to regressions. Regarding temporaries:
I was compiling some programs with the patched gfortran and the number
of created temporaries is quite low - and I think no low-lying fruits
are left. There are some cases of assignments for which middle-end array
support were helpful (hint, hint) and some other cases with MATMUL and
assignments which could be solved in the front end. (And two of the
programs here could gain some performance, if they would move to TR
15581; without the compiler has unnecessarily to assume aliasing. But as
both do not dare to drop support for F95-only compilers, yet, ...)

By the way, for RESHAPE gfortran generates temporaries what could be
avoided if RESHAPE wouldn't deallocate - that's another candidate to fix
after the new array descriptor gained a "bool allocated" component ...
cf. PR 32512.

> This bootstraps and regtests on RHEL5.2/x86_64 - OK for trunk?
>   
OK. Thanks a lot for the patch!

Tobias

> 2010-03-03  Paul Thomas  <pault@gcc.gnu.org>
>
> 	PR fortran/43243
> 	* trans-array.c (gfc_conv_array_parameter): Contiguous refs to
> 	allocatable ultimate components do not need temporaries, whilst
> 	ultimate pointer components do.
>
> 2010-03-03  Paul Thomas  <pault@gcc.gnu.org>
>
> 	PR fortran/43243
> 	* gfortran.dg/internal_pack_12.f90: New test.
>   
Comment 6 Paul Thomas 2010-03-03 17:50:19 UTC
Subject: Bug 43243

Author: pault
Date: Wed Mar  3 17:49:53 2010
New Revision: 157199

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

	PR fortran/43243
	* trans-array.c (gfc_conv_array_parameter): Contiguous refs to
	allocatable ultimate components do not need temporaries, whilst
	ultimate pointer components do.

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

	PR fortran/43243
	* gfortran.dg/internal_pack_12.f90: New test.

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

Comment 7 Paul Thomas 2010-03-03 17:53:43 UTC
Fixed on trunk and.... yes, thanks for the report!

Cheers

Paul
Comment 8 H.J. Lu 2010-03-13 16:52:50 UTC
gfortran.dg/internal_pack_12.f90 failed with gcc 4.4 branch
as of 2010-03-13. Is this a regression on 4.4 branch?
Comment 9 paul.richard.thomas@gmail.com 2010-03-13 17:26:00 UTC
Subject: Re:  [4.5 Regression] Wrong-code due to missing 
	array temp for DT with pointer component

HJ,

Thanks for doing all this backporting of the testcases.

As it happens, I have been leaving 4.4 alone, since causing all manner
of problems a few weeks ago.

I cannot check 4.4 until tomorrow night; I only have trunk on my
laptop.  However, I will certainly take a look then.

Thanks again

Paul

On Sat, Mar 13, 2010 at 5:52 PM, hjl dot tools at gmail dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #8 from hjl dot tools at gmail dot com  2010-03-13 16:52 -------
> gfortran.dg/internal_pack_12.f90 failed with gcc 4.4 branch
> as of 2010-03-13. Is this a regression on 4.4 branch?
>
>
> --
>
> hjl dot tools at gmail dot com changed:
>
>           What    |Removed                     |Added
> ----------------------------------------------------------------------------
>             Status|RESOLVED                    |UNCONFIRMED
>         Resolution|FIXED                       |
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43243
>
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
>



Comment 10 Paul Thomas 2010-03-14 06:27:46 UTC
Will check this out as soon as I am back at base.

Paul
Comment 11 Paul Thomas 2010-03-15 17:23:33 UTC
(In reply to comment #10)
> Will check this out as soon as I am back at base.

HJ,

4.4 does not deal with the original problem and so still produces the unnecessary temporaries.

from the end of internal_pack12.f90...

! { dg-final { scan-tree-dump-times "unpack" 4 "original" } }
! { dg-final { cleanup-tree-dump "original" } }
! { dg-final { cleanup-modules "m" } }

[pault@localhost pr43243]# /irun4.4/bin/gfortran -static -fdump-tree-original /svn/trunk/gcc/testsuite/gfortran.dg/internal_pack_12.f90
[pault@localhost pr43243]# grep unpack *nal
        _gfortran_internal_unpack (&a->a, D.1552);
        _gfortran_internal_unpack (&parm.0, D.1556);
        _gfortran_internal_unpack (&parm.1, D.1561);
        _gfortran_internal_unpack (&parm.2, D.1566);
        _gfortran_internal_unpack (&parm.3, D.1570);
        _gfortran_internal_unpack (&parm.4, D.1575);
        _gfortran_internal_unpack (&parm.5, D.1580);
        _gfortran_internal_unpack (&parm.6, D.1584);
        _gfortran_internal_unpack (&parm.7, D.1589);
        _gfortran_internal_unpack (&parm.8, D.1594);
        _gfortran_internal_unpack (&parm.9, D.1598);
          _gfortran_internal_unpack (&y.b, D.1845);

That's why it's failing. I suggest strongly that the test not be backported to 4.4.

I am resolving this as INVALID although I am a bit unhappy about marking a sensible, precautionary PR in this way.

Thanks for the report nevertheless!

Paul