rjmccall added a comment.
Thanks, the comments help a lot.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:439
+ ArrayRef<llvm::Constant *> IvarOffsets,
+ ArrayRef<llvm::Constant *> IvarAlign,
+ ArrayRef<Qualifiers::ObjCLifetime> IvarOwnership);
----------------
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.)
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1065
+ }
+ }
+ auto *ObjCStrGV =
----------------
Seems much more reasonable, thanks.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1067
+ auto *ObjCStrGV =
+ Fields.finishAndCreateGlobal(isNamed ? StringName : ".objc_string",
+ Align, false, isNamed ? llvm::GlobalValue::LinkOnceODRLinkage :
----------------
Micro-optimization: I'm pretty sure the type of this ternary is an r-value
`std::string`, which means copying the string in either case; if you explicitly
build a StringRef from StringName, the ternary will instead construct a
StringRef and you'll avoid those unnecessary copies.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1069
+ Align, false, isNamed ? llvm::GlobalValue::LinkOnceODRLinkage :
+ llvm::GlobalValue::PrivateLinkage);
+ ObjCStrGV->setSection(ConstantStringSection);
----------------
The indentation here is a little misleading; I'd suggest aligning the two cases
of the ternary operator.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1172
+ if (isWeak) {
+ // Placeholder for the real symbol.
+ ClassSymbol->setInitializer(new llvm::GlobalVariable(TheModule,
----------------
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?
Repository:
rC Clang
https://reviews.llvm.org/D46052
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
