[Bug c/67283] New: GCC regression over inlining of returned structures

[Bug c/67283] New: GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

            Bug ID: 67283
           Summary: GCC regression over inlining of returned structures
           Product: gcc
           Version: 5.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: roche at httrack dot com
  Target Milestone: ---

Created attachment 36219
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36219&action=edit
Sample test case (gcc -S -O3 -W -Wall)

An optimization bug regression appears to exist when dealing with structures
returned by inlined functions. This was working totally fine with GCC 4.4.7.
(see below)

A typical example is:

struct foo {
        int flags;
        /* Let it be enough NOT to be packed in registers */
        void *opaque[2];
};

static __inline__ struct foo add_flag(struct foo foo, int flag) {
        foo.flags |= flag;
        return foo;
}

Calls to "add_flag" are inlined, but the stack usage increases with latest GCC
versions (the code should be almost identical, except the flag in place in the
stack).

Tested the following GCC versions: (grep "addq.*%rsp" to get stack usage for
each function) ; tested architecture: x86-64

GCC 4.4.7: OK
        addq    $72, %rsp  # demo_1
        addq    $72, %rsp
        addq    $72, %rsp
        addq    $72, %rsp
        addq    $72, %rsp  # demo_5

GCC 4.5.3 to 4.6.4: NOK (UNTESTED between 4.4.8 to 4.5.2)
        addq    $72, %rsp   # demo_1
        addq    $136, %rsp
        addq    $168, %rsp
        addq    $200, %rsp
        addq    $232, %rsp  # demo_5

GCC 4.7.3 to 5.2.0: NOK (UNTESTED between 4.6.5 to 4.7.2)
        addq    $72, %rsp   # demo_1
        addq    $136, %rsp
        addq    $200, %rsp
        addq    $264, %rsp
        addq    $328, %rsp  # demo_5

Therefore, the test case was fine in GCC 4.4.7, first degraded between 4.4.8
and 4.5.3, and then again between 4.6.5 and 4.7.3

Note: code produced with http://gcc.godbolt.org/ with -O3 -W -Wall flags.
(same results with -01)

