[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.


================
Comment at: lib/CodeGen/CGObjCGNU.cpp:439
+                             ArrayRef<llvm::Constant *> IvarOffsets,
+                             ArrayRef<llvm::Constant *> IvarAlign,
+                             ArrayRef<Qualifiers::ObjCLifetime> IvarOwnership);
----------------
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?


================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1402
+    Stop->setVisibility(llvm::GlobalValue::HiddenVisibility);
+    return { Start, Stop };
+  }
----------------
DHowett-MSFT wrote:
> This should be readily expandable for PE/COFF, but we'll need to change some of the section names to fit better. Is it worth abstracting the names of the sections across the target format somehow?
> 
> (pe/coff will need to emit COMDAT symbols pointing into lexicographically sortable section names that the linker can fold away)
As I understand it, we can just append `$m` for the symbol names on PE/COFF and then create zero-size symbols with `$a` and `$z` suffixes for the end?  The symbol names should be able to be the same, but if that's not possible then it would be a good idea to change them so that ELF and PE/COFF binaries look the same.


================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1446
+        /*isConstant*/true, llvm::GlobalValue::LinkOnceAnyLinkage,
+        LoadFunction, ".objc_ctor");
+    // Check that this hasn't been renamed.  This shouldn't happen, because
----------------
DHowett-MSFT wrote:
> Is there a way to ensure that objc_load happens before other static initializers across the entire set of linker inputs?
This part is quite ELF-specific, and on ELF unfortunately the order of constructor initialisation is undefined (it will be in whatever order the linker decides to put them in).  Running the Objective-C code first isn't necessarily correct: C++ static constructors and Objective-C `+load` methods could each depend on the other (and C++ on ObjC classes having been loaded).

Of course, if you have control over the linker (or possibly even a linker script) then you can adjust the ordering however you want...


Repository:
  rC Clang

https://reviews.llvm.org/D46052