[CodeGenObjC] Treat ivar offsets variables as constant if they refer to ivars of a direct subclass of NSObject with an @implementation

[erik.pilkington]

Summary

This patch was originally written by Pete Cooper.

This is possible because the size of NSObject is effectively ABI, and will not change in the future. Doing this allows LLVM to avoid loading the offset.

Thanks,
Erik

[rjmccall]

We've tossed around the idea of doing things like this before, but I was hoping that it wouldn't have to be specific to NSObject and that we'd e.g. have an attribute that guarantees that the @interface declares all the ivars for a class. Are we still thinking that? Even if this is just a step towards that, I think we should at least add a method to answer whether we have a statically-known layout for the class, rather than hard-coding that decision at the leaves like this.

[rjmccall]

Also, we should just go ahead and skip using the offset variable when emitting the ivar access in the frontend.

[dexonsmith]

In D56802#1360316, @rjmccall wrote:

We've tossed around the idea of doing things like this before, but I was hoping that it wouldn't have to be specific to NSObject and that we'd e.g. have an attribute that guarantees that the @interface declares all the ivars for a class. Are we still thinking that?

The attribute idea hasn't yet survived language discussions (too much of an ABI foot-gun for library evolution). But it's safe to move forward with something NSObject-specific since it's already baked into the ABI.

Even if this is just a step towards that, I think we should at least add a method to answer whether we have a statically-known layout for the class, rather than hard-coding that decision at the leaves like this.

A method makes sense to me too.

[erik.pilkington]

Address review comments: move the check to a function, and call it when emitting the ivar offset instead of when we emit the global. Thanks!

[rjmccall]

Emitting the global as const is probably still a good idea; if the global actually gets mapped as a constant, we'll end up making a dynamic assertion that we got the offset right. It's possible that the section will inhibit that, though.

[erik.pilkington]

Sure, that makes sense. The new patch makes the global constant again.

[rjmccall]

LGTM

clang/lib/CodeGen/CGObjCMac.cpp

     if (const ObjCMethodDecl *MD =
           dyn_cast_or_null<ObjCMethodDecl>(CGF.CurFuncDecl))
       if (MD->isInstanceMethod())
         if (const ObjCInterfaceDecl *ID = MD->getClassInterface())
           return IV->getContainingInterface()->isSuperClassOf(ID);
     return false;
   }
 
+  bool isClassLayoutKnownStatically(const ObjCInterfaceDecl *ID) {
+    // NSObject is a fixed size. If we can see the @implementation of a class
+    // which inherits from NSObject then we know that all it's offsets also must
+    // be fixed. FIXME: Can we do this if see a chain of super classes with
+    // implementations leading to NSObject?
+    return ID->getImplementation() && ID->getSuperClass() &&
+           ID->getSuperClass()->getName() == "NSObject";
+  }
+
 public:
   CGObjCNonFragileABIMac(CodeGen::CodeGenModule &cgm);
 
   llvm::Constant *getNSConstantStringClassRef() override;
 
   llvm::Function *ModuleInitFunction() override;
 
   CodeGen::RValue GenerateMessageSend(CodeGen::CodeGenFunction &CGF,

     if (Ivar->getAccessControl() == ObjCIvarDecl::Private ||
         Ivar->getAccessControl() == ObjCIvarDecl::Package ||
         ID->getVisibility() == HiddenVisibility)
       IvarOffsetGV->setVisibility(llvm::GlobalValue::HiddenVisibility);
     else
       IvarOffsetGV->setVisibility(llvm::GlobalValue::DefaultVisibility);
   }
 
+  // If ID's layout is known, then make the global constant. This serves as a
+  // useful assertion: we'll never use this variable to calculate ivar offsets,
+  // so if the runtime tries to patch it then we should crash.
+  if (isClassLayoutKnownStatically(ID))
+    IvarOffsetGV->setConstant(true);
+
   if (CGM.getTriple().isOSBinFormatMachO())
     IvarOffsetGV->setSection("__DATA, __objc_ivar");
   return IvarOffsetGV;
 }
 
 /// EmitIvarList - Emit the ivar list for the given
 /// implementation. The return value has type
 /// IvarListnfABIPtrTy.

                                                const ObjCIvarDecl *Ivar,
                                                unsigned CVRQualifiers) {
   ObjCInterfaceDecl *ID = ObjectTy->getAs<ObjCObjectType>()->getInterface();
   llvm::Value *Offset = EmitIvarOffset(CGF, ID, Ivar);
   return EmitValueForIvarAtOffset(CGF, ID, BaseValue, Ivar, CVRQualifiers,
                                   Offset);
 }
 
