Optimize getters for non-atomic, copied properties

[dmaclach]

Summary

Properties that were declared @property(copy, nonatomic) id foo make an unnecessary call to objc_get_property(). This call can be replaced with a direct access to the backing variable identical to how a @property(nonatomic) id foo would do it.

This reduces codegen by 4 bytes (x86_64/arm64) and removes a cross linkage unit function call per property declared as copy/nonatomic.

[thakis]

Looks fine to me. Let's wait until Monday evening for rjmccall to reply, else I think this can go in.

[stephanemoore]

Looks reasonable to me.

GetSetProperty and SetPropertyAndExpressionGet are handled in the same code path for code generation for setters. The main difference seems like it should only be for the code generation for getters.

For libobjc, I expect this would replace a call to objc_getProperty with direct ivar access for non-atomic properties. As far as I know, objc_getProperty simply dereferences the ivar directly for non-atomic properties. I assume objc_getProperty has done this from the start (https://opensource.apple.com/source/objc4/objc4-371/runtime/Accessors.subproj/objc-accessors.m.auto.html is an early implementation that seems to support that assumption). Based on that, I don't anticipate any unintended change in behavior.

clang/lib/CodeGen/CGObjC.cpp

PropertyImplStrategy::PropertyImplStrategy(CodeGenModule &CGM,
 
   // Evaluate the ivar's size and alignment.
   ObjCIvarDecl *ivar = propImpl->getPropertyIvarDecl();
   QualType ivarType = ivar->getType();
   auto TInfo = CGM.getContext().getTypeInfoInChars(ivarType);
   IvarSize = TInfo.Width;
   IvarAlignment = TInfo.Align;
 
-  // If we have a copy property, we always have to use getProperty/setProperty.
-  // TODO: we could actually use setProperty and an expression for non-atomics.
+  // If we have a copy property, we always have to use setProperty.
+  // If the property is atomic we need to use getProperty, but in
+  // the nonatomic case we can just use expression.
   if (IsCopy) {
-    Kind = GetSetProperty;
+    Kind = IsAtomic ? GetSetProperty : SetPropertyAndExpressionGet;
     return;
   }
 
   // Handle retain.
   if (setterKind == ObjCPropertyDecl::Retain) {
     // In GC-only, there's nothing special that needs to be done.
     if (CGM.getLangOpts().getGC() == LangOptions::GCOnly) {
       // fallthrough

clang/test/CodeGenObjC/arc-blocks.m

 @synthesize ablock, nblock;
 // CHECK:    define internal void ()* @"\01-[Test12 ablock]"(
 // CHECK:    call i8* @objc_getProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i1 zeroext true)
 
 // CHECK:    define internal void @"\01-[Test12 setAblock:]"(
 // CHECK:    call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i8* {{%.*}}, i1 zeroext true, i1 zeroext true)
 
 // CHECK:    define internal void ()* @"\01-[Test12 nblock]"(
-// CHECK:    call i8* @objc_getProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i1 zeroext false)
+// CHECK:    %add.ptr = getelementptr inbounds i8, i8* %1, i64 %ivar
 
 // CHECK:    define internal void @"\01-[Test12 setNblock:]"(
 // CHECK:    call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i8* {{%.*}}, i1 zeroext false, i1 zeroext true)
 @end
 
 // rdar://problem/10131784
 void test13(id x) {
   extern void test13_helper(id);