[ObjC] Expand isClassLayoutKnownStatically to base classes as long as the implementation of it is known #85465

AZero13 · 2024-03-15T20:44:07Z

Only NSObject we can trust the layout of won't change even though we cannot directly see its @implementation

llvmbot · 2024-03-15T20:44:38Z

@llvm/pr-subscribers-clang-codegen

Author: AtariDreams (AtariDreams)

Changes

Only NSObject we can trust the layout of won't change even though we cannot directly see its @implementation

Full diff: https://github.com/llvm/llvm-project/pull/85465.diff

2 Files Affected:

(modified) clang/lib/CodeGen/CGObjCMac.cpp (+8-1)
(modified) clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m (+29-1)

diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index e815e097e1fb48..da47ef786cf816 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -1595,6 +1595,11 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
   bool isClassLayoutKnownStatically(const ObjCInterfaceDecl *ID) {
     // Test a class by checking its superclasses up to
     // its base class if it has one.
+
+    // Cannot check a null class
+    if (!ID)
+      return false;
+
     for (; ID; ID = ID->getSuperClass()) {
       // The layout of base class NSObject
       // is guaranteed to be statically known
@@ -1606,7 +1611,9 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
       if (!ID->getImplementation())
         return false;
     }
-    return false;
+
+    // We know the layout of all the intermediate classes and superclasses.
+    return true;
   }
 
 public:
diff --git a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
index 8d55e6c7d23081..ee4034e4b7f205 100644
--- a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
+++ b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
@@ -8,6 +8,12 @@
 // CHECK: @"OBJC_IVAR_$_IntermediateClass._intermediateProperty" = hidden constant i64 48
 // CHECK: @"OBJC_IVAR_$_SubClass.subClassIvar" = constant i64 56
 // CHECK: @"OBJC_IVAR_$_SubClass._subClassProperty" = hidden constant i64 64
