Skip to content

Commit 8f0fa4b

Browse files
authored
Merge pull request #18463 from slavapestov/field-offset-globals-are-too-resilient
IRGen: Don't unconditionally use field offset globals for cross-module class field access
2 parents 99b1ce7 + eb8aa7f commit 8f0fa4b

File tree

8 files changed

+96
-41
lines changed

8 files changed

+96
-41
lines changed

lib/IRGen/GenClass.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,13 +302,12 @@ namespace {
302302
NumInherited = Elements.size();
303303
}
304304
}
305-
305+
306306
// If this class was imported from another module, assume that we may
307307
// not know its exact layout.
308-
if (theClass->getModuleContext() != IGM.getSwiftModule()) {
308+
if (classHasIncompleteLayout(IGM, theClass)) {
309+
ClassMetadataRequiresDynamicInitialization = true;
309310
ClassHasFixedSize = false;
310-
if (classHasIncompleteLayout(IGM, theClass))
311-
ClassMetadataRequiresDynamicInitialization = true;
312311
}
313312

314313
// Access strategies should be set by the abstract class layout,
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
Name: ObjCStuff
22
SwiftVersions:
3-
- Version: 4
3+
- Version: 5
44
Typedefs:
55
- Name: OJCCloudButt
66
SwiftName: OJCCloud.Butt
7-
- Version: 3
7+
- Version: 4
88
Typedefs:
99
- Name: OJCCloudButt
1010
SwiftWrapper: none

test/IRGen/Inputs/other_class.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
public class Other {
2+
public final var x: Int32 = 0
3+
}
4+
5+
public class Subclass : Other {
6+
public final var y: Int32 = 0
7+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path=%t/other_class.swiftmodule %S/Inputs/other_class.swift
4+
// RUN: %target-swift-frontend -I %t -emit-ir -O %s | %FileCheck %s -DINT=i%target-ptrsize
5+
6+
import other_class
7+
8+
// CHECK-LABEL: define {{(protected )?}}{{(dllexport )?}}swiftcc i32 @"$S24class_field_other_module12getSubclassXys5Int32V0c1_A00F0CF"(%T11other_class8SubclassC* nocapture readonly)
9+
// CHECK-NEXT: entry:
10+
// CHECK-NEXT: %._value = getelementptr inbounds %T11other_class8SubclassC, %T11other_class8SubclassC* %0, [[INT]] 0, i32 1, i32 0
11+
// CHECK-NEXT: %1 = load i32, i32* %._value
12+
// CHECK-NEXT: ret i32 %1
13+
public func getSubclassX(_ o: Subclass) -> Int32 {
14+
return o.x
15+
}
16+
17+
// CHECK-LABEL: define {{(protected )?}}{{(dllexport )?}}swiftcc i32 @"$S24class_field_other_module12getSubclassYys5Int32V0c1_A00F0CF"(%T11other_class8SubclassC* nocapture readonly)
18+
// CHECK-NEXT: entry:
19+
// CHECK-NEXT: %._value = getelementptr inbounds %T11other_class8SubclassC, %T11other_class8SubclassC* %0, [[INT]] 0, i32 2, i32 0
20+
// CHECK-NEXT: %1 = load i32, i32* %._value
21+
// CHECK-NEXT: ret i32 %1
22+
public func getSubclassY(_ o: Subclass) -> Int32 {
23+
return o.y
24+
}

test/IRGen/mixed_mode_class_with_unimportable_fields.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/UsingObjCStuff.swiftmodule -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 4 %S/Inputs/mixed_mode/UsingObjCStuff.swift
3-
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 3 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize -DWORD=i%target-ptrsize
2+
// RUN: %target-swift-frontend -emit-module -o %t/UsingObjCStuff.swiftmodule -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 5 %S/Inputs/mixed_mode/UsingObjCStuff.swift
43
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 4 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize -DWORD=i%target-ptrsize
4+
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 5 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize -DWORD=i%target-ptrsize
55

66
// REQUIRES: objc_interop
77

Lines changed: 53 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/UsingObjCStuff.swiftmodule -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 4 %S/Inputs/mixed_mode/UsingObjCStuff.swift
3-
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 3 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize --check-prefix=CHECK-V3
4-
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 4 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize --check-prefix=CHECK-V4
2+
// RUN: %target-swift-frontend -emit-module -o %t/UsingObjCStuff.swiftmodule -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 5 %S/Inputs/mixed_mode/UsingObjCStuff.swift
3+
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 4 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize --check-prefix=CHECK-V4 -DWORD=i%target-ptrsize
4+
// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 5 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize --check-prefix=CHECK-V5 -DWORD=i%target-ptrsize
55

66
// REQUIRES: objc_interop
77

88
import UsingObjCStuff
99

1010
public class SubButtHolder: ButtHolder {
11-
final var w: Double = 0
11+
final var w: Float = 0
1212

1313
override public func virtual() {}
1414

@@ -20,39 +20,64 @@ public class SubSubButtHolder: SubButtHolder {
2020
public override func subVirtual() {}
2121
}
2222

23-
// CHECK-LABEL: define {{.*}} @{{.*}}accessFinalFields
23+
// CHECK-LABEL: define {{(protected )?}}{{(dllexport )?}}swiftcc void @"$S4main17accessFinalFields2ofyp_ypt14UsingObjCStuff10ButtHolderC_tF"
2424
public func accessFinalFields(of holder: ButtHolder) -> (Any, Any) {
25-
// x and z came from the other module, so we should use their accessors.
26-
// CHECK: [[OFFSET:%.*]] = load [[WORD:i[0-9]+]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1xSivpWvd"
27-
// CHECK: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
28-
// CHECK: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
29-
30-
// CHECK: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1zSSvpWvd"
31-
// CHECK: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
32-
// CHECK: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
25+
26+
// ButtHolder.y cannot be imported in Swift 4 mode, so make sure we use field
27+
// offset globals here.
28+
29+
// CHECK-V4: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1xSivpWvd"
30+
// CHECK-V4: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
31+
// CHECK-V4: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
32+
33+
// CHECK-V4: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1zSSvpWvd"
34+
// CHECK-V4: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
35+
// CHECK-V4: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
36+
37+
// ButtHolder.y is correctly imported in Swift 5 mode, so we can use fixed offsets.
38+
39+
// CHECK-V5: [[OFFSET:%.*]] = getelementptr inbounds %T14UsingObjCStuff10ButtHolderC, %T14UsingObjCStuff10ButtHolderC* %2, i32 0, i32 1
40+
41+
// CHECK-V5: [[OFFSET:%.*]] = getelementptr inbounds %T14UsingObjCStuff10ButtHolderC, %T14UsingObjCStuff10ButtHolderC* %2, i32 0, i32 3
42+
3343
return (holder.x, holder.z)
3444
}
3545

36-
// CHECK-LABEL: define {{.*}} @{{.*}}accessFinalFields
46+
// CHECK-LABEL: define {{(protected )?}}{{(dllexport )?}}swiftcc void @"$S4main17accessFinalFields5ofSubyp_ypyptAA0F10ButtHolderC_tF"
3747
public func accessFinalFields(ofSub holder: SubButtHolder) -> (Any, Any, Any) {
3848
// We should use the runtime-adjusted ivar offsets since we may not have
3949
// a full picture of the layout in mixed Swift language version modes.
40-
// CHECK: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1xSivpWvd"
41-
// CHECK: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
42-
// CHECK: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
4350

44-
// CHECK: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1zSSvpWvd"
45-
// CHECK: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
46-
// CHECK: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
51+
// ButtHolder.y cannot be imported in Swift 4 mode, so make sure we use field
52+
// offset globals here.
53+
54+
// CHECK-V4: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1xSivpWvd"
55+
// CHECK-V4: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
56+
// CHECK-V4: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
57+
58+
// CHECK-V4: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S14UsingObjCStuff10ButtHolderC1zSSvpWvd"
59+
// CHECK-V4: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
60+
// CHECK-V4: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
61+
62+
// CHECK-V4: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S4main13SubButtHolderC1wSfvpWvd"
63+
64+
// CHECK-V4: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
65+
// CHECK-V4: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
66+
67+
// ButtHolder.y is correctly imported in Swift 5 mode, so we can use fixed offsets.
68+
69+
// CHECK-V5: [[SELF:%.*]] = bitcast %T4main13SubButtHolderC* %3 to %T14UsingObjCStuff10ButtHolderC*
70+
// CHECK-V5: [[OFFSET:%.*]] = getelementptr inbounds %T14UsingObjCStuff10ButtHolderC, %T14UsingObjCStuff10ButtHolderC* [[SELF]], i32 0, i32 1
71+
72+
// CHECK-V5: [[SELF:%.*]] = bitcast %T4main13SubButtHolderC* %3 to %T14UsingObjCStuff10ButtHolderC*
73+
// CHECK-V5: [[OFFSET:%.*]] = getelementptr inbounds %T14UsingObjCStuff10ButtHolderC, %T14UsingObjCStuff10ButtHolderC* [[SELF]], i32 0, i32 3
4774

48-
// CHECK: [[OFFSET:%.*]] = load [[WORD]], [[WORD]]* @"$S4main13SubButtHolderC1wSdvpWvd"
75+
// CHECK-V5: [[OFFSET:%.*]] = getelementptr inbounds %T4main13SubButtHolderC, %T4main13SubButtHolderC* %3, i32 0, i32 4
4976

50-
// CHECK: [[INSTANCE_RAW:%.*]] = bitcast {{.*}} to i8*
51-
// CHECK: getelementptr inbounds i8, i8* [[INSTANCE_RAW]], [[WORD]] [[OFFSET]]
5277
return (holder.x, holder.z, holder.w)
5378
}
5479

55-
// CHECK-LABEL: define {{.*}} @{{.*}}invokeMethod
80+
// CHECK-LABEL: define {{(protected )?}}{{(dllexport )?}}swiftcc void @"$S4main12invokeMethod2onyAA13SubButtHolderC_tF"
5681
public func invokeMethod(on holder: SubButtHolder) {
5782
// CHECK-64: [[IMPL_ADDR:%.*]] = getelementptr inbounds {{.*}}, [[WORD]] 10
5883
// CHECK-32: [[IMPL_ADDR:%.*]] = getelementptr inbounds {{.*}}, [[WORD]] 13
@@ -66,9 +91,9 @@ public func invokeMethod(on holder: SubButtHolder) {
6691
holder.subVirtual()
6792
}
6893

69-
// CHECK-V3-LABEL: define private void @initialize_metadata_SubButtHolder
70-
// CHECK-V3: call void @swift_initClassMetadata(
94+
// CHECK-V4-LABEL: define private void @initialize_metadata_SubButtHolder
95+
// CHECK-V4: call void @swift_initClassMetadata(
7196

72-
// CHECK-V3-LABEL: define private void @initialize_metadata_SubSubButtHolder
73-
// CHECK-V3: call void @swift_initClassMetadata(
97+
// CHECK-V4-LABEL: define private void @initialize_metadata_SubSubButtHolder
98+
// CHECK-V4: call void @swift_initClassMetadata(
7499

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
Name: ObjCStuff
22
SwiftVersions:
3-
- Version: 4
3+
- Version: 5
44
Typedefs:
55
- Name: OJCCloudButt
66
SwiftName: OJCCloud.Butt
7-
- Version: 3
7+
- Version: 4
88
Typedefs:
99
- Name: OJCCloudButt
1010
SwiftWrapper: none

test/Interpreter/SDK/mixed_mode_class_with_missing_properties.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// RUN: %empty-directory(%t)
22
// RUN: cp %s %t/main.swift
3-
// RUN: %target-build-swift -whole-module-optimization -emit-module-path %t/UsingObjCStuff.swiftmodule -c -o %t/UsingObjCStuff.o -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 4 -parse-as-library %S/Inputs/mixed_mode/UsingObjCStuff.swift
4-
// RUN: %target-build-swift -o %t/a.out.v3 -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 3 %t/main.swift %t/UsingObjCStuff.o
3+
// RUN: %target-build-swift -whole-module-optimization -emit-module-path %t/UsingObjCStuff.swiftmodule -c -o %t/UsingObjCStuff.o -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 5 -parse-as-library %S/Inputs/mixed_mode/UsingObjCStuff.swift
54
// RUN: %target-build-swift -o %t/a.out.v4 -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 4 %t/main.swift %t/UsingObjCStuff.o
6-
// RUN: %target-run %t/a.out.v3 | %FileCheck %s
5+
// RUN: %target-build-swift -o %t/a.out.v5 -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 5 %t/main.swift %t/UsingObjCStuff.o
76
// RUN: %target-run %t/a.out.v4 | %FileCheck %s
7+
// RUN: %target-run %t/a.out.v5 | %FileCheck %s
88

99
// REQUIRES: objc_interop
1010
// REQUIRES: executable_test

0 commit comments

Comments
 (0)