Skip to content

Fix of mandatory inliner's handling of unowned and guaranteed captures #12764

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 1 commit into from
Nov 4, 2017
Merged

Fix of mandatory inliner's handling of unowned and guaranteed captures #12764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
78 changes: 54 additions & 24 deletions lib/SILOptimizer/Mandatory/MandatoryInlining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,30 @@ class DeleteInstructionsHandler : public DeleteNotificationHandler {
/// applications: namely, that an apply of a thick function consumes the callee
/// and that the function implementing the closure consumes its capture
/// arguments.
static void fixupReferenceCounts(SILBasicBlock::iterator I,
SILValue CalleeValue,
SmallVectorImpl<SILValue> &CaptureArgs) {
static void fixupReferenceCounts(
SILBasicBlock::iterator I, SILValue CalleeValue,
SmallVectorImpl<std::pair<SILValue, ParameterConvention>> &CaptureArgs,
bool isCalleeGuaranteed) {
// Add a copy of each non-address type capture argument to lifetime extend the
// captured argument over the inlined function. This deals with the
// possibility of the closure being destroyed by an earlier application and
// thus cause the captured argument to be destroyed.
for (auto &CaptureArg : CaptureArgs)
if (!CaptureArg->getType().isAddress())
createIncrementBefore(CaptureArg, &*I);
for (auto &CaptureArg : CaptureArgs) {
if (!CaptureArg.first->getType().isAddress() &&
CaptureArg.second != ParameterConvention::Direct_Guaranteed &&
CaptureArg.second != ParameterConvention::Direct_Unowned) {
createIncrementBefore(CaptureArg.first, &*I);
} else {
// FIXME: What about indirectly owned parameters? The invocation of the
// closure would perform an indirect copy which we should mimick here.
assert(CaptureArg.second != ParameterConvention::Indirect_In &&
"Missing indirect copy");
}
}

// Destroy the callee as the apply would have done.
createDecrementBefore(CalleeValue, &*I);
if (!isCalleeGuaranteed)
createDecrementBefore(CalleeValue, &*I);
}

static SILValue cleanupLoadedCalleeValue(SILValue CalleeValue, LoadInst *LI) {
Expand Down Expand Up @@ -159,9 +170,9 @@ static SILValue cleanupLoadedCalleeValue(SILValue CalleeValue, LoadInst *LI) {

/// \brief Removes instructions that create the callee value if they are no
/// longer necessary after inlining.
static void
cleanupCalleeValue(SILValue CalleeValue, ArrayRef<SILValue> CaptureArgs,
ArrayRef<SILValue> FullArgs) {
static void cleanupCalleeValue(
SILValue CalleeValue,
ArrayRef<SILValue> FullArgs) {
SmallVector<SILInstruction*, 16> InstsToDelete;
for (SILValue V : FullArgs) {
if (V != CalleeValue)
Expand Down Expand Up @@ -217,6 +228,22 @@ cleanupCalleeValue(SILValue CalleeValue, ArrayRef<SILValue> CaptureArgs,
}
}

static void collectPartiallyAppliedArguments(
PartialApplyInst *PAI,
SmallVectorImpl<std::pair<SILValue, ParameterConvention>> &CapturedArgs,
SmallVectorImpl<SILValue> &FullArgs) {
ApplySite Site(PAI);
SILFunctionConventions CalleeConv(Site.getSubstCalleeType(),
PAI->getModule());
for (auto &Arg : PAI->getArgumentOperands()) {
unsigned CalleeArgumentIndex = Site.getCalleeArgIndex(Arg);
assert(CalleeArgumentIndex >= CalleeConv.getSILArgIndexOfFirstParam());
auto ParamInfo = CalleeConv.getParamInfoForSILArg(CalleeArgumentIndex);
CapturedArgs.push_back(std::make_pair(Arg.get(), ParamInfo.getConvention()));
FullArgs.push_back(Arg.get());
}
}

/// \brief Returns the callee SILFunction called at a call site, in the case
/// that the call is transparent (as in, both that the call is marked
/// with the transparent flag and that callee function is actually transparently
Expand All @@ -225,12 +252,11 @@ cleanupCalleeValue(SILValue CalleeValue, ArrayRef<SILValue> CaptureArgs,
///
/// In the case that a non-null value is returned, FullArgs contains effective
/// argument operands for the callee function.
static SILFunction *getCalleeFunction(SILFunction *F, FullApplySite AI,
bool &IsThick,
SmallVectorImpl<SILValue> &CaptureArgs,
SmallVectorImpl<SILValue> &FullArgs,
PartialApplyInst *&PartialApply,
SILModule::LinkingMode Mode) {
static SILFunction *getCalleeFunction(
SILFunction *F, FullApplySite AI, bool &IsThick,
SmallVectorImpl<std::pair<SILValue, ParameterConvention>> &CaptureArgs,
SmallVectorImpl<SILValue> &FullArgs, PartialApplyInst *&PartialApply,
SILModule::LinkingMode Mode) {
IsThick = false;
PartialApply = nullptr;
CaptureArgs.clear();
Expand Down Expand Up @@ -319,10 +345,9 @@ static SILFunction *getCalleeFunction(SILFunction *F, FullApplySite AI,
// one "thin to thick function" instructions, since those are the patterns
// generated when using auto closures.
if (auto *PAI = dyn_cast<PartialApplyInst>(CalleeValue)) {
for (const auto &Arg : PAI->getArguments()) {
CaptureArgs.push_back(Arg);
FullArgs.push_back(Arg);
}

// Collect the applied arguments and their convention.
collectPartiallyAppliedArguments(PAI, CaptureArgs, FullArgs);

CalleeValue = PAI->getCallee();
IsThick = true;
Expand Down Expand Up @@ -440,7 +465,7 @@ runOnFunctionRecursively(SILFunction *F, FullApplySite AI,
// during this call and recursive subcalls).
CurrentInliningSet = SetFactory.add(CurrentInliningSet, F);

SmallVector<SILValue, 16> CaptureArgs;
SmallVector<std::pair<SILValue, ParameterConvention>, 16> CaptureArgs;
SmallVector<SILValue, 32> FullArgs;

for (auto BI = F->begin(), BE = F->end(); BI != BE; ++BI) {
Expand All @@ -465,6 +490,7 @@ runOnFunctionRecursively(SILFunction *F, FullApplySite AI,
PartialApplyInst *PAI;
SILFunction *CalleeFunction = getCalleeFunction(
F, InnerAI, IsThick, CaptureArgs, FullArgs, PAI, Mode);

if (!CalleeFunction)
continue;

Expand Down Expand Up @@ -529,8 +555,12 @@ runOnFunctionRecursively(SILFunction *F, FullApplySite AI,

// If we intend to inline a thick function, then we need to balance the
// reference counts for correctness.
if (IsThick)
fixupReferenceCounts(II, CalleeValue, CaptureArgs);
if (IsThick) {
bool IsCalleeGuaranteed =
PAI &&
PAI->getType().castTo<SILFunctionType>()->isCalleeGuaranteed();
fixupReferenceCounts(II, CalleeValue, CaptureArgs, IsCalleeGuaranteed);
}

// Decrement our iterator (carefully, to avoid going off the front) so it
// is valid after inlining is done. Inlining deletes the apply, and can
Expand All @@ -551,7 +581,7 @@ runOnFunctionRecursively(SILFunction *F, FullApplySite AI,

// Now that the IR is correct, see if we can remove dead callee
// computations (e.g. dead partial_apply closures).
cleanupCalleeValue(CalleeValue, CaptureArgs, FullArgs);
cleanupCalleeValue(CalleeValue, FullArgs);

// Reposition iterators possibly invalidated by mutation.
BI = SILFunction::iterator(ApplyBlock);
Expand Down
100 changes: 100 additions & 0 deletions test/SILOptimizer/mandatory_inlining.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,3 +1084,103 @@ bb11(%128 : $Error):
bb12(%result : $C):
return %result : $C
}

sil @use_c : $@convention(thin) (@guaranteed C) -> ()
sil @use_c_unowned : $@convention(thin) (C) -> ()

sil [transparent] @guaranteed_closure_func : $@convention(thin) (@guaranteed C) -> () {
bb0(%0 : $C):
%use = function_ref @use_c : $@convention(thin) (@guaranteed C) -> ()
apply %use(%0) : $@convention(thin) (@guaranteed C) -> ()
%t = tuple ()
return %t : $()
}

sil [transparent] @unowned_closure_func : $@convention(thin) (C) -> () {
bb0(%0 : $C):
%use = function_ref @use_c_unowned : $@convention(thin) (C) -> ()
apply %use(%0) : $@convention(thin) (C) -> ()
%t = tuple ()
return %t : $()
}

// CHECK-LABEL: sil @test_guaranteed_closure_capture
// CHECK: bb0([[ARG:%.*]] : $C):
// CHECK: strong_retain [[ARG]] : $C
// @guaranteed closure captures *don't* release the capture in the closure implementation.
// CHECK-NOT: strong_retain [[ARG]] : $C
// CHECK: [[F:%.*]] = function_ref @use_c
// CHECK: apply [[F]]([[ARG]])
// CHECK: strong_release [[ARG]]
// CHECK: return
sil @test_guaranteed_closure_capture : $@convention(thin) (@guaranteed C) -> () {
bb0(%0: $C):
strong_retain %0 : $C
%closure_fun = function_ref @guaranteed_closure_func : $@convention(thin) (@guaranteed C) -> ()
%closure = partial_apply [callee_guaranteed] %closure_fun(%0) : $@convention(thin) (@guaranteed C) -> ()
apply %closure() : $@callee_guaranteed () -> ()
strong_release %closure : $@callee_guaranteed () -> ()
%t = tuple ()
return %t : $()
}

// CHECK-LABEL: sil @test_guaranteed_closure_capture2
// CHECK: bb0([[ARG:%.*]] : $C):
// @guaranteed closure captures *don't* release the capture in the closure implementation.
// CHECK: strong_retain [[ARG]] : $C
// CHECK: strong_release [[ARG]] : $C
// CHECK: [[F:%.*]] = function_ref @use_c
// CHECK: apply [[F]]([[ARG]])
// CHECK-NOT: strong_release [[ARG]]
// CHECK: return
sil @test_guaranteed_closure_capture2 : $@convention(thin) (@guaranteed C) -> () {
bb0(%0: $C):
strong_retain %0 : $C
%closure_fun = function_ref @guaranteed_closure_func : $@convention(thin) (@guaranteed C) -> ()
%closure = partial_apply %closure_fun(%0) : $@convention(thin) (@guaranteed C) -> ()
apply %closure() : $@callee_owned () -> ()
%t = tuple ()
return %t : $()
}

// CHECK-LABEL: sil @test_unowned_closure_capture2
// CHECK: bb0([[ARG:%.*]] : $C):
// CHECK: strong_retain [[ARG]]
// CHECK: strong_release [[ARG]]
// CHECK-NOT: strong_release [[ARG]]
// CHECK: [[F:%.*]] = function_ref @use_c_unowned
// CHECK: apply [[F]]([[ARG]]) : $@convention(thin) (C) -> ()
// CHECK-NOT: strong_release [[ARG]]
// CHECK: return
sil @test_unowned_closure_capture2 : $@convention(thin) (@guaranteed C) -> () {
bb0(%0: $C):
strong_retain %0 : $C
%closure_fun = function_ref @unowned_closure_func : $@convention(thin) (C) -> ()
%closure = partial_apply %closure_fun(%0) : $@convention(thin) (C) -> ()
apply %closure() : $@callee_owned () -> ()
%t = tuple ()
return %t : $()
}
// CHECK-LABEL: sil @test_guaranteed_closure
// CHECK: bb0([[ARG:%.*]] : $C):
// CHECK: strong_retain [[ARG]] : $C
// CHECK: [[F:%.*]] = function_ref @guaranteed_closure_func
// CHECK: [[C:%.*]] = partial_apply [callee_guaranteed] [[F]]([[ARG]])
// @guaranteed closure captures *don't* release the capture in the closure implementation.
// CHECK-NOT: strong_retain [[ARG]] : $C
// @callee_guaranteed closures *don't* release the context on application.
// CHECK-NOT: strong_release [[C]]
// CHECK: [[F:%.*]] = function_ref @use_c
// CHECK: apply [[F]]([[ARG]])
// CHECK: return [[C]]

sil @test_guaranteed_closure : $@convention(thin) (@guaranteed C) -> @owned @callee_guaranteed () -> () {
bb0(%0: $C):
strong_retain %0 : $C
%closure_fun = function_ref @guaranteed_closure_func : $@convention(thin) (@guaranteed C) -> ()
%closure = partial_apply [callee_guaranteed] %closure_fun(%0) : $@convention(thin) (@guaranteed C) -> ()
// NOTE: No ``strong_retain %closure`` is needed here because the context is
// @callee_guaranteed.
apply %closure() : $@callee_guaranteed () -> ()
return %closure : $@callee_guaranteed () -> ()
}