+
+// CHECK: @"OBJC_IVAR_$_RootClass.these" = constant i64 0
+// CHECK: @"OBJC_IVAR_$_RootClass.dont" = constant i64 4
+// CHECK: @"OBJC_IVAR_$_RootClass.change" = constant i64 4
+// CHECK: @"OBJC_IVAR_$_StillStaticLayout.static_layout_ivar" = hidden global i32 12
+
 // CHECK: @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar" = hidden global i64 12
 
 @interface NSObject {
@@ -120,7 +126,29 @@ -(void)intermediateSubclassVar {
 // CHECK: getelementptr inbounds i8, ptr %1, i64 64
 @end
 
-@interface NotNSObject {
+ __attribute((objc_root_class))  @interface RootClass {
+  int these, dont, change;
+}
+@end
+
+@implementation RootClass 
+@end
+
+@interface StillStaticLayout : RootClass
+@end
+
+@implementation StillStaticLayout {
+  int static_layout_ivar;
+}
+
+// CHECK-LABEL: define internal void @"\01-[StillStaticLayout meth]"
+-(void)meth {
+  static_layout_ivar = 0;
+  // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$StillStaticLayout.static_layout_ivar
+}
+@end
+
+__attribute((objc_root_class))  @interface NotNSObject {
   int these, might, change;
 }
 @end

llvmbot · 2024-03-15T20:44:38Z

@llvm/pr-subscribers-clang

Author: AtariDreams (AtariDreams)

Changes

Only NSObject we can trust the layout of won't change even though we cannot directly see its @implementation

Full diff: https://github.com/llvm/llvm-project/pull/85465.diff

2 Files Affected:

(modified) clang/lib/CodeGen/CGObjCMac.cpp (+8-1)
(modified) clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m (+29-1)

diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index e815e097e1fb48..da47ef786cf816 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -1595,6 +1595,11 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
   bool isClassLayoutKnownStatically(const ObjCInterfaceDecl *ID) {
     // Test a class by checking its superclasses up to
     // its base class if it has one.
+
+    // Cannot check a null class
+    if (!ID)
+      return false;
+
     for (; ID; ID = ID->getSuperClass()) {
       // The layout of base class NSObject
       // is guaranteed to be statically known
@@ -1606,7 +1611,9 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
       if (!ID->getImplementation())
         return false;
     }
-    return false;
+
+    // We know the layout of all the intermediate classes and superclasses.
+    return true;
   }
 
 public:
diff --git a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
index 8d55e6c7d23081..ee4034e4b7f205 100644
--- a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
+++ b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
@@ -8,6 +8,12 @@
 // CHECK: @"OBJC_IVAR_$_IntermediateClass._intermediateProperty" = hidden constant i64 48
 // CHECK: @"OBJC_IVAR_$_SubClass.subClassIvar" = constant i64 56
 // CHECK: @"OBJC_IVAR_$_SubClass._subClassProperty" = hidden constant i64 64
+
+// CHECK: @"OBJC_IVAR_$_RootClass.these" = constant i64 0
+// CHECK: @"OBJC_IVAR_$_RootClass.dont" = constant i64 4
+// CHECK: @"OBJC_IVAR_$_RootClass.change" = constant i64 4
+// CHECK: @"OBJC_IVAR_$_StillStaticLayout.static_layout_ivar" = hidden global i32 12
+
 // CHECK: @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar" = hidden global i64 12
 
 @interface NSObject {
@@ -120,7 +126,29 @@ -(void)intermediateSubclassVar {
 // CHECK: getelementptr inbounds i8, ptr %1, i64 64
 @end
 
-@interface NotNSObject {
+ __attribute((objc_root_class))  @interface RootClass {
+  int these, dont, change;
+}
+@end
+
+@implementation RootClass 
+@end
+
+@interface StillStaticLayout : RootClass
+@end
+
+@implementation StillStaticLayout {
+  int static_layout_ivar;
+}
+
+// CHECK-LABEL: define internal void @"\01-[StillStaticLayout meth]"
+-(void)meth {
+  static_layout_ivar = 0;
+  // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$StillStaticLayout.static_layout_ivar
+}
+@end
+
+__attribute((objc_root_class))  @interface NotNSObject {
   int these, might, change;
 }
 @end

AZero13 · 2024-11-17T22:46:20Z

Going to ping @rjmccall

AZero13 · 2025-02-10T01:34:20Z

@adrian-prantl thoughts on this?

AZero13 · 2025-02-11T15:44:39Z

@davidchisnall thoughts?

clang/lib/CodeGen/CGObjCMac.cpp

ahatanak

We can remove the null check when we prove ID is never null.

AZero13 · 2025-02-25T14:19:48Z

@ahatanak Okay I removed the null check.

AZero13 · 2025-03-03T18:17:14Z

@ahatanak Can we please merge this?

ahatanak · 2025-03-03T22:24:16Z

Can we assert that ID isn't null at the beginning of isClassLayoutKnownStatically?

@implementation

… the implementation of it is known Only NSObject we can trust the layout of won't change even though we cannot directly see its @implementation.

AZero13 · 2025-03-04T14:10:59Z

Can we assert that ID isn't null at the beginning of isClassLayoutKnownStatically?

Done! @ahatanak!

@implementation

… the implementation of it is known (llvm#85465) Only NSObject we can trust the layout of won't change even though we cannot directly see its @implementation

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Mar 15, 2024

AZero13 force-pushed the what-if branch 3 times, most recently from d778404 to 3e11da6 Compare March 16, 2024 23:25

AZero13 force-pushed the what-if branch from 3e11da6 to 1c241dd Compare June 2, 2024 01:48

AZero13 mentioned this pull request Jul 15, 2024

We know the layout of base classes and their subclasses that we can see the @implementation of #98930

Closed

AZero13 force-pushed the what-if branch from 1c241dd to 50f321e Compare July 23, 2024 14:07

AZero13 closed this Sep 10, 2024

AZero13 reopened this Oct 28, 2024

AZero13 force-pushed the what-if branch from 50f321e to 5872f59 Compare October 28, 2024 17:57

AZero13 force-pushed the what-if branch 2 times, most recently from 357b8ca to ffbc1da Compare November 17, 2024 02:21

AZero13 force-pushed the what-if branch from ffbc1da to bd44589 Compare February 10, 2025 01:34

efriedma-quic requested review from davidchisnall, rjmccall and ahatanak February 10, 2025 23:26

AZero13 mentioned this pull request Feb 10, 2025

[CodeGen][ObjC] Invalidate cached ObjC class layout information after parsing ObjC class implementations if new ivars are added to the interface #126591

Merged

ahatanak reviewed Feb 12, 2025

View reviewed changes

clang/lib/CodeGen/CGObjCMac.cpp Outdated Show resolved Hide resolved

ahatanak approved these changes Feb 17, 2025

View reviewed changes

AZero13 force-pushed the what-if branch 4 times, most recently from 0572dd0 to 9cb7c41 Compare February 25, 2025 14:39

[ObjC] Expand isClassLayoutKnownStatically to base classes as long as…

b0eba86

… the implementation of it is known Only NSObject we can trust the layout of won't change even though we cannot directly see its @implementation.

AZero13 force-pushed the what-if branch from 9cb7c41 to b0eba86 Compare March 4, 2025 14:11

ahatanak merged commit 6720465 into llvm:main Mar 4, 2025
11 checks passed

AZero13 deleted the what-if branch March 4, 2025 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ObjC] Expand isClassLayoutKnownStatically to base classes as long as the implementation of it is known #85465

[ObjC] Expand isClassLayoutKnownStatically to base classes as long as the implementation of it is known #85465

Uh oh!

AZero13 commented Mar 15, 2024

Uh oh!

llvmbot commented Mar 15, 2024

Uh oh!

llvmbot commented Mar 15, 2024

Uh oh!

AZero13 commented Nov 17, 2024

Uh oh!

AZero13 commented Feb 10, 2025

Uh oh!

AZero13 commented Feb 11, 2025

Uh oh!

Uh oh!

ahatanak left a comment

Uh oh!

AZero13 commented Feb 25, 2025

Uh oh!

AZero13 commented Mar 3, 2025

Uh oh!

ahatanak commented Mar 3, 2025

Uh oh!

AZero13 commented Mar 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

[ObjC] Expand isClassLayoutKnownStatically to base classes as long as the implementation of it is known #85465

[ObjC] Expand isClassLayoutKnownStatically to base classes as long as the implementation of it is known #85465

Uh oh!

Conversation

AZero13 commented Mar 15, 2024

Uh oh!

llvmbot commented Mar 15, 2024

Uh oh!

llvmbot commented Mar 15, 2024

Uh oh!

AZero13 commented Nov 17, 2024

Uh oh!

AZero13 commented Feb 10, 2025

Uh oh!

AZero13 commented Feb 11, 2025

Uh oh!

Uh oh!

ahatanak left a comment

Choose a reason for hiding this comment

Uh oh!

AZero13 commented Feb 25, 2025

Uh oh!

AZero13 commented Mar 3, 2025

Uh oh!

ahatanak commented Mar 3, 2025

Uh oh!

AZero13 commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AZero13 commented Mar 4, 2025 •

edited

Loading