Skip to content

Commit 97c7376

Browse files
gottesmmgottesmm
authored
Merge pull request #35253 from gottesmm/pr-7d30d56fcab9d42f3788104b987f62df27540e00
[sil-inst-opt] Improve performance of InstModCallbacks by eliminating indirect call along default callback path.
2 parents 3a1c8fd + 0de00d1 commit 97c7376

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)