Skip to content

Commit a40f088

Browse files
committed
SIL: Don't DFE functions referenced from KeyPath patterns.
Fixes rdar://problem/31776015.
1 parent aaaea05 commit a40f088

File tree

7 files changed

+116
-13
lines changed

7 files changed

+116
-13
lines changed

include/swift/SIL/SILInstruction.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,6 +1599,9 @@ class KeyPathPatternComponent {
15991599
getter, setter, indices, ty);
16001600
}
16011601

1602+
void incrementRefCounts() const;
1603+
void decrementRefCounts() const;
1604+
16021605
void Profile(llvm::FoldingSetNodeID &ID);
16031606
};
16041607

@@ -1696,6 +1699,7 @@ class KeyPathInst final
16961699

16971700
public:
16981701
KeyPathPattern *getPattern() const;
1702+
bool hasPattern() const { return (bool)Pattern; }
16991703

17001704
ArrayRef<Operand> getAllOperands() const {
17011705
// TODO: Subscript keypaths will have operands.
@@ -1712,6 +1716,8 @@ class KeyPathInst final
17121716
return V->getKind() == ValueKind::KeyPathInst;
17131717
}
17141718

1719+
void dropReferencedPattern();
1720+
17151721
~KeyPathInst();
17161722
};
17171723

lib/SIL/SILInstruction.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,22 @@ void SILInstruction::dropAllReferences() {
152152

153153
// If we have a function ref inst, we need to especially drop its function
154154
// argument so that it gets a proper ref decrement.
155-
auto *FRI = dyn_cast<FunctionRefInst>(this);
156-
if (!FRI || !FRI->getReferencedFunction())
155+
if (auto *FRI = dyn_cast<FunctionRefInst>(this)) {
156+
if (!FRI->getReferencedFunction())
157+
return;
158+
FRI->dropReferencedFunction();
157159
return;
160+
}
158161

159-
FRI->dropReferencedFunction();
162+
// If we have a KeyPathInst, drop its pattern reference so that we can
163+
// decrement refcounts on referenced functions.
164+
if (auto *KPI = dyn_cast<KeyPathInst>(this)) {
165+
if (!KPI->hasPattern())
166+
return;
167+
168+
KPI->dropReferencedPattern();
169+
return;
170+
}
160171
}
161172

162173
void SILInstruction::replaceAllUsesWithUndef() {

lib/SIL/SILInstructions.cpp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1978,6 +1978,40 @@ bool KeyPathPatternComponent::isComputedSettablePropertyMutating() const {
19781978
}
19791979
}
19801980

