This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: gcc 3.0.1 & C++ ABI issues


>>>>> "Nathan" == Nathan Sidwell <nathan@codesourcery.com> writes:

> Note: The patch disables RVO for empty classes with non-trival copy-ctor.
> We lose an optimization (for a rather rare case, I guess), and gain
> correct code. To get RVO back the real fix is to teach MODIFY_EXPR and
> INIT_EXPR about empty classes in the common backend, I think. But
> we're time constrained here.

Perhaps something like TYPE_UNPADDED_SIZE would make sense.  Much simpler
than TYPE_NONCOPIED_PARTS.

> The test case works to detect RVO, and breaks when there is no RVO (but
> there's no comment in the test case saying that's what it's checking for).
> The important thing is that there are as many dtors as ctors.

> BTW, I've always considered a 'proper' RVO to consist of the ability to
> turn
> 		Obj thingy;
> 		return thingy;
> into (pseudo C++)
> 		Obj &thingy = *_return_value_ptr;
> 		return;
> This is somewhat stronger that we do right now isn't it?

We do that now, too, as of my recent patch.  We call that the Named Return
Value Optimization, or NRVO.  I still need to get it working with inlines,
though; several solutions occur to me, and I'm not sure which is The Way.

>>>>> "Mark" == Mark Mitchell <mark@codesourcery.com> writes:

> Correct code is clearly more important.  So, the patch is OK, but you
> should XFAIL the testcase, and enter a PR about it.  Since the RVO is
> new (on the 3.1 branch) there's no regression here relative to any
> released version of GCC.

No, the NRVO is new, but we have optimized 'return Class();' since before I
started working on the compiler.  So this is definitely a regression, if a
minor one.  Fortunately it won't affect STL, since the iterator tags don't
need constructing.

>> This is somewhat stronger that we do right now isn't it?

> Yes.  Ideally, the RVO would work on the CFG -- although we should
> see whether or not that will really buy us much in the way of
> additional performance before bothering.

I don't think it can.  The standard specifically allows two cases of copy
elision: when a temporary is copied into another object of the same type
(handled by TARGET_EXPR) and the NRVO.  It might be cleaner to replace
the TARGET_EXPR stuff with a tree optimization pass, but it would not
improve performance.

[and going back a bit...]

>>>>> "Nathan" == Nathan Sidwell <nathan@codesourcery.com> writes:

> Jason Merrill wrote:
>> > * build_over_call only tries to optimize the copy ctor of an empty
>> > class iff it does not need constructing
>> 
>> Why?  I don't follow the comment in the code.
> Previously, when ARG was !real_lvalue_p, we'd emit
> 	build (INIT_EXPR, DECL_CONTEXT (fn), to, arg);
> for non-empty objects, and
> 	build (MODIFY_EXPR, DECL_CONTEXT (fn), to, arg);
> for empty ones. The comment before that explains
> a) why we need a modify expr, so that TYPE_NONCOPIED_PARTS comes into play

Right.  Pretty ugly.  I do think that something like
TYPE_{UNPADDED,REAL_COPY}_SIZE(_UNIT)? would be appropriate here.

> b) we can't just ignore ARG, as it might have a TARGET_EXPR and we'd have
> unbalanced ctor/dtor pairing.

Yes.

> The bug was (a) is flawed when the class is empty, but has an empty base
> class. TYPE_NON_COPIED_PARTS is inactive and we end up smashing whatever
> overlaid the empty object in the class it belongs to. So we can't use
> the MODIFY_EXPR trick.

OK.

> We have to either a) do the copy ctor the hard way

i.e. don't optimize?

> or default construct TO and leave ARG to be dtor'd later. (I could not
> think of another way of acheiving the desired outcome.) So the type must
> have a default ctor.

No, that's wrong; we can't replace a copy ctor with a default ctor.
 
> Now, an empty class may or may not have trivial ctor, copyctor & dtor.
> It might *not* have a default ctor. If it does have a default ctor, I
> think it only safe if that is trivial. The std allows us to elide copies,
> but I don't think it allows us to turn (pseudocode)
> 	ARG.ctor (whatever);
> 	TO.ctor (ARG);	<- we are here
> 	ARG.dtor ();
> 	TO.dtor ()
> into
> 	ARG.ctor (whatever);
> 	TO.ctor ();
> 	ARG.dtor ();
> 	TO.dtor ();

Nope.  You're adding a call to the default constructor that never existed
before.

> While writing this, I think there's another user visible effect. The
> original, unoptimized code sequence consisted of some ctor into ARG,
> a copy ctor, and two dtors. We're allowed to turn that into some ctor
> into TO and a single dtor.

Yes.

> My patch turns it into some ctor into ARG, an (invisible) default ctor of
> TO, and *two* dtors (which may or may not be user visible).

If you're not going to optimize this case, it should use the copy ctor, not
the default ctor.  In addition to being correct, it's simpler--just fall
through and use the normal code.

Furthermore, there's no reason to disable the optimization for the case
where we're getting a temporary; it will be folded into the target
expression, so no bitwise copying will actually be done.  I suppose we
should split out the !real_lvalue_p case from the TYPE_HAS_TRIVIAL_INIT_REF
case for clarity.

>> Would we?  I've had the same thought, but I'm not sure it's true.  The
>> cases where we can eliminate copies are fairly few; apart from the named
>> return value optimization, we've handled them fine for a while now.

> Maybe you're right, but what about when inlining? Can we not eliminate
> parameters?

Are you thinking of this:

  inline A f (A a) { return a; }

  int main ()
  {
    A a1 = f (A ());
  }

?  Currently, this would involve two constructor calls, the default ctor to
construct the parameter and the copy ctor to initialize 'a1'.  It seems
reasonable to elide the copy ctor in this case, but IMO the standard
doesn't allow it.  We tried to come up with wording to allow that
optimization, but eventually gave up and just explicitly blessed the NRVO.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]