-llvm::Value *CGObjCNonFragileABIMac::EmitIvarOffset(
-  CodeGen::CodeGenFunction &CGF,
+llvm::Value *
+CGObjCNonFragileABIMac::EmitIvarOffset(CodeGen::CodeGenFunction &CGF,
                                        const ObjCInterfaceDecl *Interface,
                                        const ObjCIvarDecl *Ivar) {
-  llvm::Value *IvarOffsetValue = ObjCIvarOffsetVariable(Interface, Ivar);
-  IvarOffsetValue = CGF.Builder.CreateAlignedLoad(IvarOffsetValue,
-                                                  CGF.getSizeAlign(), "ivar");
-  if (IsIvarOffsetKnownIdempotent(CGF, Ivar))
-    cast<llvm::LoadInst>(IvarOffsetValue)
-        ->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
-                      llvm::MDNode::get(VMContext, None));
+  llvm::Value *IvarOffsetValue;
+  if (isClassLayoutKnownStatically(Interface)) {
+    IvarOffsetValue = llvm::ConstantInt::get(
+        ObjCTypes.IvarOffsetVarTy,
+        ComputeIvarBaseOffset(CGM, Interface->getImplementation(), Ivar));
+  } else {
+    llvm::GlobalVariable *GV = ObjCIvarOffsetVariable(Interface, Ivar);
+    IvarOffsetValue =
+        CGF.Builder.CreateAlignedLoad(GV, CGF.getSizeAlign(), "ivar");
+    if (IsIvarOffsetKnownIdempotent(CGF, Ivar))
+      cast<llvm::LoadInst>(IvarOffsetValue)
+          ->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
+                        llvm::MDNode::get(VMContext, None));
+  }
 
   // This could be 32bit int or 64bit integer depending on the architecture.
   // Cast it to 64bit integer value, if it is a 32bit integer ivar offset value
   //  as this is what caller always expects.
   if (ObjCTypes.IvarOffsetVarTy == ObjCTypes.IntTy)
     IvarOffsetValue = CGF.Builder.CreateIntCast(
         IvarOffsetValue, ObjCTypes.LongTy, true, "ivar.conv");
   return IvarOffsetValue;

clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m

+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: @"OBJC_IVAR_$_StaticLayout.static_layout_ivar" = hidden constant i64 20
+// CHECK: @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar" = hidden global i64 12
+
+@interface NSObject {
+  int these, will, never, change, ever;
+}
+@end
+
+@interface StaticLayout : NSObject
+@end
+
+@implementation StaticLayout {
+  int static_layout_ivar;
+}
+-(void)meth {
+  static_layout_ivar = 0;
+  // CHECK-NOT: load i64, i64* @"OBJC_IVAR_$_StaticLayout
+}
+@end
+
+@interface NotNSObject {
+  int these, might, change;
+}
+@end
+
+@interface NotStaticLayout : NotNSObject
+@end
+
+@implementation NotStaticLayout {
+  int not_static_layout_ivar;
+}
+-(void)meth {
+  not_static_layout_ivar = 0;
+  // CHECK: load i64, i64* @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar
+}
+@end

clang/test/CodeGenObjC/optimize-ivar-offset-load.m

 // RUN: %clang_cc1 -triple x86_64-apple-darwin10  -O0 -emit-llvm %s -o -  | FileCheck %s
 // rdar://16095748
 
-@interface NSObject 
+@interface MyNSObject 
 @end
 
-@interface SampleClass : NSObject {
+@interface SampleClass : MyNSObject {
     @public
     int _value;
 }
 + (SampleClass*) new;
 @end
 
-@interface AppDelegate  : NSObject
+@interface AppDelegate  : MyNSObject
 @end
 
 extern void foo(int);
 
 @implementation AppDelegate
 - (void)application
 {
     // Create set of objects in loop

clang/test/CodeGenObjC/reorder-synthesized-ivars.m

 {
   id MyIvar;
   BOOL _MyBool;
   char * pc;
 }
 @end
 
 // CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean1
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean2
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean3
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean4
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean5
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean6
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean7
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean8
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean9
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object1
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object2
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object3
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object4
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object5
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object6
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object7
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object8
-// CHECK-NEXT: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object9
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean2
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean3
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean4
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean5
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean6
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean7
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean8
+// CHECK: @{{.*}} = private unnamed_addr constant [10 x i8] c"_boolean9
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object1
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object2
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object3
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object4
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object5
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object6
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object7
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object8
+// CHECK: @{{.*}} = private unnamed_addr constant [9 x i8] c"_object9