Skip to content

Use known instance size and alignment across module boundaries #18464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 3, 2018
Merged

Use known instance size and alignment across module boundaries #18464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
864d9cd
IRGen: Add a new ClassLayout::HasFixedSize and use it
slavapestov Aug 2, 2018
9e15390
IRGen: Don't visit superclass in classHasIncompleteLayout() and make …
slavapestov Aug 2, 2018
bee6b09
IRGen: Remove emitClassFragileInstanceSizeAndAlignMask()
slavapestov Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 41 additions & 66 deletions lib/IRGen/GenClass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@ static SILType getSelfType(ClassDecl *base) {
return SILType::getPrimitiveObjectType(loweredTy);
}

/// If the superclass came from another module, we may have dropped
/// stored properties due to the Swift language version availability of
/// their types. In these cases we can't precisely lay out the ivars in
/// the class object at compile time so we need to do runtime layout.
static bool classHasIncompleteLayout(IRGenModule &IGM,
ClassDecl *theClass) {
if (theClass->getParentModule() == IGM.getSwiftModule())
return false;

for (auto field : theClass->getStoredPropertiesAndMissingMemberPlaceholders())
if (isa<MissingMemberDecl>(field))
return true;

return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about resilient modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resilience is handled elsewhere -- there's an isResilient() check too. I didn't want to conflate it with 'has missing members'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Do we have enough information to assert that here? That would assuage my fears of doing the wrong thing after a refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but we effectively call both classHasIncompleteLayout() and isResilient() and OR the results together. I'll see about adding better asserts as I refactor the code more; for now I don't want to change classHasIncompleteLayout(), I'm just moving it to a different place in the file.

}

