[Bug c++/23372] New: Temporary aggregate copy not elided when passing parameters by value

[Bug c++/23372] New: Temporary aggregate copy not elided when passing parameters by value

[guillaume dot melquiond at ens-lyon dot fr]

This bug is similar to bug 16405, but although 16405 was fixed in 4.0, this one
is still present and is a regression from GCC 3.4 (not from 3.3 as was the
previous one). So I prefer opening a new bug-report.

The testcase simply calls a function f by passing the parameter by value:

struct A {
  int a[1000];
  //A(A const &);
};
void f(A);
void g(A *a) { f(*a); }

When compiled with gcc 3.3 and 3.4, the generated code for g is optimal: the
value *a is directly copied in the stack frame that will be used by f. With gcc
4.0, there is first a temporary copy in the stack frame of g, before copying the
value in the stack frame of f (two memcpys instead of one).

When putting a dummy copy constructor, both memcpys disappear: the code is
optimal. So the problem seems to be with the default trivial copy constructor.

The testcase is compiled with "g++-4.0 -O3", Debian package:

Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v
--enable-languages=c,c++,java,f95,objc,ada,treelang --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib --enable-nls
--without-included-gettext --enable-threads=posix --program-suffix=-4.0
--enable-__cxa_atexit --enable-libstdcxx-allocator=mt --enable-clocale=gnu
--enable-libstdcxx-debug --enable-java-gc=boehm --enable-java-awt=gtk
--enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-4.0-1.4.2.0/jre
--enable-mpfr --disable-werror --enable-checking=release i486-linux-gnu
Thread model: posix
gcc version 4.0.2 20050806 (prerelease) (Debian 4.0.1-4)

-- 
           Summary: Temporary aggregate copy not elided when passing
                    parameters by value
           Product: gcc
           Version: 4.0.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: guillaume dot melquiond at ens-lyon dot fr
                CC: gcc-bugs at gcc dot gnu dot org
  GCC host triplet: i486-linux-gnu


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

[Bug c++/23372] Temporary aggregate copy not elided when passing parameters by value

[rguenth at tat dot physik dot uni-tuebingen dot de]

------- Additional Comments From rguenth at tat dot physik dot uni-tuebingen dot de  2005-08-13 14:17 -------
The problem is, we end up with

void g(A*) (a)
{
  struct A D.1608;

<bb 0>:
  D.1608 = *a;
  f (D.1608) [tail call];
  return;

}

after the tree optimizers.  f (*a) would not be gimple, so we create
the temporary in the first place.  TER does not remove this wart,
neither does expand - so we start with two memcpys after RTL expansion.

This is definitively different from PR16405.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org


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

[Bug c++/23372] Temporary aggregate copy not elided when passing parameters by value

[fang at csl dot cornell dot edu]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fang at csl dot cornell dot
                   |                            |edu


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-08-13 17:56 -------
Confirmed.

