Bug 7247 - [3.2 regression] copy constructor missing when inlining enabled for i386
Summary: [3.2 regression] copy constructor missing when inlining enabled for i386
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2002-07-09 10:26 UTC by gawrilow
Modified: 2003-07-25 17:33 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
bad_inline.cc (1.86 KB, application/octet-stream)
2003-05-21 15:17 UTC, gawrilow
Details

Note You need to log in before you can comment on or make changes to this bug.
Description gawrilow 2002-07-09 10:26:00 UTC
A copy constructor for a class is not called at the bottom of a complex expression involving many small inlined functions.

The class in question is "double_zero", defined at line 121. It does not matter whether the constructor is defined explicitly or by compiler itself.

The place its copy contructor ought to be called is the constructor of "modified_container_base" (line 91), instantiated from "TransformedContainer" (line 156), instantiated in its turn from "IncidenceMatrix" (line 293).

Sorry for a lengthy example; if I would simplify the code a bit further, the bug would disappear. The original program  had over 60K lines.

The bug disappears when inlining is disabled, but also when the function "attach_operation" (line 165) is made more fat by inserting "dump_op(op);" at its beginning. Surprisingly, the whole expression remains inlined, but the resulting code is suddenly correct!

On a UltraSPARC platform (Solaris 8), this bug does not occur at all, in any setting.

gcc 3.0.4 seems to be free of this bug on both platforms.

Release:
3.1

Environment:
Reading specs from /usr/site-local/experimental/lib/gcc-lib/i686-pc-linux-gnu/3.1/specs
Configured with: ../gcc-3.1/configure --prefix=/usr/site-local/experimental --with-local-prefix=/usr/site-local --with-gxx-include-dir=/usr/site-local/share/include/g++ --enable-shared --enable-libgcj --enable-languages=c++,java --with-cpu=athlon
Thread model: single
gcc version 3.1

How-To-Repeat:
compile the attached program with -O3 and run it.
If the generated code is correct, both lines printed will contain the same number, 1e-5.
Comment 1 Nathan Sidwell 2002-07-09 12:07:05 UTC
State-Changed-From-To: open->feedback
State-Changed-Why: If I understand you correctly, you are complaining that
    a copy ctor call is being optimized away.
    This is allowed, under [12.8]/15, even if the copy ctor
    has side-effects
Comment 2 gawrilow 2002-07-10 11:07:57 UTC
From: Ewgenij Gawrilow <gawrilow@math.TU-Berlin.DE>
To: nathan@gcc.gnu.org
Cc: gcc-bugs@gcc.gnu.org, gcc-gnats@gcc.gnu.org, gcc-prs@gcc.gnu.org
Subject: Re: optimization/7247: copy constructor missing when inlining enabled for i386
Date: Wed, 10 Jul 2002 11:07:57 +0200

 >>>>> "nathan" == nathan  <nathan@gcc.gnu.org> writes:
 
     nathan> Synopsis: copy constructor missing when inlining enabled
     nathan> for i386
 
     nathan> If I understand you correctly, you are
     nathan> complaining that a copy ctor call is being optimized away.
     nathan> This is allowed, under [12.8]/15, even if the copy ctor
     nathan> has side-effects
 
 The class whose construction fails is defined in my test case
 as follows:
 
 struct double_zero {
    const double epsilon;
 
    double_zero(const double& epsilon_arg=1e-8) : epsilon(epsilon_arg) { }
 };
 
 One would expect from a copy constructor, especially from the implicit
 one, that it would copy the value of the instance variable
 "epsilon". I don't think this could fall under the definition of a side effect.
 
 Alternatively, if optimization according to the clause [12.8]/15 comes
 into effect here, as you mean, then it is the constructor
 double_zero(const double&) that is being forgotten. At any rate, the
 "epsilon" variable contains garbage after the construction, as shows up
 in the debugging output.
 
 Encouraged by such a quick response, I hope for a similarly quick fix :-)
 
 With best regards,
 Ewgenij Gawrilow

Comment 3 Jason Merrill 2002-07-11 22:02:12 UTC
From: Jason Merrill <jason@redhat.com>
To: "H. J. Lu" <hjl@lucon.org>
Cc: gawrilow@math.tu-berlin.de, gcc-gnats@gcc.gnu.org,
	gcc-bugs@gcc.gnu.org
Subject: Re: optimization/7247: copy constructor missing when inlining
 enabled for i386