namespace {
class ClassLayoutBuilder : public StructLayoutBuilder {
SmallVector<ElementLayout, 8> Elements;
Expand Down Expand Up @@ -229,6 +245,7 @@ namespace {
fieldLayout.AllFieldAccesses = IGM.Context.AllocateCopy(AllFieldAccesses);
fieldLayout.MetadataRequiresDynamicInitialization =
ClassMetadataRequiresDynamicInitialization;
fieldLayout.HasFixedSize = ClassHasFixedSize;
return fieldLayout;
}

Expand All @@ -237,9 +254,6 @@ namespace {
if (theClass->isGenericContext())
ClassMetadataRequiresDynamicInitialization = true;

if (IGM.isResilient(theClass, ResilienceExpansion::Maximal))
ClassMetadataRequiresDynamicInitialization = true;

if (theClass->hasSuperclass()) {
SILType superclassType = classType.getSuperclass();
auto superclass = superclassType.getClassOrBoundGenericClass();
Expand All @@ -251,6 +265,7 @@ namespace {
// the class object at compile time so we need to do runtime layout.
if (classHasIncompleteLayout(IGM, superclass)) {
ClassMetadataRequiresDynamicInitialization = true;
ClassHasFixedSize = false;
}

if (superclass->hasClangNode()) {
Expand Down Expand Up @@ -282,9 +297,6 @@ namespace {
}
} else if (IGM.isResilient(superclass, ResilienceExpansion::Maximal)) {
ClassMetadataRequiresDynamicInitialization = true;

// If the superclass is resilient to us, we cannot statically
// know the layout of either its instances or its class objects.
ClassHasFixedSize = false;

// Furthermore, if the superclass is a generic context, we have to
Expand All @@ -310,6 +322,11 @@ namespace {
ClassHasFixedSize = false;
}

if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
ClassMetadataRequiresDynamicInitialization = true;
ClassHasFixedSize = false;
}

// Access strategies should be set by the abstract class layout,
// not using the concrete type information we have.
const ClassLayout *abstractLayout = nullptr;
Expand Down Expand Up @@ -657,7 +674,7 @@ Address irgen::emitTailProjection(IRGenFunction &IGF, llvm::Value *Base,
} else {
llvm::Value *metadata = emitHeapMetadataRefForHeapObject(IGF, Base,
ClassType);
Offset = emitClassFragileInstanceSizeAndAlignMask(IGF,
Offset = emitClassResilientInstanceSizeAndAlignMask(IGF,
ClassType.getClassOrBoundGenericClass(),
metadata).first;
}
Expand Down Expand Up @@ -789,17 +806,25 @@ llvm::Value *irgen::emitClassAllocation(IRGenFunction &IGF, SILType selfType,
emitClassHeapMetadataRef(IGF, classType, MetadataValueType::TypeMetadata,
MetadataState::Complete);

// FIXME: Long-term, we clearly need a specialized runtime entry point.
const StructLayout &structLayout = classTI.getLayout(IGF.IGM, selfType);
const ClassLayout &classLayout = classTI.getClassLayout(IGF.IGM, selfType);

llvm::Value *size, *alignMask;
std::tie(size, alignMask)
= emitClassFragileInstanceSizeAndAlignMask(IGF,
selfType.getClassOrBoundGenericClass(),
metadata);
if (classLayout.HasFixedSize) {
assert(structLayout.isFixedLayout());

const StructLayout &layout = classTI.getLayout(IGF.IGM, selfType);
llvm::Type *destType = layout.getType()->getPointerTo();
size = structLayout.emitSize(IGF.IGM);
alignMask = structLayout.emitAlignMask(IGF.IGM);
} else {
std::tie(size, alignMask)
= emitClassResilientInstanceSizeAndAlignMask(IGF,
selfType.getClassOrBoundGenericClass(),
metadata);
}

llvm::Type *destType = structLayout.getType()->getPointerTo();
llvm::Value *val = nullptr;
if (llvm::Value *Promoted = stackPromote(IGF, layout, StackAllocSize,
if (llvm::Value *Promoted = stackPromote(IGF, structLayout, StackAllocSize,
TailArrays)) {
val = IGF.Builder.CreateBitCast(Promoted, IGF.IGM.RefCountedPtrTy);
val = IGF.emitInitStackObjectCall(metadata, val, "reference.new");
Expand Down Expand Up @@ -863,7 +888,7 @@ static void getInstanceSizeAndAlignMask(IRGenFunction &IGF,
llvm::Value *metadata =
emitHeapMetadataRefForHeapObject(IGF, selfValue, selfType);
std::tie(size, alignMask)
= emitClassFragileInstanceSizeAndAlignMask(IGF, selfClass, metadata);
= emitClassResilientInstanceSizeAndAlignMask(IGF, selfClass, metadata);
}

void irgen::emitClassDeallocation(IRGenFunction &IGF, SILType selfType,
Expand Down Expand Up @@ -2225,26 +2250,6 @@ ClassDecl *irgen::getRootClassForMetaclass(IRGenModule &IGM, ClassDecl *C) {
IGM.Context.Id_SwiftObject);
}

/// If the superclass came from another module, we may have dropped
/// stored properties due to the Swift language version availability of
/// their types. In these cases we can't precisely lay out the ivars in
/// the class object at compile time so we need to do runtime layout.
bool irgen::classHasIncompleteLayout(IRGenModule &IGM,
ClassDecl *theClass) {
do {
if (theClass->getParentModule() != IGM.getSwiftModule()) {
for (auto field :
theClass->getStoredPropertiesAndMissingMemberPlaceholders()){
if (isa<MissingMemberDecl>(field)) {
return true;
}
}
return false;
}
} while ((theClass = theClass->getSuperclassDecl()));
return false;
}

bool irgen::doesClassMetadataRequireDynamicInitialization(IRGenModule &IGM,
ClassDecl *theClass) {
// Classes imported from Objective-C never requires dynamic initialization.
Expand Down Expand Up @@ -2284,36 +2289,6 @@ bool irgen::hasKnownSwiftMetadata(IRGenModule &IGM, ClassDecl *theClass) {
return theClass->hasKnownSwiftImplementation();
}

/// Given a reference to class metadata of the given type,
/// load the fragile instance size and alignment of the class.
std::pair<llvm::Value *, llvm::Value *>
irgen::emitClassFragileInstanceSizeAndAlignMask(IRGenFunction &IGF,
ClassDecl *theClass,
llvm::Value *metadata) {
// FIXME: The below checks should capture this property already, but
// resilient class metadata layout is not fully implemented yet.
auto superClass = theClass;
do {
if (superClass->getParentModule() != IGF.IGM.getSwiftModule()) {
return emitClassResilientInstanceSizeAndAlignMask(IGF, theClass,
metadata);
}
} while ((superClass = superClass->getSuperclassDecl()));

// If the class has fragile fixed layout, return the constant size and
// alignment.
if (llvm::Constant *size
= tryEmitClassConstantFragileInstanceSize(IGF.IGM, theClass)) {
llvm::Constant *alignMask
= tryEmitClassConstantFragileInstanceAlignMask(IGF.IGM, theClass);
assert(alignMask && "static size without static align");
return {size, alignMask};
}

// Otherwise, load it from the metadata.
return emitClassResilientInstanceSizeAndAlignMask(IGF, theClass, metadata);
}

std::pair<llvm::Value *, llvm::Value *>
irgen::emitClassResilientInstanceSizeAndAlignMask(IRGenFunction &IGF,
ClassDecl *theClass,
Expand Down
14 changes: 0 additions & 14 deletions lib/IRGen/GenClass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,20 +170,6 @@ namespace irgen {
/// the runtime?
bool doesClassMetadataRequireDynamicInitialization(IRGenModule &IGM,
ClassDecl *theClass);

/// If the superclass came from another module, we may have dropped
/// stored properties due to the Swift language version availability of
/// their types. In these cases we can't precisely lay out the ivars in
/// the class object at compile time so we need to do runtime layout.
bool classHasIncompleteLayout(IRGenModule &IGM,
ClassDecl *theClass);

/// Load the fragile instance size and alignment mask from a reference to
/// class type metadata of the given type.
std::pair<llvm::Value *, llvm::Value *>
emitClassFragileInstanceSizeAndAlignMask(IRGenFunction &IGF,
ClassDecl *theClass,
llvm::Value *metadata);

/// Load the instance size and alignment mask from a reference to
/// class type metadata of the given type.
Expand Down
8 changes: 7 additions & 1 deletion lib/IRGen/StructLayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,14 @@ struct ClassLayout {
ArrayRef<VarDecl*> InheritedStoredProperties;
/// Lazily-initialized array of all field access methods.
ArrayRef<FieldAccess> AllFieldAccesses;
/// Does the class metadata require dynamic initialization.
/// Does the class metadata require dynamic initialization?
bool MetadataRequiresDynamicInitialization;
/// Do instances of this class have a size and layout known at compile time?
///
/// Note: This is a stronger condition than the StructLayout of a class having
/// a fixed layout. The latter is true even when the class requires sliding
/// ivars by the Objective-C runtime.
bool HasFixedSize;

unsigned getFieldIndex(VarDecl *field) const {
// FIXME: This is algorithmically terrible.
Expand Down
59 changes: 40 additions & 19 deletions test/IRGen/mixed_mode_class_with_unimportable_fields.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %empty-directory(%t)
// 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
// 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
// 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
// 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
// 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-V5 --check-prefix=CHECK-V5-%target-ptrsize -DWORD=i%target-ptrsize
// 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-V5 --check-prefix=CHECK-V5-%target-ptrsize -DWORD=i%target-ptrsize

// REQUIRES: objc_interop

Expand All @@ -24,24 +24,45 @@ sil_vtable SubSubButtHolder {}
// CHECK-LABEL: define {{.*}} @getHolder
sil @getHolder: $@convention(thin) () -> @owned ButtHolder {
entry:
// We should load the dimensions of the class instance from metadata, not try
// to hardcode constants.
// CHECK: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S14UsingObjCStuff10ButtHolderCMa"([[WORD]] 0)
// CHECK: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-64: [[SIZE32:%.*]] = load i32
// CHECK-64: [[SIZE:%.*]] = zext i32 [[SIZE32]] to
// CHECK-32: [[SIZE:%.*]] = load i32
// CHECK: [[ALIGN16:%.*]] = load i16
// CHECK: [[ALIGN:%.*]] = zext i16 [[ALIGN16]] to [[WORD]]
// CHECK: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] [[SIZE]], [[WORD]] [[ALIGN]])

// In Swift 4 mode, we don't know the size or alignment of ButtHolder
// instances, so we should load the dimensions of the class instance
// from metadata.
//
// In Swift 5 mode, it's okay to hardcode constants.

// CHECK-V4: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S14UsingObjCStuff10ButtHolderCMa"([[WORD]] 0)
// CHECK-V4: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-V4-64: [[SIZE32:%.*]] = load i32
// CHECK-V4-64: [[SIZE:%.*]] = zext i32 [[SIZE32]] to
// CHECK-V4-32: [[SIZE:%.*]] = load i32
// CHECK-V4: [[ALIGN16:%.*]] = load i16
// CHECK-V4: [[ALIGN:%.*]] = zext i16 [[ALIGN16]] to [[WORD]]
// CHECK-V4: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] [[SIZE]], [[WORD]] [[ALIGN]])

// CHECK-V5: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S14UsingObjCStuff10ButtHolderCMa"([[WORD]] 0)
// CHECK-V5: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-V5-32: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] 28, [[WORD]] 3)
// CHECK-V5-64: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] 48, [[WORD]] 7)

%x = alloc_ref $ButtHolder
// CHECK: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S4main13SubButtHolderCMa"([[WORD]] 0)
// CHECK: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] %{{.*}}, [[WORD]] %{{.*}})
// CHECK-V4: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S4main13SubButtHolderCMa"([[WORD]] 0)
// CHECK-V4: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-V4: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] %{{.*}}, [[WORD]] %{{.*}})

// CHECK-V5: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S4main13SubButtHolderCMa"([[WORD]] 0)
// CHECK-V5: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-V5-32: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] 40, [[WORD]] 7)
// CHECK-V5-64: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] 56, [[WORD]] 7)
%y = alloc_ref $SubButtHolder
// CHECK: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S4main03SubB10ButtHolderCMa"([[WORD]] 0)
// CHECK: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] %{{.*}}, [[WORD]] %{{.*}})
// CHECK-V4: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S4main03SubB10ButtHolderCMa"([[WORD]] 0)
// CHECK-V4: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-V4: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] %{{.*}}, [[WORD]] %{{.*}})

// CHECK-V5: [[TMP:%.*]] = call swiftcc %swift.metadata_response @"$S4main03SubB10ButtHolderCMa"([[WORD]] 0)
// CHECK-V5: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[TMP]], 0
// CHECK-V5-32: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] 48, [[WORD]] 7)
// CHECK-V5-64: call noalias %swift.refcounted* @swift_allocObject(%swift.type* [[METADATA]], [[WORD]] 64, [[WORD]] 7)
%z = alloc_ref $SubSubButtHolder
return %x : $ButtHolder
}