Skip to content

Commit 0de00d1

Browse files
gottesmmgottesmm
committed
[sil-inst-opt] Improve performance of InstModCallbacks by eliminating indirect call along default callback path.
Specifically before this PR, if a caller did not customize a specific callback of InstModCallbacks, we would store a static default std::function into InstModCallbacks. This means that we always would have an indirect jump. That is unfortunate since this code is often called in loops. In this PR, I eliminate this problem by: 1. I made all of the actual callback std::function in InstModCallback private and gave them a "Func" postfix (e.x.: deleteInst -> deleteInstFunc). 2. I created public methods with the old callback names to actually call the callbacks. This ensured that as long as we are not escaping callbacks from InstModCallback, this PR would not result in the need for any source changes since we are changing a call of a std::function field to a call to a method. 3. I changed all of the places that were escaping inst mod's callbacks to take an InstModCallback. We shouldn't be doing that anyway. 4. I changed the default value of each callback in InstModCallbacks to be a nullptr and changed the public helper methods to check if a callback is null. If the callback is not null, it is called, otherwise the getter falls back to an inline default implementation of the operation. All together this means that the cost of a plain InstModCallback is reduced and one pays an indirect function cost price as one customizes it further which is better scalability. P.S. as a little extra thing, I added a madeChange field onto the InstModCallback. Now that we have the helpers calling the callbacks, I can easily insert instrumentation like this, allowing for users to pass in InstModCallback and see if anything was RAUWed without needing to specify a callback.
1 parent 79c4e3c commit 0de00d1

15 files changed

+204
-213
lines changed