[Bug c/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #1 from Xavier Roche <roche at httrack dot com> ---
Created attachment 36220
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36220&action=edit
Produced assembly code with GCC 4.4.7 on x86_64

Produced assembly code with GCC 4.4.7 on x86_64

[Bug c/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #2 from Xavier Roche <roche at httrack dot com> ---
Created attachment 36221
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36221&action=edit
Produced assembly code with GCC 4.6.4 on x86_64

Produced assembly code with GCC 4.6.4 on x86_64

[Bug c/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #3 from Xavier Roche <roche at httrack dot com> ---
Created attachment 36222
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36222&action=edit
Produced assembly code with GCC 5.2.0 on x86_64

Produced assembly code with GCC 5.2.0 on x86_64

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-08-20
                 CC|                            |jamborm at gcc dot gnu.org
          Component|c                           |tree-optimization
     Ever confirmed|0                           |1

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
;; Function demo_3 (demo_3, funcdef_no=4, decl_uid=1849, cgraph_uid=4,
symbol_order=4)

Candidate (1904): foo
Candidate (1901): foo
Candidate (1898): foo
Candidate (1870): D.1870
Candidate (1869): D.1869
Candidate (1868): D.1868
! Disqualifying D.1868 - No scalar replacements to be created.
Created a replacement for D.1869 offset: 0, size: 32: SR.30

...
demo_3 ()
{
  int foo;
  int foo;
  int SR.30;
  struct foo foo;
  struct foo foo;
  struct foo foo;
  struct foo D.1870;
  struct foo D.1869;
  struct foo D.1868;

  <bb 2>:
  foo = {};
  D.1868 = foo;
  foo ={v} {CLOBBER};
  foo = D.1868;
  foo_12 = MEM[(struct foo *)&D.1868];
  foo_11 = 1;
  MEM[(struct foo *)&foo] = foo_11;
  D.1869 = foo;
  SR.30_13 = foo_11;
  SR.30_14 = 1;
  MEM[(struct foo *)&D.1869] = SR.30_14;
  foo = D.1869;
  foo_16 = SR.30_14;
  foo_17 = 3;
  MEM[(struct foo *)&foo] = foo_17;
  D.1870 = foo;
  some_unknown_function (D.1870);
  return;

}

so it does _some_ SRA but not all required SRA for some reason.

Starts (IMHO) even with new_foo itself during early opts:

Candidate (1835): foo
! Disqualifying foo - No scalar replacements to be created.
new_foo ()
{
  struct foo foo;

  <bb 2>:
  foo = {};
  <retval> = foo;
  foo ={v} {CLOBBER};
  return <retval>;

}

where for some reason we fail to scalarize foo.  Similar add_flag
which ends up with

add_flag (struct foo foo, int flag)
{
  int foo$flags;
  int _2;
  int _4;

  <bb 2>:
  foo$flags_5 = MEM[(struct foo *)&foo];
  _2 = foo$flags_5;
  _4 = _2 | flag_3(D);
  foo$flags_7 = _4;
  MEM[(struct foo *)&foo] = foo$flags_7;
  <retval> = foo;
  return <retval>;

}

hmm, but here we are returning a parameter - we can't scalarize the parameter
and we can't scalarize the return.  But we still want to avoid storing
into 'foo' and store into retval directly.

;; Function demo_1 (demo_1, funcdef_no=2, decl_uid=1843, cgraph_uid=2,
symbol_order=2)

Candidate (1890): foo
Candidate (1858): D.1858
! Disqualifying D.1858 - No scalar replacements to be created.
! Disqualifying foo - No scalar replacements to be created.
demo_1 ()
{
  struct foo foo;
  struct foo D.1858;

  <bb 2>:
  foo = {};
  D.1858 = foo;
  foo ={v} {CLOBBER};
  some_unknown_function (D.1858);
  return;

so no total scalarization for foo?  Maybe SRA is confused by the CLOBBER?

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #6 from Xavier Roche <roche at httrack dot com> ---
> the problem is that the structure contains an array an total scalarization is not implemented for them

I confirm that without any array the inlining is fine.

Side note: the same problem appears with an union (including union with only
one void* member).

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #7 from alalaw01 at gcc dot gnu.org ---
Author: alalaw01
Date: Thu Aug 27 15:40:10 2015
New Revision: 227265

URL: https://gcc.gnu.org/viewcvs?rev=227265&root=gcc&view=rev
Log:
completely_scalarize arrays as well as records

gcc/:

        PR tree-optimization/67283
        * tree-sra.c (type_consists_of_records_p): Rename to...
        (scalarizable_type_p): ...this, add case for ARRAY_TYPE.

        (completely_scalarize_record): Rename to...
        (completely_scalarize): ...this, add ARRAY_TYPE case, move some code
to:
        (scalarize_elem): New.

gcc/testsuite/:

        * gcc.dg/tree-ssa/sra-15.c: New.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-sra.c

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

alalaw01 at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alalaw01 at gcc dot gnu.org

--- Comment #8 from alalaw01 at gcc dot gnu.org ---
I believe this should now be fixed. Do we want a testcase, and if so is there a
good way to scan for the stack usage pattern (as observed in the assembler)?
One can scan-assembler times for addq.*%rsp, but fixing the constant 72 seems
rather fragile, and I don't see a dejagnu way to scan for the constant being
the same in each demoN()...

And the case of unions is still not handled!!

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #9 from Xavier Roche <roche at httrack dot com> ---
Created attachment 36260
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36260&action=edit
Second test case (might be useful for unit testing)

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #10 from Xavier Roche <roche at httrack dot com> ---
The "Second test case" attached should produce exactly the same bytes
(byte-to-byte) for the two demo_1 and demo_2 functions. And this would not rely
on stack size that might change.

With GCC 4.4.7:

demo_2():
        subq    $72, %rsp
        movl    $0, 32(%rsp)
        movq    32(%rsp), %rax
        movq    $0, 48(%rsp)
        movq    $0, 40(%rsp)
        movq    $0, 8(%rsp)
        movq    $0, 16(%rsp)
        movq    %rax, (%rsp)
        call    some_unknown_function(foo)
        addq    $72, %rsp
        ret
demo_1():
        subq    $72, %rsp
        movl    $0, 32(%rsp)
        movq    32(%rsp), %rax
        movq    $0, 48(%rsp)
        movq    $0, 40(%rsp)
        movq    $0, 8(%rsp)
        movq    $0, 16(%rsp)
        movq    %rax, (%rsp)
        call    some_unknown_function(foo)
        addq    $72, %rsp
        ret

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[roche at httrack dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #11 from Xavier Roche <roche at httrack dot com> ---
PS: Shall I create a twin ticket for the structure case ?

[Bug tree-optimization/67283] GCC regression over inlining of returned structures

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67283

--- Comment #12 from alalaw01 at gcc dot gnu.org ---
Author: alalaw01
Date: Fri Aug 28 15:04:17 2015
New Revision: 227303

URL: https://gcc.gnu.org/viewcvs?rev=227303&root=gcc&view=rev
Log:
Revert: completely_scalarize arrays as well as records

gcc/:
        Revert:
        2015-08-27  Alan Lawrence  <alan.lawrence@arm.com>
                PR tree-optimization/67283
                * tree-sra.c (type_consists_of_records_p): Rename to...
                (scalarizable_type_p): ...this, add case for ARRAY_TYPE.

                (completely_scalarize_record): Rename to...
                (completely_scalarize): ...this, add ARRAY_TYPE case, move some
                 code to:
                (scalarize_elem): New.

gcc/testsuite/:

        Revert:
        2015-08-27  Alan Lawrence  <alan.lawrence@arm.com>
                * gcc.dg/tree-ssa/sra-15.c: New.

Removed:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-sra.c