Skip to content

Resilient witness table emission cleanup #22775

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion include/swift/ABI/Metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -2039,7 +2039,8 @@ struct TargetGenericWitnessTable {
return WitnessTablePrivateSizeInWordsAndRequiresInstantiation >> 1;
}

/// Whether the witness table is known to require instantiation.
/// This bit doesn't really mean anything. Currently, the compiler always
/// sets it when emitting a generic witness table.
uint16_t requiresInstantiation() const {
return WitnessTablePrivateSizeInWordsAndRequiresInstantiation & 0x01;
}
Expand Down
9 changes: 2 additions & 7 deletions lib/IRGen/ConformanceDescription.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ class ConformanceDescription {
/// Whether this witness table requires runtime specialization.
const unsigned requiresSpecialization : 1;

/// Whether this witness table contains dependent associated type witnesses.
const unsigned hasDependentAssociatedTypeWitnesses : 1;

/// The instantiation function, to be run at the end of witness table
/// instantiation.
llvm::Constant *instantiationFn = nullptr;
Expand All @@ -66,13 +63,11 @@ class ConformanceDescription {
llvm::Constant *pattern,
uint16_t witnessTableSize,
uint16_t witnessTablePrivateSize,
bool requiresSpecialization,
bool hasDependentAssociatedTypeWitnesses)
bool requiresSpecialization)
: conformance(conformance), wtable(wtable), pattern(pattern),
witnessTableSize(witnessTableSize),
witnessTablePrivateSize(witnessTablePrivateSize),
requiresSpecialization(requiresSpecialization),
hasDependentAssociatedTypeWitnesses(hasDependentAssociatedTypeWitnesses)
requiresSpecialization(requiresSpecialization)
{
}
};
Expand Down
45 changes: 11 additions & 34 deletions lib/IRGen/GenProto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,6 @@ static bool hasDependentTypeWitness(
static bool isDependentConformance(
IRGenModule &IGM,
const RootProtocolConformance *rootConformance,
bool considerResilience,
llvm::SmallPtrSet<const NormalProtocolConformance *, 4> &visited){
// Self-conformances are never dependent.
auto conformance = dyn_cast<NormalProtocolConformance>(rootConformance);
Expand All @@ -1024,7 +1023,7 @@ static bool isDependentConformance(
return false;

// If the conformance is resilient, this is always true.
if (considerResilience && IGM.isResilientConformance(conformance))
if (IGM.isResilientConformance(conformance))
return true;

// Check whether any of the conformances are dependent.
Expand All @@ -1043,7 +1042,6 @@ static bool isDependentConformance(
isDependentConformance(IGM,
assocConformance.getConcrete()
->getRootConformance(),
considerResilience,
visited))
return true;
}
Expand All @@ -1060,11 +1058,9 @@ static bool isDependentConformance(
/// Is there anything about the given conformance that requires witness
/// tables to be dependently-generated?
bool IRGenModule::isDependentConformance(
const RootProtocolConformance *conformance,
bool considerResilience) {
const RootProtocolConformance *conformance) {
llvm::SmallPtrSet<const NormalProtocolConformance *, 4> visited;
return ::isDependentConformance(*this, conformance, considerResilience,
visited);
return ::isDependentConformance(*this, conformance, visited);
}

static bool isSynthesizedNonUnique(const RootProtocolConformance *conformance) {
Expand Down Expand Up @@ -1284,7 +1280,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
// offsets, with conditional conformances closest to 0.
unsigned NextPrivateDataIndex = 0;
bool ResilientConformance;
bool RequiresSpecialization = false;

const ProtocolInfo &PI;

Expand All @@ -1306,21 +1301,14 @@ class AccessorConformanceInfo : public ConformanceInfo {
PI(IGM.getProtocolInfo(SILWT->getConformance()->getProtocol(),
(ResilientConformance
? ProtocolInfoKind::RequirementSignature
: ProtocolInfoKind::Full))) {
// If the conformance is resilient, we require runtime instantiation.
if (ResilientConformance)
RequiresSpecialization = true;
}
: ProtocolInfoKind::Full))) {}

/// The number of entries in the witness table.
unsigned getTableSize() const { return TableSize; }

/// The number of private entries in the witness table.
unsigned getTablePrivateSize() const { return NextPrivateDataIndex; }

/// Whether this witness table must be specialized at runtime.
bool requiresSpecialization() const { return RequiresSpecialization; }

/// The top-level entry point.
void build();

Expand Down Expand Up @@ -1370,7 +1358,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
}

// Otherwise, we'll need to derive it at instantiation time.
RequiresSpecialization = true;
SpecializedBaseConformances.push_back({Table.size(), &conf});
Table.addNullPointer(IGM.Int8PtrTy);
}
Expand Down Expand Up @@ -1437,8 +1424,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
auto associate =
Conformance.getTypeWitness(requirement.getAssociation(), nullptr)
->getCanonicalType();
if (associate->hasTypeParameter())
RequiresSpecialization = true;
llvm::Constant *witness =
IGM.getAssociatedTypeWitness(associate, /*inProtocolContext=*/false);
Table.addBitCast(witness, IGM.Int8PtrTy);
Expand All @@ -1463,9 +1448,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
requirement.getAssociation(),
requirement.getAssociatedRequirement());

