Skip to content

Commit f913006

Browse files
committed
PerformanceDiagnostics: handle closures
Check if no-escaping closures meet the performance constraints. Do this already when passing a closure to a function. rdar://94729207
1 parent 481381b commit f913006

File tree

2 files changed

+186
-4
lines changed

2 files changed

+186
-4
lines changed

lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp

Lines changed: 148 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,46 @@ class PerformanceDiagnostics {
5353
PerformanceDiagnostics(SILModule &module, BasicCalleeAnalysis *bca) :
5454
module(module), bca(bca) {}
5555

56+
/// Check a function with performance annotation(s) and all called functions
57+
/// recursively.
5658
bool visitFunction(SILFunction *function, PerformanceConstraints perfConstr) {
5759
return visitFunction(function, perfConstr, /*parentLoc*/ nullptr);
5860
}
5961

62+
/// Check functions _without_ performance annotations.
63+
///
64+
/// This is need to check closure arguments of called performance-annotated
65+
/// functions.
66+
void checkNonAnnotatedFunction(SILFunction *function);
67+
6068
private:
6169
bool visitFunction(SILFunction *function, PerformanceConstraints perfConstr,
6270
LocWithParent *parentLoc);
6371

6472
bool visitInst(SILInstruction *inst, PerformanceConstraints perfConstr,
6573
LocWithParent *parentLoc);
6674

75+
/// Check if `as` has any non-escaping closure arguments and if all those
76+
/// passed closures meet the `perfConstr`.
77+
///
78+
/// If `acceptFunctionArgs` is true it is assumed that calls to the function,
79+
/// which contains `as`, are already checked to have correct closure arguments.
80+
bool checkClosureArguments(ApplySite as,
81+
bool acceptFunctionArgs,
82+
PerformanceConstraints perfConstr,
83+
LocWithParent *parentLoc);
84+
85+
/// Check if `closure` meets the `perfConstr`.
86+
///
87+
/// If `acceptFunctionArgs` is true it is assumed that calls to the function,
88+
/// which contains `callInst`, are already checked to have correct closure
89+
/// arguments.
90+
bool checkClosureValue(SILValue closure,
91+
bool acceptFunctionArgs,
92+
SILInstruction *callInst,
93+
PerformanceConstraints perfConstr,
94+
LocWithParent *parentLoc);
95+
6796
bool visitCallee(SILInstruction *callInst, CalleeList callees,
6897
PerformanceConstraints perfConstr, LocWithParent *parentLoc);
6998

@@ -107,6 +136,22 @@ bool PerformanceDiagnostics::visitFunction(SILFunction *function,
107136
if (isEffectFreeArraySemanticCall(&inst))
108137
continue;
109138

139+
// Check if closures, which are passed to `as` are okay.
140+
if (checkClosureArguments(as, /*acceptFunctionArgs=*/ true,
141+
perfConstr, parentLoc)) {
142+
return true;
143+
}
144+
145+
if (as.getOrigCalleeType()->isNoEscape()) {
146+
// This is a call of a non-escaping closure. Check if the closure is
147+
// okay. In this case we don't need to `visitCallee`.
148+
if (checkClosureValue(as.getCallee(), /*acceptFunctionArgs=*/ true,
149+
&inst, perfConstr, parentLoc)) {
150+
return true;
151+
}
152+
continue;
153+
}
154+
110155
// Recursively walk into the callees.
111156
if (visitCallee(&inst, bca->getCalleeList(as), perfConstr, parentLoc))
112157
return true;
@@ -132,6 +177,62 @@ bool PerformanceDiagnostics::visitFunction(SILFunction *function,
132177
return false;
133178
}
134179

180+
bool PerformanceDiagnostics::checkClosureArguments(ApplySite as,
181+
bool acceptFunctionArgs,
182+
PerformanceConstraints perfConstr,
183+
LocWithParent *parentLoc) {
184+
if (SILFunction *knownCallee = as.getReferencedFunctionOrNull()) {
185+
if (knownCallee->hasSemanticsAttr(semantics::NO_PERFORMANCE_ANALYSIS))
186+
return false;
187+
}
188+
189+
for (SILValue arg : as.getArguments()) {
190+
auto fTy = arg->getType().getAs<SILFunctionType>();
191+
if (!fTy || !fTy->isNoEscape())
192+
continue;
193+
194+
if (checkClosureValue(arg, acceptFunctionArgs, as.getInstruction(),
195+
perfConstr, parentLoc)) {
196+
return true;
197+
}
198+
}
199+
return false;
200+
}
201+
202+
bool PerformanceDiagnostics::checkClosureValue(SILValue closure,
203+
bool acceptFunctionArgs,
204+
SILInstruction *callInst,
205+
PerformanceConstraints perfConstr,
206+
LocWithParent *parentLoc) {
207+
// Walk through the definition of the closure until we find the "underlying"
208+
// function_ref instruction.
209+
while (!isa<FunctionRefInst>(closure)) {
210+
if (auto *pai = dyn_cast<PartialApplyInst>(closure)) {
211+
if (checkClosureArguments(ApplySite::isa(pai), acceptFunctionArgs,
212+
perfConstr, parentLoc)) {
213+
return true;
214+
}
215+
closure = pai->getCallee();
216+
} else if (auto *tfi = dyn_cast<ThinToThickFunctionInst>(closure)) {
217+
closure = tfi->getOperand();
218+
} else if (acceptFunctionArgs && isa<SILFunctionArgument>(closure)) {
219+
// We can assume that a function closure argument is already checked at
220+
// the call site.
221+
return false;
222+
} else {
223+
diagnose(LocWithParent(callInst->getLoc().getSourceLoc(), parentLoc), diag::performance_unknown_callees);
224+
return true;
225+
}
226+
}
227+
// Check what's happening inside the closure body.
228+
auto *fri = cast<FunctionRefInst>(closure);
229+
if (visitCallee(callInst, fri->getReferencedFunction(),
230+
perfConstr, parentLoc)) {
231+
return true;
232+
}
233+
return false;
234+
}
235+
135236
bool PerformanceDiagnostics::visitCallee(SILInstruction *callInst,
136237
CalleeList callees,
137238
PerformanceConstraints perfConstr,
@@ -313,6 +414,33 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
313414
return false;
314415
}
315416

417+
void PerformanceDiagnostics::checkNonAnnotatedFunction(SILFunction *function) {
418+
for (SILBasicBlock &block : *function) {
419+
for (SILInstruction &inst : block) {
420+
auto as = FullApplySite::isa(&inst);
421+
if (!as)
422+
continue;
423+
424+
// We only consider direct calls.
425+
// TODO: this is a hole in the verification because we are not catching
426+
// cases where a "bad" closure is passed to a performance-annotated
427+
// v-table, witness table or closure function.
428+
SILFunction *callee = as.getReferencedFunctionOrNull();
429+
if (!callee)
430+
continue;
431+
432+
if (callee->getPerfConstraints() == PerformanceConstraints::None)
433+
continue;
434+
435+
if (checkClosureArguments(as, /*acceptFunctionArgs=*/ false,
436+
callee->getPerfConstraints(),
437+
/*LocWithParent*/ nullptr)) {
438+
return;
439+
}
440+
}
441+
}
442+
}
443+
316444
void PerformanceDiagnostics::diagnose(LocWithParent loc, Diagnostic &&D) {
317445
// Start with a valid location in the call tree.
318446
LocWithParent *validLoc = &loc;
@@ -346,13 +474,16 @@ class PerformanceDiagnosticsPass : public SILModuleTransform {
346474
SILModule *module = getModule();
347475

348476
PerformanceDiagnostics diagnoser(*module, getAnalysis<BasicCalleeAnalysis>());
477+
bool annotatedFunctionsFound = false;
349478

350479
for (SILFunction &function : *module) {
351-
// Don't rerun diagnostics on deserialized functions.
352-
if (function.wasDeserializedCanonical())
353-
continue;
354-
355480
if (function.getPerfConstraints() != PerformanceConstraints::None) {
481+
annotatedFunctionsFound = true;
482+
483+
// Don't rerun diagnostics on deserialized functions.
484+
if (function.wasDeserializedCanonical())
485+
continue;
486+
356487
if (!module->getOptions().EnablePerformanceAnnotations) {
357488
module->getASTContext().Diags.diagnose(
358489
function.getLocation().getSourceLoc(),
@@ -363,6 +494,19 @@ class PerformanceDiagnosticsPass : public SILModuleTransform {
363494
diagnoser.visitFunction(&function, function.getPerfConstraints());
364495
}
365496
}
497+
498+
if (!annotatedFunctionsFound)
499+
return;
500+
501+
for (SILFunction &function : *module) {
502+
// Don't rerun diagnostics on deserialized functions.
503+
if (function.wasDeserializedCanonical())
504+
continue;
505+
506+
if (function.getPerfConstraints() == PerformanceConstraints::None) {
507+
diagnoser.checkNonAnnotatedFunction(&function);
508+
}
509+
}
366510
}
367511
};
368512

test/SILOptimizer/performance-annotations.swift

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,41 @@ func globalWithInitializer(x: MyStruct) {
163163
_ = MyStruct.v // expected-note {{called from here}}
164164
}
165165

166+
@_noAllocation
167+
func callBadClosure(closure: ()->Int) -> Int {
168+
return closure()
169+
}
170+
171+
@_noAllocation
172+
func badClosure() {
173+
_ = callBadClosure(closure: { // expected-note {{called from here}}
174+
_ = Cl() // expected-error {{Using type 'Cl' can cause metadata allocation or locks}}
175+
return 42
176+
})
177+
}
178+
179+
func badClosure2() {
180+
_ = callBadClosure(closure: { // expected-note {{called from here}}
181+
_ = Cl() // expected-error {{Using type 'Cl' can cause metadata allocation or locks}}
182+
return 42
183+
})
184+
}
185+
186+
@_noAllocation
187+
func callGoodClosure(closure: ()->Int) -> Int {
188+
return closure()
189+
}
190+
191+
@_noAllocation
192+
func goodClosure() {
193+
_ = callBadClosure(closure: {
194+
return 42
195+
})
196+
}
197+
198+
func goodClosure2() {
199+
_ = callBadClosure(closure: {
200+
return 42
201+
})
202+
}
203+

0 commit comments

Comments
 (0)