[Bug fortran/43173] New: Unnecessary array temporary: Passing contiguous array as actual argument

[Bug fortran/43173] New: Unnecessary array temporary: Passing contiguous array as actual argument

[burnus at gcc dot gnu dot org]

Related to some other missed-optimization PRs, but particularly simple.

  REAL, allocatable :: ot(:)
  integer :: time_steps

  call foo (ot) ! OK, no temporary
  call foo (ot(0:5:1)) ! Unnecessary temporary
  call foo (ot(0:time_steps)) ! Unnecessary temporary
  end

Note: ot is contiguous (as it is allocatable), stride = 1, and thus all
arguments are simple contiguous - unless the stride is non-unity or one has,
e.g., "ot( [1,3,5])".

Note 2: If one has an explicit-size array, it works - there is no temporary
for:
  REAL :: ot(5)
  integer :: time_steps
  call foo (ot(1:time_steps))
I think the issue is that one mixes up the (:) of an allocatable array with the
(:) of a non-allocatable, assumed-shape dummy argument or a pointer.

The temporary seems to be generated in trans-arrays.c's
gfc_conv_array_parameter; the issue seems to be that "full_array_var" is false
and then later there is either no_pack or contiguous set to false.


-- 
           Summary: Unnecessary array temporary: Passing contiguous array as
                    actual argument
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: burnus at gcc dot gnu dot org


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[pault at gcc dot gnu dot org]

------- Comment #1 from pault at gcc dot gnu dot org  2010-02-25 12:48 -------
This does not occur on i386.  I will try on a x86_64 tonight.

Paul


-- 


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[burnus at gcc dot gnu dot org]

------- Comment #2 from burnus at gcc dot gnu dot org  2010-02-25 12:56 -------
(In reply to comment #1)
> This does not occur on i386.  I will try on a x86_64 tonight.

The warning for -Warray-temporaries is shown here for both -m32 and -m64 -- and
I also see "_gfortran_internal_pack" in the -fdump-tree-original. Do you have
by chance local changes which fix this issue?


-- 


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[pault at gcc dot gnu dot org]

------- Comment #3 from pault at gcc dot gnu dot org  2010-02-26 05:28 -------

> The warning for -Warray-temporaries is shown here for both -m32 and -m64 -- and
> I also see "_gfortran_internal_pack" in the -fdump-tree-original. Do you have
> by chance local changes which fix this issue?

      CALL foo ((MAIN__:ot(FULL)))
      CALL foo ((MAIN__:ot(0_8:5_8:1_8)))
      CALL foo ((MAIN__:ot(0_8:__convert_i4_i8[[((MAIN__:time_steps))]])))

I was distracted by the convert_i4_i8.  Not only does it not appear for the
second line but it does not appear at all with -fdefault-integer-8.

I will go back and confirm that the tree on my machine at work is clean.

Confirmed

Cheers

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-02-26 05:28:34
               date|                            |


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[pault at gcc dot gnu dot org]

------- Comment #4 from pault at gcc dot gnu dot org  2010-02-26 16:06 -------
(In reply to comment #3)

> > by chance local changes which fix this issue?

> I will go back and confirm that the tree on my machine at work is clean.

No, it wasn't, so my comment was incorrect.

Cheers

Paul


-- 


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

[Bug fortran/43173] [4.5 Regression] Unnecessary array temporary: Passing contiguous array as actual argument

[tkoenig at gcc dot gnu dot org]

------- Comment #5 from tkoenig at gcc dot gnu dot org  2010-02-27 11:35 -------
I do not see the temporaries with

gcc-Version 4.5.0 20100214 (experimental) (GCC)

but I see this with

gcc-Version 4.5.0 20100227 (experimental) (GCC)

on x86_64-unknown-linux-gnu

This makes this a regression.


-- 

tkoenig at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tkoenig at gcc dot gnu dot
                   |                            |org
            Summary|Unnecessary array temporary:|[4.5 Regression] Unnecessary
                   |Passing contiguous array as |array temporary: Passing
                   |actual argument             |contiguous array as actual
                   |                            |argument
   Target Milestone|---                         |4.5.0


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2010-03-01 11:54 -------
Not a regression (we track regressions only against releases).  I see both
temporaries also on the 4.4 branch.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.4.3
            Summary|[4.5 Regression] Unnecessary|Unnecessary array temporary:
                   |array temporary: Passing    |Passing contiguous array as
                   |contiguous array as actual  |actual argument
                   |argument                    |
   Target Milestone|4.5.0                       |---


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[burnus at gcc dot gnu dot org]

------- Comment #7 from burnus at gcc dot gnu dot org  2010-03-02 09:16 -------
> I do not see the temporaries with [...]  4.5.0 20100214
> but I see this with [...]                4.5.0 20100227

I think the "regression" is due to:
http://gcc.gnu.org/viewcvs?view=revision&revision=156926

Namely due to the added "no_pack" logic in trans-arrays.c's
gfc_conv_array_parameter (as suggested in comment 0).
http://gcc.gnu.org/viewcvs/trunk/gcc/fortran/trans-array.c?r1=156926&r2=156925

That committal fixed PR 36932, PR 36933, PR 43072, and PR 43111.


-- 


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[burnus at gcc dot gnu dot org]

------- Comment #8 from burnus at gcc dot gnu dot org  2010-03-02 09:24 -------
Completely untested patch:

--- trans-array.c       (revision 157160)
+++ trans-array.c       (working copy)
@@ -5555,5 +5555,5 @@ gfc_conv_array_parameter (gfc_se * se, g
   no_pack = ((sym && sym->as
                  && !sym->attr.pointer
-                 && sym->as->type != AS_DEFERRED
+                 && (sym->as->type != AS_DEFERRED || sym->attr.allocatable)
                  && sym->as->type != AS_ASSUMED_SHAPE)
                      ||


-- 


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[burnus at gcc dot gnu dot org]

------- Comment #9 from burnus at gcc dot gnu dot org  2010-03-02 10:01 -------
(In reply to comment #8)
> Completely untested patch:

Still untested, but I think it might lead to wrong code with either
  allocatable(1)%pointer(1:2)  ! Potentially non-contiguous
or
  pointer%allocatable(1:2) ! Cannot alias & is contiguous
Though I have not really studied the source code.

Actually, also for other parts in this function I am unsure whether it really
takes derived-type components into account.


-- 


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[pault at gcc dot gnu dot org]

------- Comment #10 from pault at gcc dot gnu dot org  2010-03-02 11:58 -------
Subject: Bug 43173

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


-- 


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

[Bug fortran/43173] Unnecessary array temporary: Passing contiguous array as actual argument

[pault at gcc dot gnu dot org]

------- Comment #11 from pault at gcc dot gnu dot org  2010-03-02 12:10 -------
Fixed on trunk, thanks for the report!

Paul


-- 

pault at gcc dot gnu dot org changed:

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


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