if (requirement.getAssociation()->hasTypeParameter())
RequiresSpecialization = true;

#ifndef NDEBUG
assert(entry.getKind() == SILWitnessTable::AssociatedTypeProtocol
&& "sil witness table does not match protocol");
Expand Down Expand Up @@ -1516,7 +1498,6 @@ class AccessorConformanceInfo : public ConformanceInfo {

/// Allocate another word of private data storage in the conformance table.
unsigned getNextPrivateDataIndex() {
RequiresSpecialization = true;
return NextPrivateDataIndex++;
}

Expand Down Expand Up @@ -2055,7 +2036,7 @@ namespace {
// WitnessTablePrivateSizeInWordsAndRequiresInstantiation
B.addInt(IGM.Int16Ty,
(Description.witnessTablePrivateSize << 1) |
Description.hasDependentAssociatedTypeWitnesses);
Description.requiresSpecialization);
// Instantiation function
B.addRelativeAddressOrNull(Description.instantiationFn);
// Private data
Expand Down Expand Up @@ -2209,13 +2190,12 @@ IRGenModule::getConformanceInfo(const ProtocolDecl *protocol,

const ConformanceInfo *info;
// If the conformance is dependent in any way, we need to unique it.
// TODO: maybe this should apply whenever it's out of the module?
// TODO: actually enable this
//
// FIXME: Both implementations of ConformanceInfo are trivially-destructible,
// so in theory we could allocate them on a BumpPtrAllocator. But there's not
// a good one for us to use. (The ASTContext's outlives the IRGenModule in
// batch mode.)
if (isDependentConformance(rootConformance, /*considerResilience=*/true) ||
if (isDependentConformance(rootConformance) ||
// Foreign types need to go through the accessor to unique the witness
// table.
isSynthesizedNonUnique(rootConformance)) {
Expand Down Expand Up @@ -2287,13 +2267,13 @@ void IRGenModule::emitSILWitnessTable(SILWitnessTable *wt) {
// Produce the initializer value.
auto initializer = wtableContents.finishAndCreateFuture();

bool isDependent = isDependentConformance(conf);

llvm::GlobalVariable *global = nullptr;
unsigned tableSize;
if (!isResilientConformance(conf)) {
bool isDependent =
isDependentConformance(conf, /*considerResilience=*/false);
global = cast<llvm::GlobalVariable>(
isDependent
(isDependent && conf->getDeclContext()->isGenericContext())
? getAddrOfWitnessTablePattern(cast<NormalProtocolConformance>(conf),
initializer)
: getAddrOfWitnessTable(conf, initializer));
Expand All @@ -2309,10 +2289,7 @@ void IRGenModule::emitSILWitnessTable(SILWitnessTable *wt) {
// descriptor.
ConformanceDescription description(conf, wt, global, tableSize,
wtableBuilder.getTablePrivateSize(),
wtableBuilder.requiresSpecialization(),
isDependentConformance(
conf,
/*considerResilience=*/false));
isDependent);

// Build the instantiation function, we if need one.
description.instantiationFn = wtableBuilder.buildInstantiationFunction();
Expand Down
3 changes: 1 addition & 2 deletions lib/IRGen/IRGenModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -794,8 +794,7 @@ class IRGenModule {

bool isResilientConformance(const NormalProtocolConformance *conformance);
bool isResilientConformance(const RootProtocolConformance *root);
bool isDependentConformance(const RootProtocolConformance *conformance,
bool considerResilience);
bool isDependentConformance(const RootProtocolConformance *conformance);

Alignment getCappedAlignment(Alignment alignment);

Expand Down
38 changes: 30 additions & 8 deletions test/IRGen/protocol_conformance_records.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ public protocol Runcible {
// -- witness table
// CHECK-SAME: @"$s28protocol_conformance_records15NativeValueTypeVAA8RuncibleAAWP"
// -- flags
// CHECK-SAME: i32 0
// CHECK-SAME: },
// CHECK-SAME: i32 0 },
public struct NativeValueType: Runcible {
public func runce() {}
}
Expand All @@ -36,8 +35,7 @@ public struct NativeValueType: Runcible {
// -- witness table
// CHECK-SAME: @"$s28protocol_conformance_records15NativeClassTypeCAA8RuncibleAAWP"
// -- flags
// CHECK-SAME: i32 0
// CHECK-SAME: },
// CHECK-SAME: i32 0 },
public class NativeClassType: Runcible {
public func runce() {}
}
Expand All @@ -50,8 +48,7 @@ public class NativeClassType: Runcible {
// -- witness table
// CHECK-SAME: @"$s28protocol_conformance_records17NativeGenericTypeVyxGAA8RuncibleAAWP"
// -- flags
// CHECK-SAME: i32 0
// CHECK-SAME: },
// CHECK-SAME: i32 0 },
public struct NativeGenericType<T>: Runcible {
public func runce() {}
}
Expand Down Expand Up @@ -87,6 +84,30 @@ extension Size: Runcible {
public func runce() {}
}

// A non-dependent type conforming to a protocol with associated conformances
// does not require a generic witness table.
public protocol Simple {}

public protocol AssociateConformance {
associatedtype X : Simple
}

public struct Other : Simple {}

public struct Concrete : AssociateConformance {
public typealias X = Other
}

// CHECK-LABEL: @"$s28protocol_conformance_records8ConcreteVAA20AssociateConformanceAAMc" ={{ dllexport | protected | }}constant
// -- protocol descriptor
// CHECK-SAME: @"$s28protocol_conformance_records20AssociateConformanceMp"
// -- nominal type descriptor
// CHECK-SAME: @"$s28protocol_conformance_records8ConcreteVMn"
// -- witness table
// CHECK-SAME: @"$s28protocol_conformance_records8ConcreteVAA20AssociateConformanceAAWP"
// -- no flags are set, and no generic witness table follows
// CHECK-SAME: i32 0 }

// CHECK-LABEL: @"\01l_protocols"
// CHECK-SAME: @"$s28protocol_conformance_records8RuncibleMp"
// CHECK-SAME: @"$s28protocol_conformance_records5SpoonMp"
Expand Down Expand Up @@ -136,7 +157,7 @@ extension Int : OtherResilientProtocol { }
// -- witness table pattern
// CHECK-SAME: @"$s28protocol_conformance_records9DependentVyxGAA9AssociateAAWp"
// -- flags
// CHECK-SAME: i32 0
// CHECK-SAME: i32 131072,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this is not a change in test output -- the test was matching the wrong "i32 0"

// -- number of words in witness table
// CHECK-SAME: i16 2,
// -- number of private words in witness table + bit for "needs instantiation"
Expand All @@ -151,5 +172,6 @@ extension Dependent : Associate {
// CHECK-SAME: @"$s28protocol_conformance_records15NativeClassTypeCAA8RuncibleAAMc"
// CHECK-SAME: @"$s28protocol_conformance_records17NativeGenericTypeVyxGAA8RuncibleAAMc"
// CHECK-SAME: @"$s16resilient_struct4SizeV28protocol_conformance_records8RuncibleADMc"
// CHECK-SAME: @"$s28protocol_conformance_records8ConcreteVAA20AssociateConformanceAAMc"
// CHECK-SAME: @"$s28protocol_conformance_records17NativeGenericTypeVyxGAA5SpoonA2aERzlMc"
// CHECK-SAME: @"$sSi18resilient_protocol22OtherResilientProtocol0B20_conformance_recordsMc"
// CHECK-SAME: @"$sSi18resilient_protocol22OtherResilientProtocol0B20_conformance_recordsMc"
8 changes: 4 additions & 4 deletions test/IRGen/protocol_resilience.sil
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ protocol InternalProtocol {
// -- number of witness table entries
// CHECK-SAME: i16 0,

// -- size of private area + 'requires instantiation' bit (not set)
// CHECK-SAME: i16 0,
// -- size of private area + 'requires instantiation' bit
// CHECK-SAME: i16 1,

// -- instantiator function
// CHECK-SAME: i32 0
Expand Down Expand Up @@ -127,8 +127,8 @@ protocol InternalProtocol {
// -- number of witness table entries
// CHECK-SAME: i16 0,

// -- size of private area + 'requires instantiation' bit (not set)
// CHECK-SAME: i16 0,
// -- size of private area + 'requires instantiation' bit
// CHECK-SAME: i16 1,

// -- instantiator function
// CHECK-SAME: i32 0
Expand Down
12 changes: 10 additions & 2 deletions test/IRGen/protocol_resilience_descriptors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,18 @@ public struct ConditionallyConforms<Element> { }
public struct Y { }

// CHECK-USAGE-LABEL: @"$s31protocol_resilience_descriptors1YV010resilient_A022OtherResilientProtocolAAMc" =
// -- flags: has generic witness table
// CHECK-USAGE-SAME: i32 131072,
// -- number of witness table entries
// CHECK-USAGE-SAME: i16 0,
// CHECK-USAGE-SAME: i16 0,
// CHECK-USAGE-SAME: i32 0
// -- size of private area + 'requires instantiation' bit
// CHECK-USAGE-SAME: i16 1,
// -- instantiator function
// CHECK-USAGE-SAME: i32 0,
// -- private data area
// CHECK-USAGE-SAME: {{@[0-9]+}}
// --
// CHECK-USAGE-SAME: }
extension Y: OtherResilientProtocol { }

// CHECK-USAGE: @"$s31protocol_resilience_descriptors29ConformsWithAssocRequirementsV010resilient_A008ProtocoleF12TypeDefaultsAAMc" =
Expand Down