Bug 42585 - [4.5 Regression] SRA is not good for structure copies with one replacement any more
Summary: [4.5 Regression] SRA is not good for structure copies with one replacement an...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.5.0
: P2 normal
Target Milestone: 4.5.0
Assignee: Martin Jambor
URL:
Keywords: missed-optimization
Depends on:
Blocks: 42586
  Show dependency treegraph
 
Reported: 2010-01-03 05:56 UTC by Andi Kleen
Modified: 2010-03-02 14:45 UTC (History)
4 users (show)

See Also:
Host: x86_64-linux
Target: x86_64-linux -m32
Build:
Known to work: 4.4.0
Known to fail: 4.5.0
Last reconfirmed: 2010-01-18 12:39:10


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andi Kleen 2010-01-03 05:56:56 UTC
From one of the examples from http://embed.cs.utah.edu/embarrassing/dec_09/harvest/gcc-head_llvm-gcc-head/

struct _fat_ptr
{
  unsigned char *curr;
  unsigned char *base;
  unsigned char *last_plus_one;
};
int Cyc_string_ungetc (int ignore, struct _fat_ptr *sptr);
int
Cyc_string_ungetc (int ignore, struct _fat_ptr *sptr)
{
  struct _fat_ptr *_T0;
  struct _fat_ptr *_T1;
  struct _fat_ptr _T2;
  int _T3;
  struct _fat_ptr _ans;
  int _change;

  {
    _T0 = sptr;
    _T1 = sptr;
    _T2 = *sptr;
    _T3 = -1;
    _ans = _T2;
    _change = -1;
    _ans.curr += 4294967295U;
    *sptr = _ans;
    return (0);
  }
}

Testing on GCC: (GNU) 4.5.0 20091219

With -O2 this generates only midly inefficient code (although still a bit larger
than llvm's very neat
   8b 44 24 08          	mov    0x8(%esp),%eax
   4:	ff 08                	decl   (%eax)
   6:	31 c0                	xor    %eax,%eax
   8:	c3                   	ret    
)

-O2 code:
        subl    $32, %esp
        movl    40(%esp), %eax
        movl    (%eax), %edx
        subl    $1, %edx
        movl    %edx, (%eax)
        xorl    %eax, %eax
        addl    $32, %esp
        ret