(In reply to comment #1)
> after the tree optimizers.  f (*a) would not be gimple, so we create
> the temporary in the first place.  TER does not remove this wart,
> neither does expand - so we start with two memcpys after RTL expansion.
TER only works on scalars so it cannot work.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
           Keywords|                            |missed-optimization
      Known to fail|                            |4.0.0 4.1.0
      Known to work|                            |3.4.0 3.3.3 3.2.3 3.0.4
                   |                            |2.95.3
   Last reconfirmed|0000-00-00 00:00:00         |2005-08-13 17:56:39
               date|                            |
            Summary|Temporary aggregate copy not|[4.0/4.1 Regression]
                   |elided when passing         |Temporary aggregate copy not
                   |parameters by value         |elided when passing
                   |                            |parameters by value
   Target Milestone|---                         |4.0.2


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2005-08-13 18:00 -------
Why doesn't this happen with the copy constructor, then? there we should be 
calling the copyctor with *a, which would have the same problem.

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[rguenth at tat dot physik dot uni-tuebingen dot de]

------- Additional Comments From rguenth at tat dot physik dot uni-tuebingen dot de  2005-08-13 18:11 -------
With the copy ctor we end up with

void g(A*) (a)
{
  struct A D.1603;

<bb 0>:
  __comp_ctor  (&D.1603, a);
  f (&D.1603);
  return;

}

which confuses me a bit, because here the prototype of f looks like
effectively

void f(A*);

do we use ABI information here, but not in the other case?  The C++
frontend in this case presents us with

{
  <<cleanup_point <<< Unknown tree: expr_stmt
  f (&TARGET_EXPR <D.1603, <<< Unknown tree: aggr_init_expr
  __comp_ctor
  0B, (struct A &) (struct A *) NON_LVALUE_EXPR <a>
  D.1603 >>>
>) >>>
>>;
}

where in the case w/o the copy ctor we have

  <<cleanup_point <<< Unknown tree: expr_stmt
  f (TARGET_EXPR <D.1608, *(struct A &) (struct A *) NON_LVALUE_EXPR <a>>) >>>
>>;

is there some different wording about by-value parameter passing
with or without explicit copy ctor in the C++ standard?!  I.e., why
isn't the above

  <<cleanup_point <<< Unknown tree: expr_stmt
  f (&TARGET_EXPR <D.1608, *(struct A &) (struct A *) NON_LVALUE_EXPR <a>>) >>>
>>;

?

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-08-13 18:12 -------
(In reply to comment #4)
> which confuses me a bit, because here the prototype of f looks like
> effectively
> 
> void f(A*);

No that is correct as it turns the class into a non pod and non pods are always passed via reference and 
not via value.

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[rguenth at tat dot physik dot uni-tuebingen dot de]

------- Additional Comments From rguenth at tat dot physik dot uni-tuebingen dot de  2005-08-13 18:16 -------
Indeed - adding a destructor (or anything else that makes it a non-POD) "fixes"
the problem, too.

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[rguenth at gcc dot gnu dot org]

------- Additional Comments From rguenth at gcc dot gnu dot org  2005-08-13 21:21 -------
The best place to fix this is probably still the expander or TER.  Or
out-of-ssa, where the necessary information is best present.  Or fix gimple and
gimplification.

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[guillaume dot melquiond at ens-lyon dot fr]

------- Additional Comments From guillaume dot melquiond at ens-lyon dot fr  2005-08-14 06:45 -------
Looking at it again, I found an even worse regression with respect to g++ 3.4.
Consider this testcase:

struct A { int a[1000]; }
A f();
void g(A);
void h() { g(f()); }

Ideally, h will allocate a stack frame for g and ask f to directly dump its
result in it. No temporary nor memcpy will be used at all. g++ 3.4 behaves this way.

g++ 4.0 however will first allocate some space for the result of f, then call f
and copy its result in another temporary, and finally it will allocate the stack
frame for g and copy the temporary in it. Two temporaries and two memcpys are
needed for g++ 4.0.

So the same issue arises when returning a result by value.

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[rguenth at gcc dot gnu dot org]

------- Additional Comments From rguenth at gcc dot gnu dot org  2005-08-26 14:30 -------
For your last testcase,

struct A { int a[1000]; }
A f();
void g(A);
void h() { g(f()); }

all of 3.4 and 4.1 produce exactly two temporaries.
One to dump the result of f(), which get's copied to a new temp passed to g().

4.0 though produces one extra unnecessary copy.

The same holds true for C testcases.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   GCC host triplet|i486-linux-gnu              |


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[guillaume dot melquiond at ens-lyon dot fr]

------- Additional Comments From guillaume dot melquiond at ens-lyon dot fr  2005-08-26 17:38 -------
> all of 3.4 and 4.1 produce exactly two temporaries.

Yet I said that g++ 3.4 did not produce any temporary, and I still think so. No
temporaries, only g's stack frame. See the following assembly code for the C
testcase (the generated assembly is the same as for C++, but easier to read
since there is no name mangling nor local labels).

h:
        pushl   %ebp
        movl    %esp, %ebp
        subl    $4008, %esp
        movl    %esp, %eax
        subl    $12, %esp
        pushl   %eax
        call    f
        addl    $12, %esp
        call    g
        leave
        ret

For the sake of completeness, I'm also writing the assembly output for GCC 4.0,
so that the regression with respect to GCC 3.4 is clearly visible. Two
temporaries and two memory copies:

h:
        pushl   %ebp
        movl    %esp, %ebp
        pushl   %esi
        pushl   %ebx
        subl    $8012, %esp
        leal    -8008(%ebp), %ebx
        pushl   %ebx
        call    f
        leal    -4008(%ebp), %esi
        subl    $8, %esp
        pushl   $4000
        pushl   %ebx
        pushl   %esi
        call    memcpy
        subl    $3968, %esp
        movl    %esp, %eax
        pushl   %edx
        pushl   $4000
        pushl   %esi
        pushl   %eax
        call    memcpy
        addl    $16, %esp
        call    g
        addl    $4000, %esp
        leal    -8(%ebp), %esp
        popl    %ebx
        popl    %esi
        popl    %ebp
        ret

The C testcase is almost identical to the C++ testcase:

typedef struct A { int a[1000]; } A;
A f();
void g(A);
void h() { g(f()); }

And this is my version of GCC 3.4:

$ LANG=C gcc-3.4 -v
Reading specs from /usr/lib/gcc/i486-linux-gnu/3.4.5/specs
Configured with: ../src/configure -v
--enable-languages=c,c++,f77,pascal,objc,ada --prefix=/usr --libexecdir=/usr/lib
--with-gxx-include-dir=/usr/include/c++/3.4 --enable-shared --with-system-zlib
--enable-nls --without-included-gettext --program-suffix=-3.4
--enable-__cxa_atexit --enable-libstdcxx-allocator=mt --enable-clocale=gnu
--enable-libstdcxx-debug i486-linux-gnu
Thread model: posix
gcc version 3.4.5 20050821 (prerelease) (Debian 3.4.4-8)

Hope it helps.

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[rguenth at gcc dot gnu dot org]

------- Additional Comments From rguenth at gcc dot gnu dot org  2005-08-29 12:15 -------
I may have a patch^Whack to fix the first testcase.  Let's see if it passes
testing...

-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[rguenth at gcc dot gnu dot org]

------- Additional Comments From rguenth at gcc dot gnu dot org  2005-08-29 13:13 -------
One possibility would be to hack out-of-ssa to coalesce single use variables
with their defs in the case of aggregates.  The real fix would involve
expanding to rtl from ssa, so we have this information ready and need not
create these useless memcpy's.  Or whatever solution is more "correct" here
("fixing" the frontends will not work for the second testcase until we allow
function calls as arguments in gimple).

Anyway, here's the hack that passed bootstrapping and regtesting for C and C++
with only some tr1 tests failing:

Index: gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.113.2.11
diff -c -3 -p -r2.113.2.11 gimplify.c
*** gimplify.c  16 Aug 2005 22:16:52 -0000      2.113.2.11
--- gimplify.c  29 Aug 2005 12:04:33 -0000
*************** gimplify_target_expr (tree *expr_p, tree
*** 3628,3633 ****
--- 3628,3641 ----

    if (init)
      {
+       /* Try to avoid the temporary if possible.  */
+       if (TREE_CODE (init) == INDIRECT_REF
+         && !TARGET_EXPR_CLEANUP (targ))
+       {
+         *expr_p = init;
+         return GS_OK;
+       }
+
        /* TARGET_EXPR temps aren't part of the enclosing block, so add it
         to the temps list.  */
       gimple_add_tmp_var (temp);



-- 


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-09-14 06:36 -------
Another way to fix this would have copy-propagation for aggregates, see PR 14295.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |14295


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

[Bug c++/23372] [4.0/4.1 Regression] Temporary aggregate copy not elided when passing parameters by value

[mmitchel at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.2                       |4.0.3


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