Skip to content

[clf] Simplify the double diamond lifetime extension for escaping swi… #25320

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
Jun 10, 2019
Merged

[clf] Simplify the double diamond lifetime extension for escaping swi… #25320

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
7 changes: 7 additions & 0 deletions include/swift/SIL/SILBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,13 @@ class SILBuilder {
return createUncheckedEnumData(Loc, Operand, Element, EltType);
}

/// Return unchecked_enum_data %Operand, #Optional<T>.some.
SILValue emitExtractOptionalPayloadOperation(SILLocation Loc,
SILValue Operand) {
auto *Decl = F->getASTContext().getOptionalSomeDecl();
return createUncheckedEnumData(Loc, Operand, Decl);
}

UncheckedTakeEnumDataAddrInst *
createUncheckedTakeEnumDataAddr(SILLocation Loc, SILValue Operand,
EnumElementDecl *Element, SILType Ty) {
Expand Down
111 changes: 83 additions & 28 deletions lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ llvm::cl::opt<bool> DisableConvertEscapeToNoEscapeSwitchEnumPeephole(

using namespace swift;

/// Given an optional diamond, return the bottom of the diamond.
///
/// That is given that sei is in bb0,
///
/// /---> bb1 ---\
/// / \
/// bb0 ---> bb3
/// \ /
/// \---> bb2 ---/
///
/// this routine will return bb3.
static SILBasicBlock *getOptionalDiamondSuccessor(SwitchEnumInst *sei) {
auto numSuccs = sei->getNumSuccessors();
if (numSuccs != 2)
Expand Down Expand Up @@ -434,25 +445,53 @@ static bool tryExtendLifetimeToLastUse(
return false;
}

/// Ensure the lifetime of the closure accross an
/// Ensure the lifetime of the closure across a two step optional conversion
/// from:
///
/// optional<@escaping () -> ()>
///
/// to:
///
/// optional<@noescape () -> ()>
///
/// to:
///
/// optional<@escaping () -> ()> to
/// optional<@noescape @convention(block) () -> ()>
///
/// conversion and its use.
/// and all uses of the block. The pattern that we are looking for is:
///
/// The pattern this is looking for
/// switch_enum %closure
/// / \
/// convert_escape_to_noescape nil
/// switch_enum
/// switch_enum %optional_closure (1)
/// / \
/// convertToBlock nil
/// %trivial_closure = CVT %closure nil (2)
/// \ /
/// switch_enum %optional_trivial_closure (3)
/// / \
/// %b = convertToBlock %trivial_closure nil (4)
/// \ /
/// (%convertOptionalBlock :)
/// We will insert a copy_value of the original %closure before the two
/// diamonds. And a destroy of %closure at the last destroy of
/// %convertOptionalBlock.
/// ... uses of %optional_block ...
/// destroy_value %optional_block
///
/// where CVT is convert_escape_to_no_escape [not_guaranteed]. We assume that
/// the %optional_block is going through a conversion sequence in SILGen meaning
/// that we should only have a single destroy of the optional block.
///
/// NOTE: There is a *lifetime gap* during the usage of the trivial_closure!
/// This means we must be careful when lifetime extending. We can only assume
/// that the underlying closure is alive immediately at the CVT. So to perform
/// our lifetime extend, we do the following:
///
/// 1. We copy and borrow optional_closure, right before the switch_enum in
/// (1).
///
/// 2. We rewrite the convert_escape_to_no_escape guaranteed to use the copy
/// instead.
///
/// 3. To make sure that even after ossa is complete, we do not move any
/// destroys above the convert_escape_to_no_escape by putting a mark_dependence
/// on %closure
///
/// 4. We insert an end_borrow, destroy for the copy at the destroy of the
/// optional block.
static bool trySwitchEnumPeephole(ConvertEscapeToNoEscapeInst *cvt) {
auto *blockArg = dyn_cast<SILArgument>(cvt->getOperand());
if (!blockArg)
Expand All @@ -478,7 +517,8 @@ static bool trySwitchEnumPeephole(ConvertEscapeToNoEscapeInst *cvt) {
if (diamondSucc2->getNumArguments() != 1)
return false;

// Look for the last and only destroy.
// Look for the last and only destroy of the diamond succ 2's argument. This
// is going to be the place where we destroy the lifetime extending copy.
SILInstruction *onlyDestroy = [&]() -> SILInstruction * {
SILInstruction *lastDestroy = nullptr;
for (auto *use : diamondSucc2->getArgument(0)->getUses()) {
Expand All @@ -497,15 +537,27 @@ static bool trySwitchEnumPeephole(ConvertEscapeToNoEscapeInst *cvt) {

// Extend the lifetime.
auto loc = RegularLocation::getAutoGeneratedLocation();
auto *copy = SILBuilderWithScope(switchEnum1)
.createCopyValue(loc, switchEnum1->getOperand());
SILValue copy, borrow;
std::tie(copy, borrow) = ([&]() -> std::pair<SILValue, SILValue> {
SILBuilderWithScope builder(switchEnum1);
auto copy = builder.emitCopyValueOperation(loc, switchEnum1->getOperand());
auto borrow = builder.emitBeginBorrowOperation(loc, copy);
return {copy, borrow};
})(); // end std::tie(copy, borrow).

{
SILBuilderWithScope builder(cvt);
auto value = builder.emitExtractOptionalPayloadOperation(loc, borrow);
cvt->setOperand(value);
cvt->setLifetimeGuaranteed();
}

cvt->setLifetimeGuaranteed();
auto *mdi = SILBuilderWithScope(cvt).createMarkDependence(
loc, cvt->getOperand(), copy);
cvt->setOperand(mdi);
{
SILBuilderWithScope builder(onlyDestroy);
builder.emitEndBorrowOperation(loc, borrow);
builder.emitDestroyValueOperation(loc, copy);
}

SILBuilderWithScope(onlyDestroy).createDestroyValue(loc, copy);
return true;
}

Expand Down Expand Up @@ -757,7 +809,9 @@ static bool fixupClosureLifetimes(SILFunction &fn, bool &checkStackNesting,

// Handle, copy_block_without_escaping instructions.
if (auto *cb = dyn_cast<CopyBlockWithoutEscapingInst>(inst)) {
changed |= fixupCopyBlockWithoutEscaping(cb, modifiedCFG);
if (fixupCopyBlockWithoutEscaping(cb, modifiedCFG)) {
changed = true;
}
continue;
}

Expand All @@ -768,22 +822,23 @@ static bool fixupClosureLifetimes(SILFunction &fn, bool &checkStackNesting,
continue;

// First try to peephole a known pattern.
if (!DisableConvertEscapeToNoEscapeSwitchEnumPeephole &&
trySwitchEnumPeephole(cvt)) {
changed |= true;
continue;
if (!DisableConvertEscapeToNoEscapeSwitchEnumPeephole) {
if (trySwitchEnumPeephole(cvt)) {
changed = true;
continue;
}
}

if (tryExtendLifetimeToLastUse(cvt, memoizedQueries, i)) {
changed |= true;
changed = true;
checkStackNesting = true;
continue;
}

// Otherwise, extend the lifetime of the operand to the end of the
// function.
extendLifetimeToEndOfFunction(fn, cvt);
changed |= true;
changed = true;
}
}
return changed;
Expand Down
117 changes: 117 additions & 0 deletions test/SILOptimizer/closure-lifetime-fixup.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// RUN: %target-sil-opt -closure-lifetime-fixup %s | %FileCheck %s

sil_stage raw

import Swift
import Builtin
import SwiftShims

class FakeNSString {}

sil @$sSSSgIgg_AAIegg_TR : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
sil @noescapeBlock3 : $@convention(c) (Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<FakeNSString>) -> ()
sil @$sSS10FoundationE19_bridgeToObjectiveCSo8FakeNSStringCyF : $@convention(method) (@guaranteed String) -> @owned FakeNSString
sil @$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
sil @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()

// Just make sure that we perform the optimization and do not trigger the ownership verifier.
//
// CHECK-LABEL: sil [ossa] @test1 : $@convention(thin) (@guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, @guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>) -> () {
// CHECK-NOT: convert_escape_to_noescape [not_guaranteed]
// CHECK: } // end sil function 'test1'
sil [ossa] @test1 : $@convention(thin) (@guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, @guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>) -> () {
bb0(%0 : @guaranteed $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, %1 : @guaranteed $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>):
%2 = copy_value %0 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>
switch_enum %2 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb1, case #Optional.none!enumelt: bb12

bb1(%4 : @owned $@callee_guaranteed (@guaranteed Optional<String>) -> ()):
%5 = convert_escape_to_noescape [not_guaranteed] %4 : $@callee_guaranteed (@guaranteed Optional<String>) -> () to $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
%6 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.some!enumelt.1, %5 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
destroy_value %4 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
br bb2(%6 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)

bb2(%9 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>):
switch_enum %9 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb3, case #Optional.none!enumelt: bb4

bb3(%11 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()):
%12 = function_ref @$sSSSgIgg_AAIegg_TR : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
%13 = partial_apply [callee_guaranteed] %12(%11) : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
%14 = mark_dependence %13 : $@callee_guaranteed (@guaranteed Optional<String>) -> () on %11 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
%15 = copy_value %14 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
%16 = alloc_stack $@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
%17 = project_block_storage %16 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
store %15 to [init] %17 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
%19 = function_ref @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()
%20 = init_block_storage_header %16 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), invoke %19 : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> (), type $@convention(block) @noescape (Optional<FakeNSString>) -> ()
%21 = copy_block_without_escaping %20 : $@convention(block) @noescape (Optional<FakeNSString>) -> () withoutEscaping %14 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
%22 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.some!enumelt.1, %21 : $@convention(block) @noescape (Optional<FakeNSString>) -> ()
destroy_addr %17 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
dealloc_stack %16 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
br bb5(%22 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)

bb4:
%26 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.none!enumelt
br bb5(%26 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)

bb5(%28 : @owned $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>):
%29 = copy_value %1 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>
switch_enum %29 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb6, case #Optional.none!enumelt: bb11

bb6(%31 : @owned $@callee_guaranteed (@guaranteed Optional<String>) -> ()):
%32 = convert_escape_to_noescape [not_guaranteed] %31 : $@callee_guaranteed (@guaranteed Optional<String>) -> () to $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
%33 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.some!enumelt.1, %32 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
destroy_value %31 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
br bb7(%33 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)

bb7(%36 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>):
switch_enum %36 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb8, case #Optional.none!enumelt: bb9

bb8(%38 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()):
%39 = function_ref @$sSSSgIgg_AAIegg_TR : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
%40 = partial_apply [callee_guaranteed] %39(%38) : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
%41 = mark_dependence %40 : $@callee_guaranteed (@guaranteed Optional<String>) -> () on %38 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
%42 = copy_value %41 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
%43 = alloc_stack $@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
%44 = project_block_storage %43 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
store %42 to [init] %44 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
%46 = function_ref @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()
%47 = init_block_storage_header %43 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), invoke %46 : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> (), type $@convention(block) @noescape (Optional<FakeNSString>) -> ()
%48 = copy_block_without_escaping %47 : $@convention(block) @noescape (Optional<FakeNSString>) -> () withoutEscaping %41 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
%49 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.some!enumelt.1, %48 : $@convention(block) @noescape (Optional<FakeNSString>) -> ()
destroy_addr %44 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
dealloc_stack %43 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
br bb10(%49 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)

bb9:
%53 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.none!enumelt
br bb10(%53 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)

bb10(%55 : @owned $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>):
%56 = string_literal utf8 "Foobar"
%57 = integer_literal $Builtin.Word, 6
%58 = integer_literal $Builtin.Int1, -1
%59 = metatype $@thin String.Type
%60 = function_ref @$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
%61 = apply %60(%56, %57, %58, %59) : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
%62 = function_ref @$sSS10FoundationE19_bridgeToObjectiveCSo8FakeNSStringCyF : $@convention(method) (@guaranteed String) -> @owned FakeNSString
%63 = begin_borrow %61 : $String
%64 = apply %62(%63) : $@convention(method) (@guaranteed String) -> @owned FakeNSString
end_borrow %63 : $String
%66 = enum $Optional<FakeNSString>, #Optional.some!enumelt.1, %64 : $FakeNSString
destroy_value %61 : $String
%68 = function_ref @noescapeBlock3 : $@convention(c) (Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<FakeNSString>) -> ()
%69 = apply %68(%28, %55, %66) : $@convention(c) (Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<FakeNSString>) -> ()
destroy_value %66 : $Optional<FakeNSString>
destroy_value %55 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>
destroy_value %28 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>
%73 = tuple ()
return %73 : $()

bb11:
%75 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.none!enumelt
br bb7(%75 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)

bb12:
%77 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.none!enumelt
br bb2(%77 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)
}
4 changes: 2 additions & 2 deletions test/SILOptimizer/definite-init-convert-to-escape.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public func returnOptionalEscape() -> (() ->())?
// CHECK: switch_enum [[V1]] : $Optional<{{.*}}>, case #Optional.some!enumelt.1: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[NONE_BB:bb[0-9]+]]
//
// CHECK: [[SOME_BB]]([[V2:%.*]] : $@callee_guaranteed () -> ()):
// CHECK: [[CVT_MARK_DEP:%.*]] = mark_dependence [[V2]] : $@callee_guaranteed () -> () on [[V1]]
// CHECK: [[CVT:%.*]] = convert_escape_to_noescape [[CVT_MARK_DEP]]
// CHECK: [[V1_UNWRAPPED:%.*]] = unchecked_enum_data [[V1]]
// CHECK: [[CVT:%.*]] = convert_escape_to_noescape [[V1_UNWRAPPED]]
// CHECK: [[SOME:%.*]] = enum $Optional<{{.*}}>, #Optional.some!enumelt.1, [[CVT]]
// CHECK: strong_release [[V2]]
// CHECK: br [[NEXT_BB:bb[0-9]+]]([[SOME]] :
Expand Down