‎include/swift/SILOptimizer/Analysis/SimplifyInstruction.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
namespace swift {
2626

2727
class SILInstruction;
28+
class InstModCallbacks;
2829

2930
/// Try to simplify the specified instruction, performing local
3031
/// analysis of the operands of the instruction, without looking at its uses
@@ -46,11 +47,17 @@ SILValue simplifyInstruction(SILInstruction *I);
4647
///
4748
/// NOTE: When OSSA is enabled this API assumes OSSA is properly formed and will
4849
/// insert compensating instructions.
49-
SILBasicBlock::iterator replaceAllSimplifiedUsesAndErase(
50-
SILInstruction *I, SILValue result,
51-
std::function<void(SILInstruction *)> eraseNotify = nullptr,
52-
std::function<void(SILInstruction *)> newInstNotify = nullptr,
53-
DeadEndBlocks *deadEndBlocks = nullptr);
50+
SILBasicBlock::iterator
51+
replaceAllSimplifiedUsesAndErase(SILInstruction *I, SILValue result,
52+
InstModCallbacks &callbacks,
53+
DeadEndBlocks *deadEndBlocks = nullptr);
54+
55+
/// This is a low level routine that makes all uses of \p svi uses of \p
56+
/// newValue (ignoring end scope markers) and then deletes \p svi and all end
57+
/// scope markers. Then returns the next inst to process.
58+
SILBasicBlock::iterator replaceAllUsesAndErase(SingleValueInstruction *svi,
59+
SILValue newValue,
60+
InstModCallbacks &callbacks);
5461

5562
/// Simplify invocations of builtin operations that may overflow.
5663
/// All such operations return a tuple (result, overflow_flag).

‎include/swift/SILOptimizer/Utils/CFGOptUtils.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ namespace swift {
3636
class DominanceInfo;
3737
class SILLoop;
3838
class SILLoopInfo;
39-
struct InstModCallbacks;
39+
class InstModCallbacks;
4040

4141
/// Adds a new argument to an edge between a branch and a destination
4242
/// block. Allows for user injected callbacks via \p callbacks.
@@ -47,8 +47,7 @@ struct InstModCallbacks;
4747
/// \return The created branch. The old branch is deleted.
4848
/// The argument is appended at the end of the argument tuple.
4949
TermInst *addNewEdgeValueToBranch(TermInst *branch, SILBasicBlock *dest,
50-
SILValue val,
51-
const InstModCallbacks &callbacks);
50+
SILValue val, InstModCallbacks &callbacks);
5251

5352
/// Adds a new argument to an edge between a branch and a destination
5453
/// block.
@@ -60,7 +59,8 @@ TermInst *addNewEdgeValueToBranch(TermInst *branch, SILBasicBlock *dest,
6059
/// The argument is appended at the end of the argument tuple.
6160
inline TermInst *addNewEdgeValueToBranch(TermInst *branch, SILBasicBlock *dest,
6261
SILValue val) {
63-
return addNewEdgeValueToBranch(branch, dest, val, InstModCallbacks());
62+
InstModCallbacks callbacks;
63+
return addNewEdgeValueToBranch(branch, dest, val, callbacks);
6464
}
6565

6666
/// Changes the edge value between a branch and destination basic block

‎include/swift/SILOptimizer/Utils/CanonicalizeInstruction.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "swift/SIL/BasicBlockUtils.h"
3030
#include "swift/SIL/SILBasicBlock.h"
3131
#include "swift/SIL/SILInstruction.h"
32+
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
3233
#include "llvm/Support/Debug.h"
3334

3435
namespace swift {
@@ -40,10 +41,18 @@ struct CanonicalizeInstruction {
4041
static constexpr const char *defaultDebugType = "sil-canonicalize";
4142
const char *debugType = defaultDebugType;
4243
DeadEndBlocks &deadEndBlocks;
44+
InstModCallbacks callbacks;
4345

4446
CanonicalizeInstruction(const char *passDebugType,
4547
DeadEndBlocks &deadEndBlocks)
46-
: deadEndBlocks(deadEndBlocks) {
48+
: deadEndBlocks(deadEndBlocks),
49+
callbacks(
50+
[&](SILInstruction *toDelete) { killInstruction(toDelete); },
51+
[&](SILInstruction *newInst) { notifyNewInstruction(newInst); },
52+
[&](SILValue oldValue, SILValue newValue) {
53+
oldValue->replaceAllUsesWith(newValue);
54+
notifyHasNewUsers(newValue);
55+
}) {
4756
#ifndef NDEBUG
4857
if (llvm::DebugFlag && !llvm::isCurrentDebugType(debugType))
4958
debugType = passDebugType;

‎include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 84 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -310,44 +310,97 @@ bool tryCheckedCastBrJumpThreading(
310310

311311
/// A structure containing callbacks that are called when an instruction is
312312
/// removed or added.
313-
struct InstModCallbacks {
314-
static const std::function<void(SILInstruction *)> defaultDeleteInst;
315-
static const std::function<void(SILInstruction *)> defaultCreatedNewInst;
316-
static const std::function<void(SILValue, SILValue)> defaultReplaceValueUsesWith;
317-
static const std::function<void(SingleValueInstruction *, SILValue)>
318-
defaultEraseAndRAUWSingleValueInst;
319-
320-
std::function<void(SILInstruction *)> deleteInst =
321-
InstModCallbacks::defaultDeleteInst;
322-
std::function<void(SILInstruction *)> createdNewInst =
323-
InstModCallbacks::defaultCreatedNewInst;
324-
std::function<void(SILValue, SILValue)>
325-
replaceValueUsesWith =
326-
InstModCallbacks::defaultReplaceValueUsesWith;
327-
std::function<void(SingleValueInstruction *, SILValue)>
328-
eraseAndRAUWSingleValueInst =
329-
InstModCallbacks::defaultEraseAndRAUWSingleValueInst;
330-
331-
InstModCallbacks(decltype(deleteInst) deleteInst,
332-
decltype(createdNewInst) createdNewInst,
333-
decltype(replaceValueUsesWith) replaceValueUsesWith)
334-
: deleteInst(deleteInst), createdNewInst(createdNewInst),
335-
replaceValueUsesWith(replaceValueUsesWith),
336-
eraseAndRAUWSingleValueInst(
337-
InstModCallbacks::defaultEraseAndRAUWSingleValueInst) {}
313+
///
314+
/// PERFORMANCE NOTES: This code can be used in loops, so we want to make sure
315+
/// to not have overhead when the user does not specify a callback. To do that
316+
/// instead of defining a "default" std::function, we represent the "default"
317+
/// functions as nullptr. Then, in the helper function trampoline that actually
318+
/// gets called, we check if we have a nullptr and if we do, we perform the
319+
/// default operation inline. What is nice about this from a perf perspective is
320+
/// that in a loop this property should predict well since you have a single
321+
/// branch that is going to go the same way everytime.
322+
class InstModCallbacks {
323+
/// A function that takes in an instruction and deletes the inst.
324+
///
325+
/// Default implementation is instToDelete->eraseFromParent();
326+
std::function<void(SILInstruction *instToDelete)> deleteInstFunc;
327+
328+
/// A function that is called to notify that a new function was created.
329+
///
330+
/// Default implementation is a no-op, but we still mark madeChange.
331+
std::function<void(SILInstruction *newlyCreatedInst)> createdNewInstFunc;
332+
333+
/// A function that first replaces all uses of oldValue with uses of newValue.
334+
///
335+
/// Default implementation just calls oldValue->replaceAllUsesWith(newValue);
336+
std::function<void(SILValue oldValue, SILValue newValue)>
337+
replaceValueUsesWithFunc;
338+
339+
/// A function that first replaces all uses of oldValue with uses of newValue
340+
/// and then erases oldValue.
341+
///
342+
/// Default implementation just calls replaceValueUsesWithFunc and then
343+
/// deleteInstFunc.
344+
std::function<void(SingleValueInstruction *oldValue, SILValue newValue)>
345+
eraseAndRAUWSingleValueInstFunc;
346+
347+
/// A boolean that tracks if any of our callbacks were ever called.
348+
bool madeChange = false;
349+
350+
public:
351+
InstModCallbacks(decltype(deleteInstFunc) deleteInstFunc)
352+
: deleteInstFunc(deleteInstFunc) {}
353+
354+
InstModCallbacks(decltype(deleteInstFunc) deleteInstFunc,
355+
decltype(createdNewInstFunc) createdNewInstFunc,
356+
decltype(replaceValueUsesWithFunc) replaceValueUsesWithFunc)
357+
: deleteInstFunc(deleteInstFunc), createdNewInstFunc(createdNewInstFunc),
358+
replaceValueUsesWithFunc(replaceValueUsesWithFunc) {}
338359

339360
InstModCallbacks(
340-
decltype(deleteInst) deleteInst, decltype(createdNewInst) createdNewInst,
341-
decltype(replaceValueUsesWith) replaceValueUsesWith,
342-
decltype(eraseAndRAUWSingleValueInst) eraseAndRAUWSingleValueInst)
343-
: deleteInst(deleteInst), createdNewInst(createdNewInst),
344-
replaceValueUsesWith(replaceValueUsesWith),
345-
eraseAndRAUWSingleValueInst(eraseAndRAUWSingleValueInst) {}
361+
decltype(deleteInstFunc) deleteInstFunc,
362+
decltype(createdNewInstFunc) createdNewInstFunc,
363+
decltype(replaceValueUsesWithFunc) replaceValueUsesWithFunc,
364+
decltype(eraseAndRAUWSingleValueInstFunc) eraseAndRAUWSingleValueInstFunc)
365+
: deleteInstFunc(deleteInstFunc), createdNewInstFunc(createdNewInstFunc),
366+
replaceValueUsesWithFunc(replaceValueUsesWithFunc),
367+
eraseAndRAUWSingleValueInstFunc(eraseAndRAUWSingleValueInstFunc) {}
346368

347369
InstModCallbacks() = default;
348370
~InstModCallbacks() = default;
349371
InstModCallbacks(const InstModCallbacks &) = default;
350372
InstModCallbacks(InstModCallbacks &&) = default;
373+
374+
void deleteInst(SILInstruction *instToDelete) {
375+
madeChange = true;
376+
if (deleteInstFunc)
377+
return deleteInstFunc(instToDelete);
378+
instToDelete->eraseFromParent();
379+
}
380+
381+
void createdNewInst(SILInstruction *newlyCreatedInst) {
382+
madeChange = true;
383+
if (createdNewInstFunc)
384+
createdNewInstFunc(newlyCreatedInst);
385+
}
386+
387+
void replaceValueUsesWith(SILValue oldValue, SILValue newValue) {
388+
madeChange = true;
389+
if (replaceValueUsesWithFunc)
390+
return replaceValueUsesWithFunc(oldValue, newValue);
391+
oldValue->replaceAllUsesWith(newValue);
392+
}
393+
394+
void eraseAndRAUWSingleValueInst(SingleValueInstruction *oldInst,
395+
SILValue newValue) {
396+
madeChange = true;
397+
if (eraseAndRAUWSingleValueInstFunc)
398+
return eraseAndRAUWSingleValueInstFunc(oldInst, newValue);
399+
replaceValueUsesWith(oldInst, newValue);
400+
deleteInst(oldInst);
401+
}
402+
403+
bool getMadeChange() const { return madeChange; }
351404
};
352405

353406
/// Get all consumed arguments of a partial_apply.

‎include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@
2121

2222
#include "swift/SIL/OwnershipUtils.h"
2323
#include "swift/SIL/SILModule.h"
24+
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
2425

2526
namespace swift {
2627

2728
// Defined in BasicBlockUtils.h
2829
struct JointPostDominanceSetComputer;
2930

3031
struct OwnershipFixupContext {
31-
std::function<void(SILInstruction *)> eraseNotify;
32-
std::function<void(SILInstruction *)> newInstNotify;
32+
InstModCallbacks callbacks;
3333
DeadEndBlocks &deBlocks;
3434
JointPostDominanceSetComputer &jointPostDomSetComputer;
3535

‎lib/SILOptimizer/Analysis/SimplifyInstruction.cpp

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,9 @@ case BuiltinValueKind::id:
735735
// Top Level Entrypoints
736736
//===----------------------------------------------------------------------===//
737737

738-
static SILBasicBlock::iterator
739-
replaceAllUsesAndEraseInner(SingleValueInstruction *svi, SILValue newValue,
740-
std::function<void(SILInstruction *)> eraseNotify) {
738+
SILBasicBlock::iterator
739+
swift::replaceAllUsesAndErase(SingleValueInstruction *svi, SILValue newValue,
740+
InstModCallbacks &callbacks) {
741741
assert(svi != newValue && "Cannot RAUW a value with itself");
742742
SILBasicBlock::iterator nextii = std::next(svi->getIterator());
743743

@@ -749,18 +749,13 @@ replaceAllUsesAndEraseInner(SingleValueInstruction *svi, SILValue newValue,
749749
if (isEndOfScopeMarker(user)) {
750750
if (&*nextii == user)
751751
++nextii;
752-
if (eraseNotify)
753-
eraseNotify(user);
754-
else
755-
user->eraseFromParent();
752+
callbacks.deleteInst(user);
756753
continue;
757754
}
758755
use->set(newValue);
759756
}
760-
if (eraseNotify)
761-
eraseNotify(svi);
762-
else
763-
svi->eraseFromParent();
757+
758+
callbacks.deleteInst(svi);
764759

765760
return nextii;
766761
}
@@ -775,21 +770,19 @@ replaceAllUsesAndEraseInner(SingleValueInstruction *svi, SILValue newValue,
775770
/// before this is called. It will perform fixups as necessary to preserve OSSA.
776771
///
777772
/// Return an iterator to the next (nondeleted) instruction.
778-
SILBasicBlock::iterator swift::replaceAllSimplifiedUsesAndErase(
779-
SILInstruction *i, SILValue result,
780-
std::function<void(SILInstruction *)> eraseNotify,
781-
std::function<void(SILInstruction *)> newInstNotify,
782-
DeadEndBlocks *deadEndBlocks) {
773+
SILBasicBlock::iterator
774+
swift::replaceAllSimplifiedUsesAndErase(SILInstruction *i, SILValue result,
775+
InstModCallbacks &callbacks,
776+
DeadEndBlocks *deadEndBlocks) {
783777
auto *svi = cast<SingleValueInstruction>(i);
784778
assert(svi != result && "Cannot RAUW a value with itself");
785779

786780
if (svi->getFunction()->hasOwnership()) {
787781
JointPostDominanceSetComputer computer(*deadEndBlocks);
788-
OwnershipFixupContext ctx{eraseNotify, newInstNotify, *deadEndBlocks,
789-
computer};
782+
OwnershipFixupContext ctx{callbacks, *deadEndBlocks, computer};
790783
return ctx.replaceAllUsesAndEraseFixingOwnership(svi, result);
791784
}
792-
return replaceAllUsesAndEraseInner(svi, result, eraseNotify);
785+
return replaceAllUsesAndErase(svi, result, callbacks);
793786
}
794787

795788
/// Simplify invocations of builtin operations that may overflow.

‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,8 @@ static void replaceValueMetatypeInstWithMetatypeArgument(
866866
valueMetatype->getLoc(), metatypeArgument,
867867
valueMetatype->getType());
868868
}
869-
replaceAllSimplifiedUsesAndErase(valueMetatype, metatypeArgument);
869+
InstModCallbacks callbacks;
870+
replaceAllSimplifiedUsesAndErase(valueMetatype, metatypeArgument, callbacks);
870871
}
871872

872873
void LifetimeChecker::handleLoadForTypeOfSelfUse(DIMemoryUse &Use) {

‎lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,10 +435,10 @@ static bool stripOwnership(SILFunction &func) {
435435
if (!value.hasValue())
436436
continue;
437437
if (SILValue newValue = simplifyInstruction(*value)) {
438-
replaceAllSimplifiedUsesAndErase(*value, newValue,
439-
[&](SILInstruction *instToErase) {
440-
visitor.eraseInstruction(instToErase);
441-
});
438+
InstModCallbacks callbacks([&](SILInstruction *instToErase) {
439+
visitor.eraseInstruction(instToErase);
440+
});
441+
replaceAllSimplifiedUsesAndErase(*value, newValue, callbacks);
442442
madeChange = true;
443443
}
444444
}

‎lib/SILOptimizer/Transforms/CSE.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,7 @@ static bool isLazyPropertyGetter(ApplyInst *ai) {
955955

956956
bool CSE::processNode(DominanceInfoNode *Node) {
957957
SILBasicBlock *BB = Node->getBlock();
958+
InstModCallbacks callbacks;
958959
bool Changed = false;
959960

960961
// See if any instructions in the block can be eliminated. If so, do it. If
@@ -981,8 +982,7 @@ bool CSE::processNode(DominanceInfoNode *Node) {
981982
if (SILValue V = simplifyInstruction(Inst)) {
982983
LLVM_DEBUG(llvm::dbgs()
983984
<< "SILCSE SIMPLIFY: " << *Inst << " to: " << *V << '\n');
984-
nextI = replaceAllSimplifiedUsesAndErase(Inst, V, nullptr, nullptr,
985-
&DeadEndBBs);
985+
nextI = replaceAllSimplifiedUsesAndErase(Inst, V, callbacks, &DeadEndBBs);
986986
Changed = true;
987987
++NumSimplify;
988988
continue;
@@ -1412,9 +1412,8 @@ class SILCSE : public SILFunctionTransform {
14121412
InstructionCloner Cloner(Fn);
14131413
DeadEndBlocks DeadEndBBs(Fn);
14141414
JointPostDominanceSetComputer Computer(DeadEndBBs);
1415-
OwnershipFixupContext FixupCtx{/* eraseNotify */ nullptr,
1416-
/* newInstNotify */ nullptr, DeadEndBBs,
1417-
Computer};
1415+
InstModCallbacks callbacks;
1416+
OwnershipFixupContext FixupCtx{callbacks, DeadEndBBs, Computer};
14181417
CSE C(RunsOnHighLevelSil, SEA, FuncBuilder, OpenedArchetypesTracker, Cloner,
14191418
DeadEndBBs, FixupCtx);
14201419
bool Changed = false;

‎lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,7 @@ bool SimplifyCFG::tryJumpThreading(BranchInst *BI) {
11391139
/// result in exposing opportunities for CFG simplification.
11401140
bool SimplifyCFG::simplifyBranchOperands(OperandValueArrayRef Operands) {
11411141
bool Simplified = false;
1142+
InstModCallbacks callbacks;
11421143
for (auto O = Operands.begin(), E = Operands.end(); O != E; ++O) {
11431144
// All of our interesting simplifications are on single-value instructions
11441145
// for now.
@@ -1149,7 +1150,7 @@ bool SimplifyCFG::simplifyBranchOperands(OperandValueArrayRef Operands) {
11491150
// unreachable block. In this case it can reference itself as operand.
11501151
if (Result && Result != I) {
11511152
LLVM_DEBUG(llvm::dbgs() << "simplify branch operand " << *I);
1152-
replaceAllSimplifiedUsesAndErase(I, Result);
1153+
replaceAllSimplifiedUsesAndErase(I, Result, callbacks);
11531154
Simplified = true;
11541155
}
11551156
}

0 commit comments

Comments
 (0)