ahatanak marked 2 inline comments as done.
ahatanak added inline comments.
================
Comment at: lib/CodeGen/CGExprAgg.cpp:315
+ }
+ }
}
----------------
ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > Do these functions have a memcpy as a precondition? I would expect
> > > > them to do the full copy (for code size if nothing else).
> > > Yes, there should always be a call to memcpy before the copy/move special
> > > functions are called. I don't think we want to fold the call to memcpy
> > > into the body of the special functions since another special function can
> > > be called from the body if the non-trivial C struct has a field whose
> > > type is a non-trivial C struct too (in which case, there will be a
> > > redundant memcpy to copy the C struct field).
> > >
> > > For example, in the following code, there shouldn't be a call to memcpy
> > > to copy field "f0" of StrongOuter if there is already a memcpy that
> > > copies struct StrongOuter:
> > >
> > > ```
> > > typedef struct {
> > > int f0;
> > > id f1;
> > > } Strong;
> > >
> > > typedef struct {
> > > Strong f0;
> > > id f1;
> > > } StrongOuter;
> > > ```
> > >
> > Well, I guess I was imagining something more C++-ish where you don't
> > necessarily have a struct-wide memcpy, and instead you just memcpy the
> > parts where that's profitable and otherwise do something type-specific,
> > which would mean recursing for a struct. Your approach is reasonable if
> > the non-trivial copying is relatively sparse and the structure is large; on
> > the other hand, if the non-trivial copying is dense, the memcpy itself
> > might be mostly redundant. And it does mean a bigger code-size hit in the
> > original place that kicks off the copy.
> >
> > IIRC we do already have some code in copy-constructor emission that's
> > capable of emitting sequences of field copies with a memcpy, if you wanted
> > to try the C++ style, you could probably take advantage of that pretty
> > easily. But I won't hold up the patch if you think the memcpy precondition
> > is the right way to go.
> I made changes so that either a load/store pair or a call to memcpy is
> emitted in the copy/move special functions to copy fields that are not
> non-trivial.
I haven't implemented the optimization you suggested which merges a sequence of
field copies into a single memcpy. I'll look into it later.
https://reviews.llvm.org/D41228
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
