[Bug middle-end/54315] New: Unnecessary copy of return value

[Bug middle-end/54315] New: Unnecessary copy of return value

[hjl.tools at gmail dot com]

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

             Bug #: 54315
           Summary: Unnecessary copy of return value
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com
                CC: areg.melikadamyan@gmail.com, ubizjak@gmail.com


On Linux/x86-64, I got

[hjl@gnu-6 pr20020]$ cat x.i
union S160
{
  long double a;
};
extern union S160 check160 (void);
extern void checkx160 (union S160);
void
test160 (void)
{
  checkx160 (check160 ());
}
[hjl@gnu-6 pr20020]$ gcc -S -O2 x.i
[hjl@gnu-6 pr20020]$ cat x.s
    .file    "x.i"
    .text
    .p2align 4,,15
    .globl    test160
    .type    test160, @function
test160:
.LFB0:
    .cfi_startproc
    subq    $72, %rsp
    .cfi_def_cfa_offset 80
    call    check160
    fstpt    16(%rsp)
    movq    16(%rsp), %rdx
    movq    24(%rsp), %rax
    movq    %rdx, 32(%rsp)
    movq    %rax, 40(%rsp)
    movq    %rdx, (%rsp)
    movq    %rax, 8(%rsp)
    call    checkx160
    addq    $72, %rsp
    .cfi_def_cfa_offset 8
    ret
    .cfi_endproc
.LFE0:
    .size    test160, .-test160
    .ident    "GCC: (GNU) 4.7.1 20120629 (Red Hat 4.7.1-1)"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 pr20020]$ cat y.i
struct S160
{
  long double a;
};
extern struct S160 check160 (void);
extern void checkx160 (struct S160);
void
test160 (void)
{
  checkx160 (check160 ());
}
[hjl@gnu-6 pr20020]$ gcc -S -O2 y.i
[hjl@gnu-6 pr20020]$ cat y.s
    .file    "y.i"
    .text
    .p2align 4,,15
    .globl    test160
    .type    test160, @function
test160:
.LFB0:
    .cfi_startproc
    subq    $24, %rsp
    .cfi_def_cfa_offset 32
    call    check160
    fstpt    (%rsp)
    call    checkx160
    addq    $24, %rsp
    .cfi_def_cfa_offset 8
    ret
    .cfi_endproc
.LFE0:
    .size    test160, .-test160
    .ident    "GCC: (GNU) 4.7.1 20120629 (Red Hat 4.7.1-1)"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 pr20020]$ 

union causes unnecessary copy of return value. I don't believe
ABI calls for it.

[Bug middle-end/54315] Unnecessary copy of return value

[hjl.tools at gmail dot com]

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

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-18 17:42:58 UTC ---
-m32 also causes extra copy:

[hjl@gnu-6 pr54315]$ gcc -S -O2 -m32 y.i
[hjl@gnu-6 pr54315]$ cat y.s
    .file    "y.i"
    .text
    .p2align 4,,15
    .globl    test160
    .type    test160, @function
test160:
.LFB0:
    .cfi_startproc
    subl    $44, %esp
    .cfi_def_cfa_offset 48
    leal    16(%esp), %eax
    movl    %eax, (%esp)
    call    check160
    .cfi_def_cfa_offset 44
    subl    $4, %esp
    .cfi_def_cfa_offset 48
    fldt    16(%esp)
    fstpt    (%esp)
    call    checkx160
    addl    $44, %esp
    .cfi_def_cfa_offset 4
    ret
    .cfi_endproc
.LFE0:
    .size    test160, .-test160
    .ident    "GCC: (GNU) 4.7.1 20120629 (Red Hat 4.7.1-1)"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 pr54315]$

[Bug middle-end/54315] Unnecessary copy of return value

[hjl.tools at gmail dot com]

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

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-18 17:45:43 UTC ---
The difference between -m32 and -m64 is

-  struct S160 D.1692;
+  struct S160 D.1719;

 ;;   basic block 2, loop depth 0
 ;;    pred:       ENTRY
-  D.1692 = check160 (); [return slot optimization]
-  checkx160 (D.1692);
+  D.1719 = check160 ();
+  checkx160 (D.1719);
   return;
 ;;    succ:       EXIT

"return slot optimization" generates extra copy.

[Bug middle-end/54315] Unnecessary copy of return value

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-09-14
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org
     Ever Confirmed|0                           |1

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-09-14 14:54:53 UTC ---
The big discrepancy between structures and unions comes from:

/* Compute the TYPE_MODE for the TYPE (which is a RECORD_TYPE).  */

void
compute_record_mode (tree type)
{
[...]

  /* If we only have one real field; use its mode if that mode's size
     matches the type's size.  This only applies to RECORD_TYPE.  This
     does not apply to unions.  */
  if (TREE_CODE (type) == RECORD_TYPE && mode != VOIDmode
      && host_integerp (TYPE_SIZE (type), 1)
      && GET_MODE_BITSIZE (mode) == TREE_INT_CST_LOW (TYPE_SIZE (type)))
    SET_TYPE_MODE (type, mode);
  else
    SET_TYPE_MODE (type, mode_for_size_tree (TYPE_SIZE (type), MODE_INT, 1));

so the structure is given XFmode but the union is given BKLmode.

Apparently someone has attempted to bridge the gap but this was reverted by:

------------------------------------------------------------------------
r37041 | wilson | 2000-10-25 03:30:25 +0200 (Wed, 25 Oct 2000) | 6 lines

Fixes for C++ structure layout breakage.
        * expmed.c (store_bit_field): Move integer pun code down after
        code that calls emit_move_insn for entire register move.
        * stor-layout.c (compute_record_mode): Revert Mar 25, Aug 18, and
        Oct 20 changes.  Only store mode in TYPE_MODE if RECORD_TYPE.

[Bug middle-end/54315] Unnecessary copy of return value

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |ebotcazou at gcc dot
                   |gnu.org                     |gnu.org

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-09-15 08:23:41 UTC ---
I'll try to improve things on x86-64 at least.

[Bug middle-end/54315] unnecessary copy of return value for union

[ebotcazou at gcc dot gnu.org]

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

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-10-20 21:00:26 UTC ---
Author: ebotcazou
Date: Sat Oct 20 21:00:23 2012
New Revision: 192641

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192641
Log:
    PR rtl-optimization/54315
    * calls.c (expand_call): Don't deal specifically with BLKmode values
    returned in naked registers.
    * expr.h (copy_blkmode_from_reg): Adjust prototype.
    * expr.c (copy_blkmode_from_reg): Rename first parameter into TARGET and
    make it required.  Assert that SRCREG hasn't BLKmode.  Add a couple of 
    short-circuits for common cases and be prepared for sub-word registers.
    (expand_assignment): Call copy_blkmode_from_reg for BLKmode values
    returned in naked registers.
    (store_expr): Likewise.
    (store_field): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/calls.c
    trunk/gcc/expr.c
    trunk/gcc/expr.h

[Bug middle-end/54315] unnecessary copy of return value for union

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
         AssignedTo|ebotcazou at gcc dot        |unassigned at gcc dot
                   |gnu.org                     |gnu.org

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-10-20 21:08:48 UTC ---
The patch removes half of the useless stores on x86-64.