That's a bit dumb because it allocates stack without using it and 
also does not use a subl $1,(eax), but explicit load-modify-store, but not overly bad (I'll open a separate bug for the load/modify/store)

But with -Os the code is really bad:

    pushl   %edi
        movl    $3, %ecx
        pushl   %esi
        subl    $32, %esp
        movl    48(%esp), %eax
        leal    20(%esp), %edi
        movl    %eax, %esi
        rep movsl
        leal    8(%esp), %edi
        leal    20(%esp), %esi
        movl    (%eax), %edx
        movb    $3, %cl
        rep movsl
        leal    8(%esp), %esi
        movl    %eax, %edi
        decl    %edx
        movb    $3, %cl
        rep movsl
        movl    %edx, (%eax)
        addl    $32, %esp
        xorl    %eax, %eax
        popl    %esi
        popl    %edi
        ret


It doesn't modify the object in place, but instead copies it around.
Unsurprisingly that generates that much larger (and slower code)
Comment 1 Andrew Pinski 2010-01-03 06:41:47 UTC
I think fixing this will also fix the PR 42586 on the trunk.

Confirmed, a regression from 4.4.  SRA is worse off on the trunk for this case than with 4.4.
Comment 2 Martin Jambor 2010-01-18 16:29:02 UTC
Patch restoring the 4.4 behavior posted to the mailing list:
http://gcc.gnu.org/ml/gcc-patches/2010-01/msg00976.html
Comment 3 Martin Jambor 2010-01-21 16:18:22 UTC
Subject: Bug 42585

Author: jamborm
Date: Thu Jan 21 16:18:06 2010
New Revision: 156156

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156156
Log:
2010-01-21  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/42585
	* tree-sra.c (struct access): New field grp_total_scalarization.
	(dump_access): Dump the new field.
	(should_scalarize_away_bitmap): New variable.
	(cannot_scalarize_away_bitmap): Likewise.
	(sra_initialize): Allocate new bitmaps.
	(sra_deinitialize): Free new bitmaps.
	(create_access_1): New function.
	(create_access): Parts moved to create_access_1.
	(type_consists_of_records_p): New function.
	(completely_scalarize_record): Likewise.
	(build_access_from_expr): Set bit in cannot_scalarize_away_bitmap.
	(build_accesses_from_assign): Set bits in should_scalarize_away_bitmap.
	(sort_and_splice_var_accesses): Hint groups with a total_scalarization
	access.
	(analyze_all_variable_accesses): Completely scalarize small eligible
	records.

	* testsuite/gcc.dg/tree-ssa/pr42585.c: New test.


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

Comment 4 Ryan Mansfield 2010-01-22 05:37:32 UTC
Rev 156156 breaks the build for targets that do not explicitly define MOVE_RATIO in their backend (e.g. rs6000).
Comment 5 Steven Bosscher 2010-01-22 08:07:09 UTC
Problem here, is that the default MOVE_RATIO is in expr.h. It is really a default definition of a target macro so it should be defined in defaults.h.
Comment 6 Steven Bosscher 2010-01-22 09:39:34 UTC
*** Bug 42842 has been marked as a duplicate of this bug. ***
Comment 7 Martin Jambor 2010-01-25 10:38:05 UTC
I understand that the bootstrap problem is PR 42836 and is now fixed
(thanks for that).  SRA now removes the structures in the testcase
from bug description and so I consider this to be fixed.
Comment 8 hjl@gcc.gnu.org 2010-02-07 04:42:59 UTC
Subject: Bug 42585

Author: hjl
Date: Sun Feb  7 04:41:22 2010
New Revision: 156562

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156562
Log:
Backport testcases from mainline to 4.4.

2010-02-06  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline:
	2010-02-05  Dodji Seketeli  <dodji@redhat.com>

	PR c++/42915
	* g++.dg/other/crash-9.C: New test.

	2010-02-03  Jason Merrill  <jason@redhat.com>

	PR c++/40138
	* g++.dg/ext/builtin11.C: New.

	2010-02-03  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42944
	* gcc.dg/errno-1.c: New testcase.

	2010-02-03  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42927
	* gcc.c-torture/compile/pr42927.c: New testcase.

	2010-01-29  Dodji Seketeli  <dodji@redhat.com>

	PR c++/42758
	PR c++/42634
	PR c++/42336
	PR c++/42797
	PR c++/42880
	* g++.dg/other/crash-5.C: New test.
	* g++.dg/other/crash-7.C: New test.
	* g++.dg/other/crash-8.C: New test.

	2010-01-28  Uros Bizjak  <ubizjak@gmail.com>

	PR target/42891
	* gcc.target/i386/pr42891.c: New test.

	2010-01-28  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42883
	* g++.dg/torture/pr42883.C: New testcase.

	2010-01-28  Michael Matz  <matz@suse.de>

	* gcc.target/i386/pr42881.c: New test.

	2010-01-28  Dodji Seketeli  <dodji@redhat.com>

	PR c++/42713
	PR c++/42820
	* g++.dg/template/typedef27.C: New test case.
	* g++.dg/template/typedef28.C: New test case.

	2010-01-27  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/42874
	* gcc.dg/vla-22.c: New test.

	2010-01-26  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42250
	* gcc.dg/pr42250.c: New testcase.

	2010-01-25  Tobias Burnus  <burnus@net-b.de>

	PR fortran/42858
	* gfortran.dg/generic_21.f90: New test.

	2010-01-21  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/42585
	* gcc.dg/tree-ssa/pr42585.c: New test.

	2010-01-20  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/42715
	* gcc.dg/pr42715.c: New.

	2010-01-20  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42717
	* gcc.c-torture/compile/pr42717.c: New testcase.

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

	PR fortran/42783
	* gfortran.dg/bounds_check_15.f90 : New test.

	2010-01-18  Dodji Seketeli  <dodji@redhat.com>

	PR c++/42766
	* g++.dg/conversion/op6.C: New test.

	2010-01-18  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42781
	* gfortran.fortran-torture/compile/pr42781.f90: New testcase.

	2010-01-17  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42248
	* gcc.c-torture/execute/pr42248.c: New testcase.

	2010-01-17  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/42677
	* gfortran.dg/interface_assignment_5.f90: New test.

	2010-01-15  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42739
	* g++.dg/torture/pr42739.C: New testcase.

	2010-01-14 Jerry DeLisle <jvdelisle@gcc.gnu.org>

	PR fortran/42684
	* gfortran.dg/interface_31.f90: New test.

	2010-01-14  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/42706
	* gcc.dg/ipa/pr42706.c: New testcase.

	2010-01-14  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/42714
	* g++.dg/torture/pr42714.C: New test.

	2010-01-14  Alexander Monakov  <amonakov@ispras.ru>

	PR rtl-optimization/42388
	* gcc.dg/pr42388.c: New.

	2010-01-14  Alexander Monakov <amonakov@ispras.ru>

	PR rtl-optimization/42294
	* gfortran.dg/pr42294.f: New.

	2010-01-14  Ira Rosen  <irar@il.ibm.com>

	PR tree-optimization/42709
	* gcc.dg/vect/pr42709.c: New test.

	2010-01-13  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42730
	* gcc.c-torture/compile/pr42730.c: New testcase.

	2010-01-13  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/42704
	* g++.dg/torture/pr42704.C: New test.

	2010-01-13  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/42703
	* gcc.c-torture/compile/pr42703.c: New test.

	2010-01-13  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42705
	* gcc.c-torture/compile/pr42705.c: New testcase.

	2010-01-13  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42716
	* gcc.c-torture/compile/pr42716.c: New testcase.

	2010-01-12  Joseph Myers  <joseph@codesourcery.com>

	PR c/42708
	* gcc.c-torture/compile/pr42708-1.c: New test.

	2010-01-09  Alexandre Oliva  <aoliva@redhat.com>

	PR middle-end/42363
	* gcc.dg/torture/pr42363.c: New.

	2010-01-09  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/42604
	PR debug/42395
	* gcc.dg/vect/pr42604.c: New.
	* gcc.dg/vect/pr42395.c: New.

	2010-01-09  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42512
	* gcc.c-torture/execute/pr42512.c: New testcase.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/conversion/op6.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/conversion/op6.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/ext/builtin11.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/ext/builtin11.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/other/crash-5.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/other/crash-5.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/other/crash-7.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/other/crash-7.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/other/crash-8.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/other/crash-8.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/other/crash-9.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/other/crash-9.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/template/typedef27.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/template/typedef27.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/template/typedef28.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/template/typedef28.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr42704.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/torture/pr42704.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr42714.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/torture/pr42714.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr42739.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/torture/pr42739.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr42883.C
      - copied unchanged from r156561, trunk/gcc/testsuite/g++.dg/torture/pr42883.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42703.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42703.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42705.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42705.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42708-1.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42708-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42716.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42716.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42717.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42717.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42730.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42730.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42927.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/compile/pr42927.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr42248.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/execute/pr42248.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr42512.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.c-torture/execute/pr42512.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/errno-1.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/errno-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/ipa/pr42706.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/ipa/pr42706.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr42250.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/pr42250.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr42388.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/pr42388.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr42715.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/pr42715.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/torture/pr42363.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/torture/pr42363.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/tree-ssa/pr42585.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/tree-ssa/pr42585.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/vect/pr42395.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/vect/pr42395.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/vect/pr42604.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/vect/pr42604.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/vect/pr42709.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/vect/pr42709.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/vla-22.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.dg/vla-22.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.target/i386/pr42881.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.target/i386/pr42881.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.target/i386/pr42891.c
      - copied unchanged from r156561, trunk/gcc/testsuite/gcc.target/i386/pr42891.c
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/bounds_check_15.f90
      - copied unchanged from r156561, trunk/gcc/testsuite/gfortran.dg/bounds_check_15.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/generic_21.f90
      - copied unchanged from r156561, trunk/gcc/testsuite/gfortran.dg/generic_21.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/interface_31.f90
      - copied unchanged from r156561, trunk/gcc/testsuite/gfortran.dg/interface_31.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/interface_assignment_5.f90
      - copied unchanged from r156561, trunk/gcc/testsuite/gfortran.dg/interface_assignment_5.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr42294.f
      - copied unchanged from r156561, trunk/gcc/testsuite/gfortran.dg/pr42294.f
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.fortran-torture/compile/pr42781.f90
      - copied unchanged from r156561, trunk/gcc/testsuite/gfortran.fortran-torture/compile/pr42781.f90
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Comment 9 Mikael Pettersson 2010-02-28 11:53:02 UTC
(In reply to comment #8)
> Subject: Bug 42585
> 
> Author: hjl
> Date: Sun Feb  7 04:41:22 2010
> New Revision: 156562
> 
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156562
> Log:
> Backport testcases from mainline to 4.4.
> 
> 2010-02-06  H.J. Lu  <hongjiu.lu@intel.com>
> 
>         Backport from mainline:
...
>         PR tree-optimization/42585
>         * gcc.dg/tree-ssa/pr42585.c: New test.

This caused testsuite regressions for 4.4 on (at least) powerpc64 and arm:
http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02633.html
http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02441.html

Note that this test case also fails on trunk for (at least) powerpc64 and arm:
http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02728.html
http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02695.html
Comment 10 Martin Jambor 2010-03-02 14:45:03 UTC
(In reply to comment #9)
> 
> This caused testsuite regressions for 4.4 on (at least) powerpc64 and arm:
> http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02633.html
> http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02441.html
> 
> Note that this test case also fails on trunk for (at least) powerpc64 and arm:
> http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02728.html
> http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg02695.html
> 

This is a known problem tracked as PR 42855.  I am aware of it and
intend to do something about it but at the moment there are at least
three more important issues I need to take care of.