1981+
static void
1982+
forEachRefcountableReference(const KeyPathPatternComponent &component,
1983+
llvm::function_ref<void (SILFunction*)> forFunction) {
1984+
switch (component.getKind()) {
1985+
case KeyPathPatternComponent::Kind::StoredProperty:
1986+
return;
1987+
case KeyPathPatternComponent::Kind::SettableProperty:
1988+
forFunction(component.getComputedPropertySetter());
1989+
LLVM_FALLTHROUGH;
1990+
case KeyPathPatternComponent::Kind::GettableProperty:
1991+
forFunction(component.getComputedPropertyGetter());
1992+
1993+
switch (component.getComputedPropertyId().getKind()) {
1994+
case KeyPathPatternComponent::ComputedPropertyId::DeclRef:
1995+
// Mark the vtable entry as used somehow?
1996+
return;
1997+
case KeyPathPatternComponent::ComputedPropertyId::Function:
1998+
forFunction(component.getComputedPropertyId().getFunction());
1999+
return;
2000+
case KeyPathPatternComponent::ComputedPropertyId::Property:
2001+
return;
2002+
}
2003+
}
2004+
}
2005+
2006+
void KeyPathPatternComponent::incrementRefCounts() const {
2007+
forEachRefcountableReference(*this,
2008+
[&](SILFunction *f) { f->incrementRefCount(); });
2009+
}
2010+
void KeyPathPatternComponent::decrementRefCounts() const {
2011+
forEachRefcountableReference(*this,
2012+
[&](SILFunction *f) { f->decrementRefCount(); });
2013+
}
2014+
19812015
KeyPathPattern *
19822016
KeyPathPattern::get(SILModule &M, CanGenericSignature signature,
19832017
CanType rootType, CanType valueType,
@@ -2116,6 +2150,11 @@ KeyPathInst::KeyPathInst(SILDebugLocation Loc,
21162150
{
21172151
auto *subsBuf = getTrailingObjects<Substitution>();
21182152
std::uninitialized_copy(Subs.begin(), Subs.end(), subsBuf);
2153+
2154+
// Increment the use of any functions referenced from the keypath pattern.
2155+
for (auto component : Pattern->getComponents()) {
2156+
component.incrementRefCounts();
2157+
}
21192158
}
21202159

21212160
MutableArrayRef<Substitution>
@@ -2130,9 +2169,24 @@ KeyPathInst::getAllOperands() {
21302169
}
21312170

21322171
KeyPathInst::~KeyPathInst() {
2172+
if (!Pattern)
2173+
return;
2174+
2175+
// Decrement the use of any functions referenced from the keypath pattern.
2176+
for (auto component : Pattern->getComponents()) {
2177+
component.decrementRefCounts();
2178+
}
21332179
// TODO: destroy operands
21342180
}
21352181

21362182
KeyPathPattern *KeyPathInst::getPattern() const {
2183+
assert(Pattern && "pattern was reset!");
21372184
return Pattern;
21382185
}
2186+
2187+
void KeyPathInst::dropReferencedPattern() {
2188+
for (auto component : Pattern->getComponents()) {
2189+
component.decrementRefCounts();
2190+
}
2191+
Pattern = nullptr;
2192+
}

lib/SILOptimizer/IPO/DeadFunctionElimination.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,46 @@ class FunctionLivenessComputation {
192192

193193
}
194194

195+
/// Marks the declarations referenced by a key path pattern as alive if they
196+
/// aren't yet.
197+
void ensureKeyPathComponentsAreAlive(KeyPathPattern *KP) {
198+
for (auto &component : KP->getComponents()) {
199+
switch (component.getKind()) {
200+
case KeyPathPatternComponent::Kind::SettableProperty:
201+
ensureAlive(component.getComputedPropertySetter());
202+
LLVM_FALLTHROUGH;
203+
case KeyPathPatternComponent::Kind::GettableProperty: {
204+
ensureAlive(component.getComputedPropertyGetter());
205+
auto id = component.getComputedPropertyId();
206+
switch (id.getKind()) {
207+
case KeyPathPatternComponent::ComputedPropertyId::DeclRef: {
208+
auto decl = cast<AbstractFunctionDecl>(id.getDeclRef().getDecl());
209+
if (auto clas = dyn_cast<ClassDecl>(decl->getDeclContext())) {
210+
ensureAliveClassMethod(getMethodInfo(decl, /*witness*/ false),
211+
dyn_cast<FuncDecl>(decl),
212+
clas);
213+
} else if (auto proto =
214+
dyn_cast<ProtocolDecl>(decl->getDeclContext())) {
215+
ensureAliveProtocolMethod(getMethodInfo(decl, /*witness*/ true));
216+
} else {
217+
llvm_unreachable("key path keyed by a non-class, non-protocol method");
218+
}
219+
break;
220+
}
221+
case KeyPathPatternComponent::ComputedPropertyId::Function:
222+
ensureAlive(id.getFunction());
223+
break;
224+
case KeyPathPatternComponent::ComputedPropertyId::Property:
225+
break;
226+
}
227+
continue;
228+
}
229+
case KeyPathPatternComponent::Kind::StoredProperty:
230+
continue;
231+
}
232+
}
233+
}
234+
195235
/// Marks a function as alive if it is not alive yet.
196236
void ensureAlive(SILFunction *F) {
197237
if (!isAlive(F))
@@ -313,6 +353,8 @@ class FunctionLivenessComputation {
313353
ensureAliveClassMethod(mi, dyn_cast<FuncDecl>(funcDecl), MethodCl);
314354
} else if (auto *FRI = dyn_cast<FunctionRefInst>(&I)) {
315355
ensureAlive(FRI->getReferencedFunction());
356+
} else if (auto *KPI = dyn_cast<KeyPathInst>(&I)) {
357+
ensureKeyPathComponentsAreAlive(KPI->getPattern());
316358
}
317359
}
318360
}

test/stdlib/KeyPath.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
// REQUIRES: executable_test
55
// REQUIRES: PTRSIZE=64
66

7-
// Disabled for now
8-
// REQUIRES: rdar://problem/31776015
9-
107
import StdlibUnittest
118

129
var keyPath = TestSuite("key paths")

test/stdlib/KeyPathImplementation.swift

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@
44
// REQUIRES: executable_test
55
// REQUIRES: PTRSIZE=64
66

7-
// Disabled for now
8-
// REQUIRES: rdar://problem/31776015
9-
10-
117
import StdlibUnittest
128

139
var keyPathImpl = TestSuite("key path implementation")

test/stdlib/KeyPathObjC.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@
55
// REQUIRES: PTRSIZE=64
66
// REQUIRES: objc_interop
77

8-
// Disabled for now
9-
// REQUIRES: rdar://problem/31776015
10-
118
import StdlibUnittest
129
import Foundation
1310

0 commit comments

Comments
 (0)