From cb8760148d6861eafed25b5a7b3a7518166c0b51 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Sat, 6 Dec 2014 23:08:17 +0000 Subject: [PATCH] Reapply "Revert "[func-sig-opts] If we know all callsites of a function, delete the old unoptimized function instead of creating a thunk."" This reverts commit r23725. This time with fixes so we don't hit that assertion. Swift SVN r23762 --- include/swift/SIL/SILFunction.h | 4 +++ lib/SILPasses/FunctionSignatureOpts.cpp | 38 +++++++++++++++++++++---- test/SILPasses/functionsigopts.sil | 24 ++++++++++++++-- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/include/swift/SIL/SILFunction.h b/include/swift/SIL/SILFunction.h index 0d636c4ccfa..a202c103ef4 100644 --- a/include/swift/SIL/SILFunction.h +++ b/include/swift/SIL/SILFunction.h @@ -154,6 +154,10 @@ public: return LoweredType; } + bool canBeDeleted() const { + return !getRefCount() && !isZombie(); + } + /// Return the number of entities referring to this function (other /// than the SILModule). unsigned getRefCount() const { return RefCount; } diff --git a/lib/SILPasses/FunctionSignatureOpts.cpp b/lib/SILPasses/FunctionSignatureOpts.cpp index ae63217f1c5..98da3f96321 100644 --- a/lib/SILPasses/FunctionSignatureOpts.cpp +++ b/lib/SILPasses/FunctionSignatureOpts.cpp @@ -18,6 +18,7 @@ #include "swift/SILPasses/Transforms.h" #include "swift/SILPasses/Utils/Local.h" #include "swift/Basic/LLVM.h" +#include "swift/Basic/BlotMapVector.h" #include "swift/Basic/Range.h" #include "swift/SIL/Projection.h" #include "swift/SIL/SILFunction.h" @@ -569,8 +570,9 @@ rewriteApplyInstToCallNewFunction(FunctionAnalyzer &Analyzer, SILFunction *NewF, Builder.createReleaseValue(Loc, AI->getArgument(ArgDesc.Index)); } - // Erase the old apply. - AI->eraseFromParent(); + // Erase the old apply and its callee. + recursivelyDeleteTriviallyDeadInstructions(AI, true, + [](SILInstruction *){}); ++NumCallSitesOptimized; } @@ -641,7 +643,7 @@ moveFunctionBodyToNewFunctionWithName(SILFunction *F, ArgOffset = ArgDesc.updateOptimizedBBArgs(Builder, NewFEntryBB, ArgOffset); } - // Then create the new thunk body since NewF has the right signature now. + // Otherwise generate the thunk body just in case. SILBasicBlock *ThunkBody = F->createBasicBlock(); for (auto &ArgDesc : ArgDescs) { ThunkBody->createBBArg(ArgDesc.ParameterInfo.getSILType(), @@ -659,9 +661,11 @@ moveFunctionBodyToNewFunctionWithName(SILFunction *F, /// function returns true if we were successful in creating the new function and /// returns false otherwise. static bool -optimizeFunctionSignature(SILFunction *F, +optimizeFunctionSignature(RCIdentityAnalysis *RCIA, + SILFunction *F, CallGraphNode::CallerCallSiteList CallSites, - RCIdentityAnalysis *RCIA) { + bool CallerSetIsComplete, + std::vector &DeadFunctions) { DEBUG(llvm::dbgs() << "Optimizing Function Signature of " << F->getName() << "\n"); @@ -716,6 +720,12 @@ optimizeFunctionSignature(SILFunction *F, // appropriate given the form of function signature optimization performed. rewriteApplyInstToCallNewFunction(Analyzer, NewF, CallSites); + // Now that we have rewritten all apply insts that referenced the old + // function, if the caller set was complete, delete the old function. + if (CallerSetIsComplete) { + DeadFunctions.push_back(F); + } + return true; } @@ -796,6 +806,9 @@ public: // those calls. bool Changed = false; + std::vector DeadFunctions; + DeadFunctions.reserve(128); + for (auto &F : *M) { // Check the signature of F to make sure that it is a function that we can // specialize. These are conditions independent of the call graph. @@ -820,8 +833,21 @@ public: if (CallSites.empty()) continue; + // Check if we know the callgraph is complete with respect to this + // function. In such a case, we don't need to generate the thunk. + bool CallerSetIsComplete = FNode->isCallerSetComplete(); + // Otherwise, try to optimize the function signature of F. - Changed |= optimizeFunctionSignature(&F, CallSites, RCIA); + Changed |= optimizeFunctionSignature(RCIA, &F, CallSites, + CallerSetIsComplete, + DeadFunctions); + } + + while (!DeadFunctions.empty()) { + SILFunction *F = DeadFunctions.back(); + if (F->canBeDeleted()) + M->eraseFunction(F); + DeadFunctions.pop_back(); } // If we changed anything, invalidate the call graph. diff --git a/test/SILPasses/functionsigopts.sil b/test/SILPasses/functionsigopts.sil index 10a31df3dda..31bf86b937d 100644 --- a/test/SILPasses/functionsigopts.sil +++ b/test/SILPasses/functionsigopts.sil @@ -54,6 +54,24 @@ bb3: return %4 : $() } +// CHECK-NOT: sil hidden [fragile] @private_dead_arg_with_callsites : $@thin (Builtin.NativeObject, Builtin.NativeObject) -> () { +sil private [fragile] @private_dead_arg_with_callsites : $@thin (Builtin.NativeObject, Builtin.NativeObject) -> () { +bb0(%0 : $Builtin.NativeObject, %1 : $Builtin.NativeObject): + cond_br undef, bb1, bb2 + +bb1: + br bb3 + +bb2: + br bb3 + +bb3: + %2 = function_ref @user : $@thin (Builtin.NativeObject) -> () + %3 = apply %2(%1) : $@thin (Builtin.NativeObject) -> () + %4 = tuple() + return %4 : $() +} + // CHECK-LABEL: sil [fragile] @dead_arg_callsite_1 : $@thin (Builtin.NativeObject, Builtin.NativeObject) -> () { // CHECK: bb0 // CHECK: [[OPT_FUN:%[0-9]+]] = function_ref @_TTOS_dn_dead_arg_with_callsites : $@thin (Builtin.NativeObject) -> () @@ -66,8 +84,10 @@ bb0(%0 : $Builtin.NativeObject, %1 : $Builtin.NativeObject): // This ensures that %0 is not dead in this function. We don't want to specialize this. apply %4(%0) : $@thin (Builtin.NativeObject) -> () - %5 = tuple() - return %5 : $() + %5 = function_ref @private_dead_arg_with_callsites : $@thin (Builtin.NativeObject, Builtin.NativeObject) -> () + %6 = apply %5(%0, %1) : $@thin (Builtin.NativeObject, Builtin.NativeObject) -> () + %7 = tuple() + return %7 : $() } // This test makes sure we can look up an already specialized callee instead of regenerating one.