theraven marked 3 inline comments as done.
theraven added inline comments.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:439
+ ArrayRef<llvm::Constant *> IvarOffsets,
+ ArrayRef<llvm::Constant *> IvarAlign,
+ ArrayRef<Qualifiers::ObjCLifetime> IvarOwnership);
----------------
rjmccall wrote:
> theraven wrote:
> > DHowett-MSFT wrote:
> > > While we're here, is there value in storing the ivar size in layout as
> > > well, so that the runtime doesn't need to calculate it from the distance
> > > to the next ivar/end of the instance?
> > Normally the runtime calculates it from the type encoding, if it's
> > required. I agree that it might be nice to have though. Do you have
> > strong feelings about needing it in the 2.0 ABI? The looser coupling means
> > that it would be easy to add in the 2.1 ABI if we want it later. Are you
> > seeing cases where the runtime is calculating it incorrectly because of
> > insufficient information in the type encoding, or where calculating it is
> > causing performance problems?
> The distance between ivar offsets wouldn't be correct anyway because of
> alignment padding. A set-ivar function might be able to get away with
> copying too many bytes from an input buffer (although I wouldn't recommend
> it!), but a get-ivar function definitely should not copy too many bytes into
> an output buffer.
>
> And all that's assuming that the runtime promises not to reorder ivars
> dynamically. I don't know what the GNU runtime says about that. (Static
> reordering is fine, since the runtime can reasonably demand that the compiler
> emit ivars in layout order.)
The runtime was calculating the size from the type encoding (which can also be
wrong in a few cases, such as packed structures). I agree it makes sense to
add the size though, and have done..
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1172
+ if (isWeak) {
+ // Placeholder for the real symbol.
+ ClassSymbol->setInitializer(new llvm::GlobalVariable(TheModule,
----------------
rjmccall wrote:
> I would suggest clarifying in what sense this is a placeholder. Does the
> runtime recognize it specially? If so, how? Is it replaced statically by a
> later pass in IRGen?
This comment was inherited from the old version and is actually nonsense now.
I have replaced it with one that actually makes sense.
Repository:
rC Clang
https://reviews.llvm.org/D46052
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