Date: Thu, 11 Jul 2002 22:02:12 +0100

 >>>>> "H" == H J Lu <hjl@lucon.org> writes:
 
 > Jason, it looks like your patch doesn't catch this one.
 
 No, as the bug isn't related to the NRVO; this testcase doesn't have any
 returns which would be affected.  Rather, it seems to be a dependency
 tracking bug.  I can reproduce the bug by adding 'inline' to the definition
 of incidence_matrix and compiling with -O -fschedule-insns2.  Without
 -fschedule-insns2, the code leading up to the dump_op calls looks like
 
         movl    -96(%ebp), %eax
         movl    -92(%ebp), %edx
         movl    %eax, -52(%ebp)
         movl    %edx, -48(%ebp)
         leal    -96(%ebp), %eax
         movl    %eax, (%esp)
         call    pm::dump_op(pm::double_zero const&)
         leal    -52(%ebp), %eax
         movl    %eax, (%esp)
         call    pm::dump_op(pm::double_zero const&)
 
 so the epsilon value is properly copied before it is dumped.  But with
 scheduling, it looks like
 
         movl    -96(%ebp), %eax
         movl    %eax, -52(%ebp)
         leal    -96(%ebp), %eax
         movl    %eax, (%esp)
         call    pm::dump_op(pm::double_zero const&)
         leal    -52(%ebp), %eax
         movl    %eax, (%esp)
         call    pm::dump_op(pm::double_zero const&)
 
 Which suggests that the scheduler incorrectly decides that the second word
 wasn't used by the call and optimizes away the copy.  This bug, or a
 similar one, seems still to be present in the trunk, though to see it you
 also need to move the definition of the double_zero constructor outside the
 class, so it is not inlined.
 
 Jason
Comment 4 Volker Reichelt 2002-12-19 06:19:45 UTC
State-Changed-From-To: feedback->analyzed
State-Changed-Why: Confirmed (just for the record).
    
    Here's a reduced testcase that fails on the 3.2 branch.
    gcc 3.0.x, 3.3 and mainline do not seem to be affected.
    
    --------------------------------snip here---------------------------------
    extern "C" { extern int printf (const char *, ...) throw (); }
    
    struct A {
       const double d;
       A() : d(1e-4) {}
       void dump () const { printf("%e\n", d); }
    };
    
    struct B {
       A a;
       int i;
    
       B(const A& arg) : a(arg) { arg.dump(); a.dump();}
    };
    
    struct C : B {
       C(const A& arg) : B(arg) {}
    };
    
    struct X {
       int i, j, k;
    
       X() : j(0) {}
    };
    
    struct Y {
       template <typename T> Y(int, const T&) {}
    };
    
    inline int foo() { X x; return 0; }
    
    int main() {
       Y y(foo(), C(A()));
       return 0;
    }
    --------------------------------snip here---------------------------------
    
    Just compile with "-O2" or "-O -fschedule-insns2" (on i686-pc-linux-gnu)
    to get the wrong result
    
      1.000000e-04
      1.948841e-314
    
    instead of
    
      1.000000e-04
      1.000000e-04
    
    
    Regards,
    Volker
Comment 5 janis187 2003-01-14 14:57:50 UTC
From: Janis Johnson <janis187@us.ibm.com>
To: gcc-gnats@gcc.gnu.org, gcc-prs@gcc.gnu.org, gawrilow@math.tu-berlin.de,
   gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org
Cc:  
Subject: Re: optimization/7247: [3.2 regression] copy constructor missing when 
 inlining enabled for i386
Date: Tue, 14 Jan 2003 14:57:50 -0800

 This bug was fixed on the mainline with this patch:
 
 --- gcc/cp/ChangeLog ---
 2002-03-11  Dan Nicolaescu  <dann@ics.uci.edu>
           Daniel Berlin  <dan@dberlin.org>
 
       * cp-lang.c (ok_to_generate_alias_set_for_type): New function.
       (cxx_get_alias_set): Use it.
 
 --- gcc/ChangeLog ---
 2002-03-11  Dan Nicolaescu  <dann@ics.uci.edu>
           Daniel Berlin  <dan@dberlin.org>
 
       C++ alias analysis improvement.
       * alias.c (record_component_aliases): Record aliases for base
       classes too.
 
 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=7247
 
 
 
Comment 6 Joe Buck 2003-04-25 19:51:03 UTC
State-Changed-From-To: analyzed->closed
State-Changed-Why: Fixed for the next release (3.3).