From bd6d7ad5241fe88b8b4353ce26a83e87feec6ead Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Thu, 29 Mar 2018 09:39:27 -0700 Subject: [PATCH 1/9] [4.2] SILGen: Don't postpone the cleanup of the convertEscapeToNoEscape inside an bind optional This would violate dominance since the convertEscapeToNoEscape instruction does not dominate the postpone point. rdar://38124009 * Ensure lifetimes of optional convert_escape_to_noescape closures --- lib/SILGen/SILGenBuilder.cpp | 10 +- lib/SILGen/SILGenBuilder.h | 6 +- lib/SILGen/SILGenExpr.cpp | 7 +- lib/SILGen/SILGenFunction.h | 6 +- lib/SILGen/SILGenPoly.cpp | 28 +++-- .../Mandatory/DefiniteInitialization.cpp | 113 ++++++++++++++++++ .../Inputs/usr/include/BridgeTestFoundation.h | 3 + test/SILGen/objc_blocks_bridging.swift | 22 +++- test/SILOptimizer/Inputs/Closure.h | 6 + .../definite-init-convert-to-escape.swift | 55 +++++++++ 10 files changed, 233 insertions(+), 23 deletions(-) create mode 100644 test/SILOptimizer/Inputs/Closure.h create mode 100644 test/SILOptimizer/definite-init-convert-to-escape.swift diff --git a/lib/SILGen/SILGenBuilder.cpp b/lib/SILGen/SILGenBuilder.cpp index a05ed75a31985..8b28f40570ae4 100644 --- a/lib/SILGen/SILGenBuilder.cpp +++ b/lib/SILGen/SILGenBuilder.cpp @@ -209,7 +209,8 @@ ManagedValue SILGenBuilder::createConvertFunction(SILLocation loc, ManagedValue SILGenBuilder::createConvertEscapeToNoEscape(SILLocation loc, ManagedValue fn, - SILType resultTy) { + SILType resultTy, + bool dontPostponeToNoEscapeCleanup) { auto fnType = fn.getType().castTo(); auto resultFnType = resultTy.castTo(); @@ -221,9 +222,12 @@ ManagedValue SILGenBuilder::createConvertEscapeToNoEscape(SILLocation loc, !fnType->isNoEscape() && resultFnType->isNoEscape() && "Expect a escaping to noescape conversion"); - SILValue fnValue = fn.ensurePlusOne(SGF, loc).forward(SGF); + SILValue fnValue = dontPostponeToNoEscapeCleanup + ? fn.getValue() + : fn.ensurePlusOne(SGF, loc).forward(SGF); SILValue result = createConvertEscapeToNoEscape(loc, fnValue, resultTy); - getSILGenFunction().enterPostponedCleanup(fnValue); + if (!dontPostponeToNoEscapeCleanup) + getSILGenFunction().enterPostponedCleanup(fnValue); return ManagedValue::forTrivialObjectRValue(result); } diff --git a/lib/SILGen/SILGenBuilder.h b/lib/SILGen/SILGenBuilder.h index 828a45e493b1a..00b534c9b868c 100644 --- a/lib/SILGen/SILGenBuilder.h +++ b/lib/SILGen/SILGenBuilder.h @@ -361,8 +361,10 @@ class SILGenBuilder : public SILBuilder { SILType resultTy); using SILBuilder::createConvertEscapeToNoEscape; - ManagedValue createConvertEscapeToNoEscape(SILLocation loc, ManagedValue fn, - SILType resultTy); + ManagedValue + createConvertEscapeToNoEscape(SILLocation loc, ManagedValue fn, + SILType resultTy, + bool dontPostponeToNoEscapeCleanup = false); using SILBuilder::createStore; /// Forward \p value into \p address. diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index a6c5d8998a542..6aa14c4e4e136 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -2090,8 +2090,11 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e, if (srcRepTy != srcTy) result = convertFunctionRepresentation(SGF, e, result, srcRepTy, srcTy); - if (srcTy != destTy) - result = SGF.emitTransformedValue(e, result, srcTy, destTy); + if (srcTy != destTy) { + bool dontPostponeToNoEscapeCleanup = isa(e->getSubExpr()); + result = SGF.emitTransformedValue(e, result, srcTy, destTy, SGFContext(), + dontPostponeToNoEscapeCleanup); + } if (destTy != destRepTy) result = convertFunctionRepresentation(SGF, e, result, destTy, destRepTy); diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 2b63382464b49..ee456792c0c52 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -1617,7 +1617,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction ManagedValue emitTransformedValue(SILLocation loc, ManagedValue input, CanType inputType, CanType outputType, - SGFContext ctx = SGFContext()); + SGFContext ctx = SGFContext(), + bool dontPostponeToNoEscapeCleanup = false); /// Most general form of the above. ManagedValue emitTransformedValue(SILLocation loc, ManagedValue input, @@ -1625,7 +1626,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction CanType inputSubstType, AbstractionPattern outputOrigType, CanType outputSubstType, - SGFContext ctx = SGFContext()); + SGFContext ctx = SGFContext(), + bool dontPostponeToNoEscapeCleanup = false); RValue emitTransformedValue(SILLocation loc, RValue &&input, AbstractionPattern inputOrigType, CanType inputSubstType, diff --git a/lib/SILGen/SILGenPoly.cpp b/lib/SILGen/SILGenPoly.cpp index 5f501dc0d6104..1da94df622f05 100644 --- a/lib/SILGen/SILGenPoly.cpp +++ b/lib/SILGen/SILGenPoly.cpp @@ -136,7 +136,8 @@ namespace { CanType inputSubstType, AbstractionPattern outputOrigType, CanType outputSubstType, - SGFContext ctxt); + SGFContext ctxt, + bool dontPostponeToNoEscapeCleanup = false); /// Transform a metatype value. ManagedValue transformMetatype(ManagedValue fn, @@ -159,7 +160,8 @@ namespace { CanAnyFunctionType inputSubstType, AbstractionPattern outputOrigType, CanAnyFunctionType outputSubstType, - const TypeLowering &expectedTL); + const TypeLowering &expectedTL, + bool dontPostponeToNoEscapeCleanup = false); }; } // end anonymous namespace ; @@ -356,7 +358,8 @@ ManagedValue Transform::transform(ManagedValue v, CanType inputSubstType, AbstractionPattern outputOrigType, CanType outputSubstType, - SGFContext ctxt) { + SGFContext ctxt, + bool dontPostponeToNoEscapeCleanup) { // Look through inout types. if (isa(inputSubstType)) inputSubstType = CanType(inputSubstType->getInOutObjectType()); @@ -440,7 +443,8 @@ ManagedValue Transform::transform(ManagedValue v, return transformFunction(v, inputOrigType, inputFnType, outputOrigType, outputFnType, - expectedTL); + expectedTL, + dontPostponeToNoEscapeCleanup); } // - tuples of transformable values @@ -3041,7 +3045,8 @@ ManagedValue Transform::transformFunction(ManagedValue fn, CanAnyFunctionType inputSubstType, AbstractionPattern outputOrigType, CanAnyFunctionType outputSubstType, - const TypeLowering &expectedTL) { + const TypeLowering &expectedTL, + bool dontPostponeToNoEscapeCleanup) { assert(fn.getType().isObject() && "expected input to emitTransformedFunctionValue to be loaded"); @@ -3100,7 +3105,7 @@ ManagedValue Transform::transformFunction(ManagedValue fn, } else if (newFnType != expectedFnType) { // Escaping to noescape conversion. SILType resTy = SILType::getPrimitiveObjectType(expectedFnType); - fn = SGF.B.createConvertEscapeToNoEscape(Loc, fn, resTy); + fn = SGF.B.createConvertEscapeToNoEscape(Loc, fn, resTy, dontPostponeToNoEscapeCleanup); } return fn; @@ -3196,11 +3201,12 @@ ManagedValue SILGenFunction::emitTransformedValue(SILLocation loc, ManagedValue v, CanType inputType, CanType outputType, - SGFContext ctxt) { + SGFContext ctxt, + bool dontPostponeToNoEscapeCleanup) { return emitTransformedValue(loc, v, AbstractionPattern(inputType), inputType, AbstractionPattern(outputType), outputType, - ctxt); + ctxt, dontPostponeToNoEscapeCleanup); } ManagedValue @@ -3209,12 +3215,14 @@ SILGenFunction::emitTransformedValue(SILLocation loc, ManagedValue v, CanType inputSubstType, AbstractionPattern outputOrigType, CanType outputSubstType, - SGFContext ctxt) { + SGFContext ctxt, + bool dontPostponeToNoEscapeCleanup) { return Transform(*this, loc).transform(v, inputOrigType, inputSubstType, outputOrigType, - outputSubstType, ctxt); + outputSubstType, ctxt, + dontPostponeToNoEscapeCleanup); } RValue diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index 485057d2c8184..9ee0129109977 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -2952,6 +2952,114 @@ static bool lowerRawSILOperations(SILFunction &Fn) { return Changed; } +static SILBasicBlock *getOptionalDiamondSuccessor(SwitchEnumInst *SEI) { + auto numSuccs = SEI->getNumSuccessors(); + if (numSuccs != 2) + return nullptr; + auto *SuccSome = SEI->getCase(0).second; + auto *SuccNone = SEI->getCase(1).second; + if (SuccSome->args_size() != 1) + std::swap(SuccSome, SuccNone); + + if (SuccSome->args_size() != 1 || SuccNone->args_size() != 0) + return nullptr; + + auto *Succ = SuccSome->getSingleSuccessorBlock(); + if (!Succ) + return nullptr; + + if (SuccNone == Succ) + return Succ; + + SuccNone = SuccNone->getSingleSuccessorBlock(); + if (SuccNone == Succ) + return Succ; + + SuccNone = SuccNone->getSingleSuccessorBlock(); + if (SuccNone == Succ) + return Succ; + + return nullptr; +} +/// Ensure the lifetime of the closure accross an +/// +/// optional<@escaping () -> ()> to +/// optional<@noescape @convention(block) () -> ()> +/// +/// conversion and its use. +/// +/// The pattern this is looking for +/// switch_enum %closure +/// / \ +/// convert_escape_to_noescape nil +/// switch_enum +/// / \ +/// convertToBlock nil +/// \ / +/// (%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. +static bool fixupConvertEscapeToNoEscapeLifetime(SILFunction &Fn) { + bool Changed = false; + for (auto &BB : Fn) { + auto I = BB.begin(); + while (I != BB.end()) { + SILInstruction *Inst = &*I; + ++I; + auto *convertEscapeToNoescape = + dyn_cast(Inst); + if (!convertEscapeToNoescape) + continue; + auto *blockArg = + dyn_cast(convertEscapeToNoescape->getOperand()); + if (!blockArg) + continue; + auto *PredBB = + convertEscapeToNoescape->getParent()->getSinglePredecessorBlock(); + if (!PredBB) + continue; + auto *ConvertSuccessorBlock = convertEscapeToNoescape->getParent()->getSingleSuccessorBlock(); + if (!ConvertSuccessorBlock) + continue; + auto *SwitchEnum1 = dyn_cast(PredBB->getTerminator()); + if (!SwitchEnum1) + continue; + auto *DiamondSucc = getOptionalDiamondSuccessor(SwitchEnum1); + if (!DiamondSucc) + continue; + auto *SwitchEnum2 = dyn_cast(DiamondSucc->getTerminator()); + if (!SwitchEnum2) + continue; + auto *DiamondSucc2 = getOptionalDiamondSuccessor(SwitchEnum2); + if (!DiamondSucc2) + continue; + if (DiamondSucc2->getNumArguments() != 1) + continue; + SILInstruction *onlyDestroy = [&]() -> SILInstruction * { + SILInstruction *lastDestroy = nullptr; + for (auto *Use : DiamondSucc2->getArgument(0)->getUses()) { + SILInstruction *Usr = Use->getUser(); + if (isa(Usr) || isa(Usr) || + isa(Usr)) { + if (lastDestroy) + return nullptr; + lastDestroy = Usr; + } + } + return lastDestroy; + }(); + if (!onlyDestroy) + continue; + SILBuilderWithScope B(SwitchEnum1); + auto copy = + B.createCopyValue(SwitchEnum1->getLoc(), SwitchEnum1->getOperand()); + B.setInsertionPoint(onlyDestroy); + B.createDestroyValue(SwitchEnum1->getLoc(), copy); + } + } + return Changed; +} namespace { /// Perform definitive initialization analysis and promote alloc_box uses into @@ -2974,6 +3082,11 @@ class DefiniteInitialization : public SILFunctionTransform { // Lower raw-sil only instructions used by this pass, like "assign". if (lowerRawSILOperations(*getFunction())) invalidateAnalysis(SILAnalysis::InvalidationKind::FunctionBody); + + // Fixup lifetimes of optional convertEscapeToNoEscape. + if (fixupConvertEscapeToNoEscapeLifetime(*getFunction())) + invalidateAnalysis(SILAnalysis::InvalidationKind::FunctionBody); + } }; diff --git a/test/SILGen/Inputs/usr/include/BridgeTestFoundation.h b/test/SILGen/Inputs/usr/include/BridgeTestFoundation.h index 456b446659aff..2ac7d33af2ab3 100644 --- a/test/SILGen/Inputs/usr/include/BridgeTestFoundation.h +++ b/test/SILGen/Inputs/usr/include/BridgeTestFoundation.h @@ -82,6 +82,9 @@ void nonnullSetBlockResult(NSSet *_Nonnull (^block)(void)); void noescapeBlock(__attribute__((noescape)) void (^block)(void)); void escapeBlock(void (^block)(void)); +void noescapeBlock3(__attribute__((noescape)) void (^block)(NSString *s), + __attribute__((noescape)) void (^block2)(NSString *s), + NSString *f); void noescapeNonnullBlock(__attribute__((noescape)) void (^_Nonnull block)(void)); void escapeNonnullBlock(void (^_Nonnull block)(void)); diff --git a/test/SILGen/objc_blocks_bridging.swift b/test/SILGen/objc_blocks_bridging.swift index 904faf551aa27..c5d8d0a3ccecc 100644 --- a/test/SILGen/objc_blocks_bridging.swift +++ b/test/SILGen/objc_blocks_bridging.swift @@ -170,9 +170,9 @@ func bridgeNonnullBlockResult() { nonnullStringBlockResult { return "test" } } -// CHECK-LABEL: sil hidden @$S20objc_blocks_bridging19bridgeNoescapeBlock2fnyyyXE_tF -func bridgeNoescapeBlock(fn: () -> ()) { - // CHECK: [[CLOSURE_FN:%.*]] = function_ref @$S20objc_blocks_bridging19bridgeNoescapeBlock2fnyyyXE_tFyyXEfU_ +// CHECK-LABEL: sil hidden @$S20objc_blocks_bridging19bridgeNoescapeBlock2fn5optFnyyyXE_yycSgtF +func bridgeNoescapeBlock(fn: () -> (), optFn: (() -> ())?) { + // CHECK: [[CLOSURE_FN:%.*]] = function_ref @$S20objc_blocks_bridging19bridgeNoescapeBlock2fn5optFnyyyXE_yycSgtFyyXEfU_ // CHECK: [[CONV_FN:%.*]] = convert_function [[CLOSURE_FN]] // CHECK: [[THICK_FN:%.*]] = thin_to_thick_function [[CONV_FN]] // CHECK: [[BLOCK_ALLOC:%.*]] = alloc_stack $@block_storage @noescape @callee_guaranteed () -> () @@ -212,7 +212,7 @@ func bridgeNoescapeBlock(fn: () -> ()) { // CHECK: apply [[FN]]([[NIL_BLOCK]]) noescapeBlock(nil) - // CHECK: [[CLOSURE_FN:%.*]] = function_ref @$S20objc_blocks_bridging19bridgeNoescapeBlock2fnyyyXE_tFyyXEfU0_ + // CHECK: [[CLOSURE_FN:%.*]] = function_ref @$S20objc_blocks_bridging19bridgeNoescapeBlock2fn5optFnyyyXE_yycSgtF // CHECK: [[CONV_FN:%.*]] = convert_function [[CLOSURE_FN]] // CHECK: [[THICK_FN:%.*]] = thin_to_thick_function [[CONV_FN]] // CHECK: [[BLOCK_ALLOC:%.*]] = alloc_stack $@block_storage @noescape @callee_guaranteed () -> () @@ -242,6 +242,8 @@ func bridgeNoescapeBlock(fn: () -> ()) { // CHECK: apply [[FN]]([[BLOCK]]) noescapeNonnullBlock(fn) + noescapeBlock(optFn) + noescapeBlockAlias { } noescapeBlockAlias(fn) noescapeBlockAlias(nil) @@ -250,6 +252,18 @@ func bridgeNoescapeBlock(fn: () -> ()) { noescapeNonnullBlockAlias(fn) } +public func bridgeNoescapeBlock( optFn: ((String?) -> ())?, optFn2: ((String?) -> ())?) { + noescapeBlock3(optFn, optFn2, "Foobar") +} + + +@_silgen_name("_returnOptionalEscape") +public func returnOptionalEscape() -> (() ->())? + +public func bridgeNoescapeBlock() { + noescapeBlock(returnOptionalEscape()) +} + class ObjCClass : NSObject {} extension ObjCClass { diff --git a/test/SILOptimizer/Inputs/Closure.h b/test/SILOptimizer/Inputs/Closure.h new file mode 100644 index 0000000000000..1929dcae4c3cb --- /dev/null +++ b/test/SILOptimizer/Inputs/Closure.h @@ -0,0 +1,6 @@ +#import + +void noescapeBlock(__attribute__((noescape)) void (^block)(void)); +void noescapeBlock3(__attribute__((noescape)) void (^block)(NSString *s), + __attribute__((noescape)) void (^block2)(NSString *s), + NSString *f); diff --git a/test/SILOptimizer/definite-init-convert-to-escape.swift b/test/SILOptimizer/definite-init-convert-to-escape.swift new file mode 100644 index 0000000000000..8ee4877b95a8e --- /dev/null +++ b/test/SILOptimizer/definite-init-convert-to-escape.swift @@ -0,0 +1,55 @@ +// RUN: %target-swift-frontend -module-name A -verify -emit-sil -import-objc-header %S/Inputs/Closure.h -disable-objc-attr-requires-foundation-module -enable-sil-ownership %s | %FileCheck %s + +import Foundation + +// Make sure that we keep the escaping closures alive accross the ultimate call. +// CHECK-LABEL: sil @$S1A19bridgeNoescapeBlock5optFn0D3Fn2yySSSgcSg_AFtF +// CHECK: bb0 +// CHECK: retain_value %0 +// CHECK: retain_value %0 +// CHECK: bb2 +// CHECK: convert_escape_to_noescape +// CHECK: strong_release +// CHECK: bb6 +// CHECK: retain_value %1 +// CHECK: retain_value %1 +// CHECK: bb8 +// CHECK: convert_escape_to_noescape +// CHECK: strong_release +// CHECK: bb12 +// CHECK: [[F:%.*]] = function_ref @noescapeBlock3 +// CHECK: apply [[F]] +// CHECK: release_value {{.*}} : $Optional +// CHECK: release_value %1 : $Optional<@callee_guaranteed (@guaranteed Optional) -> ()> +// CHECK: release_value {{.*}} : $Optional<@convention(block) @noescape (Optional) -> ()> +// CHECK: release_value %0 : $Optional<@callee_guaranteed (@guaranteed Optional) -> ()> +// CHECK: release_value {{.*}} : $Optional<@convention(block) @noescape (Optional) +public func bridgeNoescapeBlock( optFn: ((String?) -> ())?, optFn2: ((String?) -> ())?) { + noescapeBlock3(optFn, optFn2, "Foobar") +} + + +@_silgen_name("_returnOptionalEscape") +public func returnOptionalEscape() -> (() ->())? + +// Make sure that we keep the escaping closure alive accross the ultimate call. + +// CHECK-LABEL: sil @$S1A19bridgeNoescapeBlockyyF : $@convention(thin) () -> () { +// CHECK: bb0: +// CHECK: [[V0:%.*]] = function_ref @_returnOptionalEscape +// CHECK: [[V1:%.*]] = apply [[V0]] +// CHECK: retain_value [[V1]] +// CHECK: switch_enum {{.*}}bb2 +// CHECK: bb2([[V2:%.*]]: $@callee_guaranteed () -> ()): +// CHECK: convert_escape_to_noescape +// CHECK: strong_release [[V2]] +// CHECK: bb6({{.*}} : $Optional<@convention(block) @noescape () -> ()>) +// CHECK: [[F:%.*]] = function_ref @noescapeBlock +// CHECK: apply [[F]]({{.*}}) +// CHECK: release_value [[V1]] : $Optional<@callee_guaranteed () -> ()> + +public func bridgeNoescapeBlock() { + noescapeBlock(returnOptionalEscape()) +} + + From 0db9f6cf8ba2e0c9c3895539cb458c21379d01fc Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Thu, 29 Mar 2018 18:54:18 -0700 Subject: [PATCH 2/9] Disable definite-init-convert-to-escape.swift test case on linux --- test/SILOptimizer/definite-init-convert-to-escape.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/SILOptimizer/definite-init-convert-to-escape.swift b/test/SILOptimizer/definite-init-convert-to-escape.swift index 8ee4877b95a8e..d3afc0fd923ac 100644 --- a/test/SILOptimizer/definite-init-convert-to-escape.swift +++ b/test/SILOptimizer/definite-init-convert-to-escape.swift @@ -1,5 +1,7 @@ // RUN: %target-swift-frontend -module-name A -verify -emit-sil -import-objc-header %S/Inputs/Closure.h -disable-objc-attr-requires-foundation-module -enable-sil-ownership %s | %FileCheck %s +// REQUIRES: objc_interop + import Foundation // Make sure that we keep the escaping closures alive accross the ultimate call. From fca180f62c7ea83e93182f4e75b2bf43579e64cc Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 08:46:58 -0700 Subject: [PATCH 3/9] SILGen: Remove variable with negated name: --- lib/SILGen/SILGenBuilder.cpp | 16 ++++++++-------- lib/SILGen/SILGenBuilder.h | 2 +- lib/SILGen/SILGenExpr.cpp | 4 ++-- lib/SILGen/SILGenFunction.h | 4 ++-- lib/SILGen/SILGenPoly.cpp | 21 +++++++++++---------- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/SILGen/SILGenBuilder.cpp b/lib/SILGen/SILGenBuilder.cpp index 8b28f40570ae4..d08a4b7ea4f0d 100644 --- a/lib/SILGen/SILGenBuilder.cpp +++ b/lib/SILGen/SILGenBuilder.cpp @@ -207,10 +207,10 @@ ManagedValue SILGenBuilder::createConvertFunction(SILLocation loc, return cloner.clone(result); } -ManagedValue SILGenBuilder::createConvertEscapeToNoEscape(SILLocation loc, - ManagedValue fn, - SILType resultTy, - bool dontPostponeToNoEscapeCleanup) { +ManagedValue SILGenBuilder::createConvertEscapeToNoEscape( + SILLocation loc, ManagedValue fn, SILType resultTy, + bool postponeToNoEscapeCleanup) { + auto fnType = fn.getType().castTo(); auto resultFnType = resultTy.castTo(); @@ -222,11 +222,11 @@ ManagedValue SILGenBuilder::createConvertEscapeToNoEscape(SILLocation loc, !fnType->isNoEscape() && resultFnType->isNoEscape() && "Expect a escaping to noescape conversion"); - SILValue fnValue = dontPostponeToNoEscapeCleanup - ? fn.getValue() - : fn.ensurePlusOne(SGF, loc).forward(SGF); + SILValue fnValue = postponeToNoEscapeCleanup + ? fn.ensurePlusOne(SGF, loc).forward(SGF) + : fn.getValue(); SILValue result = createConvertEscapeToNoEscape(loc, fnValue, resultTy); - if (!dontPostponeToNoEscapeCleanup) + if (postponeToNoEscapeCleanup) getSILGenFunction().enterPostponedCleanup(fnValue); return ManagedValue::forTrivialObjectRValue(result); } diff --git a/lib/SILGen/SILGenBuilder.h b/lib/SILGen/SILGenBuilder.h index 00b534c9b868c..a59d8fec8f435 100644 --- a/lib/SILGen/SILGenBuilder.h +++ b/lib/SILGen/SILGenBuilder.h @@ -364,7 +364,7 @@ class SILGenBuilder : public SILBuilder { ManagedValue createConvertEscapeToNoEscape(SILLocation loc, ManagedValue fn, SILType resultTy, - bool dontPostponeToNoEscapeCleanup = false); + bool postponeToNoEscapeCleanup = true); using SILBuilder::createStore; /// Forward \p value into \p address. diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 6aa14c4e4e136..fbb86e7e87fa0 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -2091,9 +2091,9 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e, result = convertFunctionRepresentation(SGF, e, result, srcRepTy, srcTy); if (srcTy != destTy) { - bool dontPostponeToNoEscapeCleanup = isa(e->getSubExpr()); + bool postponeToNoEscapeCleanup = !isa(e->getSubExpr()); result = SGF.emitTransformedValue(e, result, srcTy, destTy, SGFContext(), - dontPostponeToNoEscapeCleanup); + postponeToNoEscapeCleanup); } if (destTy != destRepTy) diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index ee456792c0c52..fb77e77789cad 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -1618,7 +1618,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction CanType inputType, CanType outputType, SGFContext ctx = SGFContext(), - bool dontPostponeToNoEscapeCleanup = false); + bool postponeToNoEscapeCleanup = true); /// Most general form of the above. ManagedValue emitTransformedValue(SILLocation loc, ManagedValue input, @@ -1627,7 +1627,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction AbstractionPattern outputOrigType, CanType outputSubstType, SGFContext ctx = SGFContext(), - bool dontPostponeToNoEscapeCleanup = false); + bool postponeToNoEscapeCleanup = true); RValue emitTransformedValue(SILLocation loc, RValue &&input, AbstractionPattern inputOrigType, CanType inputSubstType, diff --git a/lib/SILGen/SILGenPoly.cpp b/lib/SILGen/SILGenPoly.cpp index 1da94df622f05..bbc0ea596d3b5 100644 --- a/lib/SILGen/SILGenPoly.cpp +++ b/lib/SILGen/SILGenPoly.cpp @@ -137,7 +137,7 @@ namespace { AbstractionPattern outputOrigType, CanType outputSubstType, SGFContext ctxt, - bool dontPostponeToNoEscapeCleanup = false); + bool postponeToNoEscapeCleanup = true); /// Transform a metatype value. ManagedValue transformMetatype(ManagedValue fn, @@ -161,7 +161,7 @@ namespace { AbstractionPattern outputOrigType, CanAnyFunctionType outputSubstType, const TypeLowering &expectedTL, - bool dontPostponeToNoEscapeCleanup = false); + bool postponeToNoEscapeCleanup = true); }; } // end anonymous namespace ; @@ -359,7 +359,7 @@ ManagedValue Transform::transform(ManagedValue v, AbstractionPattern outputOrigType, CanType outputSubstType, SGFContext ctxt, - bool dontPostponeToNoEscapeCleanup) { + bool postponeToNoEscapeCleanup) { // Look through inout types. if (isa(inputSubstType)) inputSubstType = CanType(inputSubstType->getInOutObjectType()); @@ -444,7 +444,7 @@ ManagedValue Transform::transform(ManagedValue v, inputOrigType, inputFnType, outputOrigType, outputFnType, expectedTL, - dontPostponeToNoEscapeCleanup); + postponeToNoEscapeCleanup); } // - tuples of transformable values @@ -3046,7 +3046,7 @@ ManagedValue Transform::transformFunction(ManagedValue fn, AbstractionPattern outputOrigType, CanAnyFunctionType outputSubstType, const TypeLowering &expectedTL, - bool dontPostponeToNoEscapeCleanup) { + bool postponeToNoEscapeCleanup) { assert(fn.getType().isObject() && "expected input to emitTransformedFunctionValue to be loaded"); @@ -3105,7 +3105,8 @@ ManagedValue Transform::transformFunction(ManagedValue fn, } else if (newFnType != expectedFnType) { // Escaping to noescape conversion. SILType resTy = SILType::getPrimitiveObjectType(expectedFnType); - fn = SGF.B.createConvertEscapeToNoEscape(Loc, fn, resTy, dontPostponeToNoEscapeCleanup); + fn = SGF.B.createConvertEscapeToNoEscape(Loc, fn, resTy, + postponeToNoEscapeCleanup); } return fn; @@ -3202,11 +3203,11 @@ SILGenFunction::emitTransformedValue(SILLocation loc, ManagedValue v, CanType inputType, CanType outputType, SGFContext ctxt, - bool dontPostponeToNoEscapeCleanup) { + bool postponeToNoEscapeCleanup) { return emitTransformedValue(loc, v, AbstractionPattern(inputType), inputType, AbstractionPattern(outputType), outputType, - ctxt, dontPostponeToNoEscapeCleanup); + ctxt, postponeToNoEscapeCleanup); } ManagedValue @@ -3216,13 +3217,13 @@ SILGenFunction::emitTransformedValue(SILLocation loc, ManagedValue v, AbstractionPattern outputOrigType, CanType outputSubstType, SGFContext ctxt, - bool dontPostponeToNoEscapeCleanup) { + bool postponeToNoEscapeCleanup) { return Transform(*this, loc).transform(v, inputOrigType, inputSubstType, outputOrigType, outputSubstType, ctxt, - dontPostponeToNoEscapeCleanup); + postponeToNoEscapeCleanup); } RValue From bca456ecd5bf77028e1a79f0291130d0870c581c Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 08:53:40 -0700 Subject: [PATCH 4/9] Add a convert_escape_to_noescape [not_guaranteed] variant --- include/swift/SIL/SILBuilder.h | 6 ++++-- include/swift/SIL/SILCloner.h | 8 +++---- include/swift/SIL/SILInstruction.h | 16 ++++++++++---- include/swift/Serialization/ModuleFormat.h | 2 +- lib/IRGen/LoadableByAddress.cpp | 3 ++- lib/ParseSIL/ParseSIL.cpp | 6 +++++- lib/SIL/SILInstructions.cpp | 4 ++-- lib/SIL/SILPrinter.cpp | 3 ++- lib/SILOptimizer/IPO/ClosureSpecializer.cpp | 4 ++-- lib/Serialization/DeserializeSIL.cpp | 13 +++++++++++- lib/Serialization/SerializeSIL.cpp | 23 ++++++++++++++++----- 11 files changed, 64 insertions(+), 24 deletions(-) diff --git a/include/swift/SIL/SILBuilder.h b/include/swift/SIL/SILBuilder.h index b9d82b153901b..377c3cbdd46bd 100644 --- a/include/swift/SIL/SILBuilder.h +++ b/include/swift/SIL/SILBuilder.h @@ -797,9 +797,11 @@ class SILBuilder { } ConvertEscapeToNoEscapeInst * - createConvertEscapeToNoEscape(SILLocation Loc, SILValue Op, SILType Ty) { + createConvertEscapeToNoEscape(SILLocation Loc, SILValue Op, SILType Ty, + bool lifetimeGuaranteed) { return insert(ConvertEscapeToNoEscapeInst::create( - getSILDebugLocation(Loc), Op, Ty, getFunction(), OpenedArchetypes)); + getSILDebugLocation(Loc), Op, Ty, getFunction(), OpenedArchetypes, + lifetimeGuaranteed)); } ThinFunctionToPointerInst * diff --git a/include/swift/SIL/SILCloner.h b/include/swift/SIL/SILCloner.h index 239c114a742b6..3138d24804b98 100644 --- a/include/swift/SIL/SILCloner.h +++ b/include/swift/SIL/SILCloner.h @@ -984,10 +984,10 @@ template void SILCloner::visitConvertEscapeToNoEscapeInst( ConvertEscapeToNoEscapeInst *Inst) { getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope())); - doPostProcess(Inst, getBuilder().createConvertEscapeToNoEscape( - getOpLocation(Inst->getLoc()), - getOpValue(Inst->getOperand()), - getOpType(Inst->getType()))); + doPostProcess( + Inst, getBuilder().createConvertEscapeToNoEscape( + getOpLocation(Inst->getLoc()), getOpValue(Inst->getOperand()), + getOpType(Inst->getType()), Inst->isLifetimeGuaranteed())); } template diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index fffdcb9254c68..668769597974a 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -3946,18 +3946,26 @@ class ConvertEscapeToNoEscapeInst final ConvertEscapeToNoEscapeInst, ConversionInst> { friend SILBuilder; + bool lifetimeGuaranteed; + ConvertEscapeToNoEscapeInst(SILDebugLocation DebugLoc, SILValue Operand, - ArrayRef TypeDependentOperands, - SILType Ty) + ArrayRef TypeDependentOperands, + SILType Ty, bool isLifetimeGuaranteed) : UnaryInstructionWithTypeDependentOperandsBase( - DebugLoc, Operand, TypeDependentOperands, Ty) { + DebugLoc, Operand, TypeDependentOperands, Ty), + lifetimeGuaranteed(isLifetimeGuaranteed) { assert(!Operand->getType().castTo()->isNoEscape()); assert(Ty.castTo()->isNoEscape()); } static ConvertEscapeToNoEscapeInst * create(SILDebugLocation DebugLoc, SILValue Operand, SILType Ty, - SILFunction &F, SILOpenedArchetypesState &OpenedArchetypes); + SILFunction &F, SILOpenedArchetypesState &OpenedArchetypes, + bool lifetimeGuaranteed); +public: + bool isLifetimeGuaranteed() const { + return lifetimeGuaranteed; + } }; /// ThinFunctionToPointerInst - Convert a thin function pointer to a diff --git a/include/swift/Serialization/ModuleFormat.h b/include/swift/Serialization/ModuleFormat.h index 84beb4f621355..a4308e6c837c9 100644 --- a/include/swift/Serialization/ModuleFormat.h +++ b/include/swift/Serialization/ModuleFormat.h @@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0; /// describe what change you made. The content of this comment isn't important; /// it just ensures a conflict if two people change the module format. /// Don't worry about adhering to the 80-column limit for this line. -const uint16_t VERSION_MINOR = 404; // Last change: inheritsSuperclassInitializers +const uint16_t VERSION_MINOR = 407; // Last change: convert_escape_to_noescape using DeclIDField = BCFixed<31>; diff --git a/lib/IRGen/LoadableByAddress.cpp b/lib/IRGen/LoadableByAddress.cpp index 4d63ace834041..f9e18dc11c930 100644 --- a/lib/IRGen/LoadableByAddress.cpp +++ b/lib/IRGen/LoadableByAddress.cpp @@ -2494,7 +2494,8 @@ void LoadableByAddress::recreateConvInstrs() { case SILInstructionKind::ConvertEscapeToNoEscapeInst: { auto instr = cast(convInstr); newInstr = convBuilder.createConvertEscapeToNoEscape( - instr->getLoc(), instr->getOperand(), newType); + instr->getLoc(), instr->getOperand(), newType, + instr->isLifetimeGuaranteed()); break; } case SILInstructionKind::MarkDependenceInst: { diff --git a/lib/ParseSIL/ParseSIL.cpp b/lib/ParseSIL/ParseSIL.cpp index 5175609097d2e..977b8d67c2fad 100644 --- a/lib/ParseSIL/ParseSIL.cpp +++ b/lib/ParseSIL/ParseSIL.cpp @@ -2979,6 +2979,10 @@ bool SILParser::parseSILInstruction(SILBuilder &B) { SILType Ty; Identifier ToToken; SourceLoc ToLoc; + bool guaranteed = true; + if (Opcode == SILInstructionKind::ConvertEscapeToNoEscapeInst) + if(parseSILOptional(guaranteed, *this, "not_guaranteed")) + return true; if (parseTypedValueRef(Val, B) || parseSILIdentifier(ToToken, ToLoc, diag::expected_tok_in_sil_instr, "to") || @@ -3012,7 +3016,7 @@ bool SILParser::parseSILInstruction(SILBuilder &B) { ResultVal = B.createConvertFunction(InstLoc, Val, Ty); break; case SILInstructionKind::ConvertEscapeToNoEscapeInst: - ResultVal = B.createConvertEscapeToNoEscape(InstLoc, Val, Ty); + ResultVal = B.createConvertEscapeToNoEscape(InstLoc, Val, Ty, guaranteed); break; case SILInstructionKind::AddressToPointerInst: ResultVal = B.createAddressToPointer(InstLoc, Val, Ty); diff --git a/lib/SIL/SILInstructions.cpp b/lib/SIL/SILInstructions.cpp index 74d7a3cc659c2..dec0573fbc51f 100644 --- a/lib/SIL/SILInstructions.cpp +++ b/lib/SIL/SILInstructions.cpp @@ -1998,7 +1998,7 @@ ConvertFunctionInst::create(SILDebugLocation DebugLoc, SILValue Operand, ConvertEscapeToNoEscapeInst *ConvertEscapeToNoEscapeInst::create( SILDebugLocation DebugLoc, SILValue Operand, SILType Ty, SILFunction &F, - SILOpenedArchetypesState &OpenedArchetypes) { + SILOpenedArchetypesState &OpenedArchetypes, bool isLifetimeGuaranteed) { SILModule &Mod = F.getModule(); SmallVector TypeDependentOperands; collectTypeDependentOperands(TypeDependentOperands, OpenedArchetypes, F, @@ -2007,7 +2007,7 @@ ConvertEscapeToNoEscapeInst *ConvertEscapeToNoEscapeInst::create( totalSizeToAlloc(1 + TypeDependentOperands.size()); void *Buffer = Mod.allocateInst(size, alignof(ConvertEscapeToNoEscapeInst)); auto *CFI = ::new (Buffer) ConvertEscapeToNoEscapeInst( - DebugLoc, Operand, TypeDependentOperands, Ty); + DebugLoc, Operand, TypeDependentOperands, Ty, isLifetimeGuaranteed); // If we do not have lowered SIL, make sure that are not performing // ABI-incompatible conversions. // diff --git a/lib/SIL/SILPrinter.cpp b/lib/SIL/SILPrinter.cpp index c5b54a68f69dc..e1870e6c346e2 100644 --- a/lib/SIL/SILPrinter.cpp +++ b/lib/SIL/SILPrinter.cpp @@ -1435,7 +1435,8 @@ class SILPrinter : public SILInstructionVisitor { printUncheckedConversionInst(CI, CI->getOperand()); } void visitConvertEscapeToNoEscapeInst(ConvertEscapeToNoEscapeInst *CI) { - printUncheckedConversionInst(CI, CI->getOperand()); + *this << (CI->isLifetimeGuaranteed() ? "" : "[not_guaranteed] ") + << getIDAndType(CI->getOperand()) << " to " << CI->getType(); } void visitThinFunctionToPointerInst(ThinFunctionToPointerInst *CI) { printUncheckedConversionInst(CI, CI->getOperand()); diff --git a/lib/SILOptimizer/IPO/ClosureSpecializer.cpp b/lib/SILOptimizer/IPO/ClosureSpecializer.cpp index ebcce9be05360..6beb3d7878a3f 100644 --- a/lib/SILOptimizer/IPO/ClosureSpecializer.cpp +++ b/lib/SILOptimizer/IPO/ClosureSpecializer.cpp @@ -662,8 +662,8 @@ SILValue ClosureSpecCloner::cloneCalleeConversion(SILValue calleeValue, auto *Cvt = cast(calleeValue); calleeValue = cloneCalleeConversion(Cvt->getOperand(), NewClosure, Builder); - return Builder.createConvertEscapeToNoEscape(CallSiteDesc.getLoc(), - calleeValue, Cvt->getType()); + return Builder.createConvertEscapeToNoEscape( + CallSiteDesc.getLoc(), calleeValue, Cvt->getType(), true); } /// \brief Populate the body of the cloned closure, modifying instructions as diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp index b4d07b3c740d7..6a103a94c8a63 100644 --- a/lib/Serialization/DeserializeSIL.cpp +++ b/lib/Serialization/DeserializeSIL.cpp @@ -1066,7 +1066,6 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn, SILBasicBlock *BB, ONEOPERAND_ONETYPE_INST(ObjCMetatypeToObject) ONEOPERAND_ONETYPE_INST(ObjCExistentialMetatypeToObject) ONEOPERAND_ONETYPE_INST(ConvertFunction) - ONEOPERAND_ONETYPE_INST(ConvertEscapeToNoEscape) ONEOPERAND_ONETYPE_INST(ThinFunctionToPointer) ONEOPERAND_ONETYPE_INST(PointerToThinFunction) ONEOPERAND_ONETYPE_INST(ProjectBlockStorage) @@ -1082,6 +1081,18 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn, SILBasicBlock *BB, TyID); break; } + case SILInstructionKind::ConvertEscapeToNoEscapeInst: { + assert(RecordKind == SIL_ONE_TYPE_ONE_OPERAND && + "Layout should be OneTypeOneOperand."); + bool isLifetimeGuaranteed = Attr & 0x01; + ResultVal = Builder.createConvertEscapeToNoEscape( + Loc, + getLocalValue(ValID, getSILType(MF->getType(TyID2), + (SILValueCategory)TyCategory2)), + getSILType(MF->getType(TyID), (SILValueCategory)TyCategory), + isLifetimeGuaranteed); + break; + } case SILInstructionKind::PointerToAddressInst: { assert(RecordKind == SIL_ONE_TYPE_ONE_OPERAND && diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp index 34c65b569d22e..7844533df7dbd 100644 --- a/lib/Serialization/SerializeSIL.cpp +++ b/lib/Serialization/SerializeSIL.cpp @@ -250,7 +250,8 @@ namespace { void writeSILBlock(const SILModule *SILMod); void writeIndexTables(); - void writeConversionLikeInstruction(const SingleValueInstruction *I); + void writeConversionLikeInstruction(const SingleValueInstruction *I, + bool guaranteed); void writeOneTypeLayout(SILInstructionKind valueKind, SILType type); void writeOneTypeOneOperandLayout(SILInstructionKind valueKind, unsigned attrs, @@ -579,10 +580,18 @@ void SILSerializer::writeOneTypeOneOperandLayout(SILInstructionKind valueKind, /// Write an instruction that looks exactly like a conversion: all /// important information is encoded in the operand and the result type. -void SILSerializer::writeConversionLikeInstruction(const SingleValueInstruction *I) { +__attribute__((noinline)) +static unsigned getZeroOrOne(bool getOne) { + if (getOne) + return 0x1; + return 0x0; +} + +void SILSerializer::writeConversionLikeInstruction( + const SingleValueInstruction *I, bool guaranteed) { assert(I->getNumOperands() - I->getTypeDependentOperands().size() == 1); - writeOneTypeOneOperandLayout(I->getKind(), 0, I->getType(), - I->getOperand(0)); + writeOneTypeOneOperandLayout(I->getKind(), getZeroOrOne(guaranteed), + I->getType(), I->getOperand(0)); } void @@ -1446,7 +1455,11 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) { case SILInstructionKind::ObjCMetatypeToObjectInst: case SILInstructionKind::ObjCExistentialMetatypeToObjectInst: case SILInstructionKind::ProjectBlockStorageInst: { - writeConversionLikeInstruction(cast(&SI)); + bool guaranteed = false; + if (SI.getKind() == SILInstructionKind::ConvertEscapeToNoEscapeInst) + guaranteed = cast(SI).isLifetimeGuaranteed(); + writeConversionLikeInstruction(cast(&SI), + guaranteed); break; } case SILInstructionKind::PointerToAddressInst: { From ccc81ba1c30c857fb4a9f4cffdf1273f9948c971 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 08:55:24 -0700 Subject: [PATCH 5/9] Use the convert_escape_to_noescape [not_guaranteed] instruction --- lib/SILGen/SILGenBuilder.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/SILGen/SILGenBuilder.cpp b/lib/SILGen/SILGenBuilder.cpp index d08a4b7ea4f0d..e69658ad4cb5d 100644 --- a/lib/SILGen/SILGenBuilder.cpp +++ b/lib/SILGen/SILGenBuilder.cpp @@ -225,7 +225,8 @@ ManagedValue SILGenBuilder::createConvertEscapeToNoEscape( SILValue fnValue = postponeToNoEscapeCleanup ? fn.ensurePlusOne(SGF, loc).forward(SGF) : fn.getValue(); - SILValue result = createConvertEscapeToNoEscape(loc, fnValue, resultTy); + SILValue result = createConvertEscapeToNoEscape(loc, fnValue, resultTy, + postponeToNoEscapeCleanup); if (postponeToNoEscapeCleanup) getSILGenFunction().enterPostponedCleanup(fnValue); return ManagedValue::forTrivialObjectRValue(result); From 50fecdc1b69d032aa6ea057b7ffd19d00ebd1627 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 08:56:13 -0700 Subject: [PATCH 6/9] Rewrite DI to always guaranteed the lifetime of the convert_escape_to_noescape [not_guaranteed] operand Either through a peephole or we keep the closure operand alive to the end of the function rdar://38124009 --- .../Mandatory/DefiniteInitialization.cpp | 185 +++++++++++++----- .../definite-init-convert-to-escape.swift | 21 ++ 2 files changed, 159 insertions(+), 47 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index 9ee0129109977..5440897e823e7 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -2981,6 +2981,59 @@ static SILBasicBlock *getOptionalDiamondSuccessor(SwitchEnumInst *SEI) { return nullptr; } + +/// Extend the lifetime of the convert_escape_to_noescape's operand to the end +/// of the function. +void extendLifetimeToEndOfFunction(SILFunction &Fn, + ConvertEscapeToNoEscapeInst *Cvt) { + auto EscapingClosure = Cvt->getOperand(); + auto EscapingClosureTy = EscapingClosure->getType(); + auto OptionalEscapingClosureTy = SILType::getOptionalType(EscapingClosureTy); + auto loc = RegularLocation::getAutoGeneratedLocation(); + + SILBuilderWithScope B(Cvt); + auto NewCvt = B.createConvertEscapeToNoEscape( + Cvt->getLoc(), Cvt->getOperand(), Cvt->getType(), true); + Cvt->replaceAllUsesWith(NewCvt); + Cvt->eraseFromParent(); + Cvt = NewCvt; + + // Create an alloc_stack Optional<() -> ()> at the beginning of the function. + AllocStackInst *Slot; + auto &Context = Cvt->getModule().getASTContext(); + { + SILBuilderWithScope B(Fn.getEntryBlock()->begin()); + Slot = B.createAllocStack(loc, OptionalEscapingClosureTy); + auto *NoneDecl = Context.getOptionalNoneDecl(); + // Store None to it. + B.createStore( + loc, B.createEnum(loc, SILValue(), NoneDecl, OptionalEscapingClosureTy), + Slot, StoreOwnershipQualifier::Init); + } + // Insert a copy before the convert_escape_to_noescape and store it to the + // alloc_stack location. + { + SILBuilderWithScope B(Cvt); + auto *SomeDecl = Context.getOptionalSomeDecl(); + B.createDestroyAddr(loc, Slot); + auto ClosureCopy = B.createCopyValue(loc, EscapingClosure); + B.createStore( + loc, + B.createEnum(loc, ClosureCopy, SomeDecl, OptionalEscapingClosureTy), + Slot, StoreOwnershipQualifier::Init); + } + // Insert destroys at the function exits. + SmallVector ExitingBlocks; + Fn.findExitingBlocks(ExitingBlocks); + for (auto *Exit : ExitingBlocks) { + auto *Term = Exit->getTerminator(); + SILBuilderWithScope B(Term); + B.setInsertionPoint(Term); + B.createDestroyAddr(loc, Slot); + B.createDeallocStack(loc, Slot); + } +} + /// Ensure the lifetime of the closure accross an /// /// optional<@escaping () -> ()> to @@ -3000,6 +3053,82 @@ static SILBasicBlock *getOptionalDiamondSuccessor(SwitchEnumInst *SEI) { /// 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. +static bool trySwitchEnumPeephole(ConvertEscapeToNoEscapeInst *Cvt) { + auto *blockArg = dyn_cast(Cvt->getOperand()); + if (!blockArg) + return false; + auto *PredBB = Cvt->getParent()->getSinglePredecessorBlock(); + if (!PredBB) + return false; + auto *ConvertSuccessorBlock = Cvt->getParent()->getSingleSuccessorBlock(); + if (!ConvertSuccessorBlock) + return false; + auto *SwitchEnum1 = dyn_cast(PredBB->getTerminator()); + if (!SwitchEnum1) + return false; + auto *DiamondSucc = getOptionalDiamondSuccessor(SwitchEnum1); + if (!DiamondSucc) + return false; + auto *SwitchEnum2 = dyn_cast(DiamondSucc->getTerminator()); + if (!SwitchEnum2) + return false; + auto *DiamondSucc2 = getOptionalDiamondSuccessor(SwitchEnum2); + if (!DiamondSucc2) + return false; + if (DiamondSucc2->getNumArguments() != 1) + return false; + + // Look for the last and only destroy. + SILInstruction *onlyDestroy = [&]() -> SILInstruction * { + SILInstruction *lastDestroy = nullptr; + for (auto *Use : DiamondSucc2->getArgument(0)->getUses()) { + SILInstruction *Usr = Use->getUser(); + if (isa(Usr) || isa(Usr) || + isa(Usr)) { + if (lastDestroy) + return nullptr; + lastDestroy = Usr; + } + } + return lastDestroy; + }(); + if (!onlyDestroy) + return false; + + // Replace the convert_escape_to_noescape instruction. + { + SILBuilderWithScope B(Cvt); + auto NewCvt = B.createConvertEscapeToNoEscape( + Cvt->getLoc(), Cvt->getOperand(), Cvt->getType(), true); + Cvt->replaceAllUsesWith(NewCvt); + Cvt->eraseFromParent(); + Cvt = NewCvt; + } + + // Extend the lifetime. + SILBuilderWithScope B(SwitchEnum1); + auto loc = RegularLocation::getAutoGeneratedLocation(); + auto copy = + B.createCopyValue(loc, SwitchEnum1->getOperand()); + B.setInsertionPoint(onlyDestroy); + B.createDestroyValue(loc, copy); + return true; +} + +llvm::cl::opt DIDisableConvertEscapeToNoEscapeSwitchEnumPeephole( + "sil-di-disable-convert-escape-to-noescape-switch-peephole", + llvm::cl::init(false), + llvm::cl::desc( + "Disable the convert_escape_to_noescape switch enum peephole. "), + llvm::cl::Hidden); + +/// Fix-up the lifetime of the escaping closure argument of +/// convert_escape_to_noescape [not_guaranteed] instructions. +/// +/// convert_escape_to_noescape [not_guaranteed] assume that someone guarantees +/// the lifetime of the operand for the duration of the trivial closure result. +/// SILGen does not guarantee this for '[not_guaranteed]' instructions so we +/// ensure it here. static bool fixupConvertEscapeToNoEscapeLifetime(SILFunction &Fn) { bool Changed = false; for (auto &BB : Fn) { @@ -3007,55 +3136,17 @@ static bool fixupConvertEscapeToNoEscapeLifetime(SILFunction &Fn) { while (I != BB.end()) { SILInstruction *Inst = &*I; ++I; - auto *convertEscapeToNoescape = - dyn_cast(Inst); - if (!convertEscapeToNoescape) - continue; - auto *blockArg = - dyn_cast(convertEscapeToNoescape->getOperand()); - if (!blockArg) - continue; - auto *PredBB = - convertEscapeToNoescape->getParent()->getSinglePredecessorBlock(); - if (!PredBB) - continue; - auto *ConvertSuccessorBlock = convertEscapeToNoescape->getParent()->getSingleSuccessorBlock(); - if (!ConvertSuccessorBlock) + auto *Cvt = dyn_cast(Inst); + if (!Cvt || Cvt->isLifetimeGuaranteed()) continue; - auto *SwitchEnum1 = dyn_cast(PredBB->getTerminator()); - if (!SwitchEnum1) + // First try to peephole a known pattern. + if (!DIDisableConvertEscapeToNoEscapeSwitchEnumPeephole && + (Changed |= trySwitchEnumPeephole(Cvt))) continue; - auto *DiamondSucc = getOptionalDiamondSuccessor(SwitchEnum1); - if (!DiamondSucc) - continue; - auto *SwitchEnum2 = dyn_cast(DiamondSucc->getTerminator()); - if (!SwitchEnum2) - continue; - auto *DiamondSucc2 = getOptionalDiamondSuccessor(SwitchEnum2); - if (!DiamondSucc2) - continue; - if (DiamondSucc2->getNumArguments() != 1) - continue; - SILInstruction *onlyDestroy = [&]() -> SILInstruction * { - SILInstruction *lastDestroy = nullptr; - for (auto *Use : DiamondSucc2->getArgument(0)->getUses()) { - SILInstruction *Usr = Use->getUser(); - if (isa(Usr) || isa(Usr) || - isa(Usr)) { - if (lastDestroy) - return nullptr; - lastDestroy = Usr; - } - } - return lastDestroy; - }(); - if (!onlyDestroy) - continue; - SILBuilderWithScope B(SwitchEnum1); - auto copy = - B.createCopyValue(SwitchEnum1->getLoc(), SwitchEnum1->getOperand()); - B.setInsertionPoint(onlyDestroy); - B.createDestroyValue(SwitchEnum1->getLoc(), copy); + // Otherwise, extend the lifetime of the operand to the end of the + // function. + extendLifetimeToEndOfFunction(Fn, Cvt); + Changed |= true; } } return Changed; diff --git a/test/SILOptimizer/definite-init-convert-to-escape.swift b/test/SILOptimizer/definite-init-convert-to-escape.swift index d3afc0fd923ac..a101406e3e449 100644 --- a/test/SILOptimizer/definite-init-convert-to-escape.swift +++ b/test/SILOptimizer/definite-init-convert-to-escape.swift @@ -1,4 +1,5 @@ // RUN: %target-swift-frontend -module-name A -verify -emit-sil -import-objc-header %S/Inputs/Closure.h -disable-objc-attr-requires-foundation-module -enable-sil-ownership %s | %FileCheck %s +// RUN: %target-swift-frontend -module-name A -verify -emit-sil -import-objc-header %S/Inputs/Closure.h -disable-objc-attr-requires-foundation-module -enable-sil-ownership -Xllvm -sil-di-disable-convert-escape-to-noescape-switch-peephole %s | %FileCheck %s --check-prefix=NOPEEPHOLE // REQUIRES: objc_interop @@ -50,6 +51,26 @@ public func returnOptionalEscape() -> (() ->())? // CHECK: apply [[F]]({{.*}}) // CHECK: release_value [[V1]] : $Optional<@callee_guaranteed () -> ()> +// NOPEEPHOLE-LABEL: sil @$S1A19bridgeNoescapeBlockyyF : $@convention(thin) () -> () { +// NOPEEPHOLE: bb0: +// NOPEEPHOLE: [[SLOT:%.*]] = alloc_stack $Optional<@callee_guaranteed () -> ()> +// NOPEEPHOLE: [[NONE:%.*]] = enum $Optional +// NOPEEPHOLE: store [[NONE]] to [[SLOT]] +// NOPEEPHOLE: [[V0:%.*]] = function_ref @_returnOptionalEscape +// NOPEEPHOLE: [[V1:%.*]] = apply [[V0]] +// NOPEEPHOLE: switch_enum {{.*}}bb2 +// NOPEEPHOLE: bb2([[V2:%.*]]: $@callee_guaranteed () -> ()): +// NOPEEPHOLE: destroy_addr [[SLOT]] +// NOPEEPHOLE: [[SOME:%.*]] = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt.1, [[V2]] +// NOPEEPHOLE: store [[SOME]] to [[SLOT]] +// NOPEEPHOLE: convert_escape_to_noescape +// NOPEEPHOLE-NOT: strong_release +// NOPEEPHOLE: br +// NOPEEPHOLE: bb6({{.*}} : $Optional<@convention(block) @noescape () -> ()>) +// NOPEEPHOLE: [[F:%.*]] = function_ref @noescapeBlock +// NOPEEPHOLE: apply [[F]]({{.*}}) +// NOPEEPHOLE: destroy_addr [[SLOT]] +// NOPEEPHOLE: dealloc_stack [[SLOT]] public func bridgeNoescapeBlock() { noescapeBlock(returnOptionalEscape()) } From cf05fe0ec5384c2966af0bc36fd2bd58b551462b Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 09:46:52 -0700 Subject: [PATCH 7/9] Remove hack --- lib/Serialization/SerializeSIL.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp index 7844533df7dbd..eaad5e22c14fd 100644 --- a/lib/Serialization/SerializeSIL.cpp +++ b/lib/Serialization/SerializeSIL.cpp @@ -580,18 +580,11 @@ void SILSerializer::writeOneTypeOneOperandLayout(SILInstructionKind valueKind, /// Write an instruction that looks exactly like a conversion: all /// important information is encoded in the operand and the result type. -__attribute__((noinline)) -static unsigned getZeroOrOne(bool getOne) { - if (getOne) - return 0x1; - return 0x0; -} - void SILSerializer::writeConversionLikeInstruction( const SingleValueInstruction *I, bool guaranteed) { assert(I->getNumOperands() - I->getTypeDependentOperands().size() == 1); - writeOneTypeOneOperandLayout(I->getKind(), getZeroOrOne(guaranteed), - I->getType(), I->getOperand(0)); + writeOneTypeOneOperandLayout(I->getKind(), guaranteed ? 1 : 0, I->getType(), + I->getOperand(0)); } void From d417880b47ca2051737c8da189c345961c176e12 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 12:41:31 -0700 Subject: [PATCH 8/9] Fix logic error in fixupConvertEscapeToNoEscapeLifetime --- test/SILOptimizer/definite-init-convert-to-escape.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/SILOptimizer/definite-init-convert-to-escape.swift b/test/SILOptimizer/definite-init-convert-to-escape.swift index a101406e3e449..74d8d99d6214c 100644 --- a/test/SILOptimizer/definite-init-convert-to-escape.swift +++ b/test/SILOptimizer/definite-init-convert-to-escape.swift @@ -11,13 +11,13 @@ import Foundation // CHECK: retain_value %0 // CHECK: retain_value %0 // CHECK: bb2 -// CHECK: convert_escape_to_noescape +// CHECK: convert_escape_to_noescape % // CHECK: strong_release // CHECK: bb6 // CHECK: retain_value %1 // CHECK: retain_value %1 // CHECK: bb8 -// CHECK: convert_escape_to_noescape +// CHECK: convert_escape_to_noescape % // CHECK: strong_release // CHECK: bb12 // CHECK: [[F:%.*]] = function_ref @noescapeBlock3 @@ -44,7 +44,7 @@ public func returnOptionalEscape() -> (() ->())? // CHECK: retain_value [[V1]] // CHECK: switch_enum {{.*}}bb2 // CHECK: bb2([[V2:%.*]]: $@callee_guaranteed () -> ()): -// CHECK: convert_escape_to_noescape +// CHECK: convert_escape_to_noescape % // CHECK: strong_release [[V2]] // CHECK: bb6({{.*}} : $Optional<@convention(block) @noescape () -> ()>) // CHECK: [[F:%.*]] = function_ref @noescapeBlock @@ -63,7 +63,7 @@ public func returnOptionalEscape() -> (() ->())? // NOPEEPHOLE: destroy_addr [[SLOT]] // NOPEEPHOLE: [[SOME:%.*]] = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt.1, [[V2]] // NOPEEPHOLE: store [[SOME]] to [[SLOT]] -// NOPEEPHOLE: convert_escape_to_noescape +// NOPEEPHOLE: convert_escape_to_noescape % // NOPEEPHOLE-NOT: strong_release // NOPEEPHOLE: br // NOPEEPHOLE: bb6({{.*}} : $Optional<@convention(block) @noescape () -> ()>) From 39a0334f2e9dd2c252aad43466a6a5e8a4db0c44 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 30 Mar 2018 12:55:41 -0700 Subject: [PATCH 9/9] Really fix the logic error in fixupConvertEscapeToNoEscapeLifetime --- lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index 5440897e823e7..bc5b5a469e95e 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -3139,10 +3139,14 @@ static bool fixupConvertEscapeToNoEscapeLifetime(SILFunction &Fn) { auto *Cvt = dyn_cast(Inst); if (!Cvt || Cvt->isLifetimeGuaranteed()) continue; + // First try to peephole a known pattern. if (!DIDisableConvertEscapeToNoEscapeSwitchEnumPeephole && - (Changed |= trySwitchEnumPeephole(Cvt))) + trySwitchEnumPeephole(Cvt)) { + Changed |= true; continue; + } + // Otherwise, extend the lifetime of the operand to the end of the // function. extendLifetimeToEndOfFunction(Fn, Cvt);