[DCE] Process instructions added during deletion.

In cea0f00598, `InstructionDeleter` began
deleting `load [take]` instructions.  Analogous to how it creates a
`destroy_value` when deleting an instruction which consumes a value, in
the case of deleting a `load [take]` the `InstructionDeleter` inserts a
compensating `destroy_addr`.

Previously, `DeadCodeElimination` did not observe the creation of any
instructions created by the `InstructionDeleter`.  In the case of the
newly created `destroy_addr`, DCE didn't mark that the `destroy_addr`
was live and so deleted it.  The result was a leak.

Here, this is fixed by passing an `InstModCallbacks`--with an
`onCreateNewInst` implementation--down into `erasePhiArgument` that
eventually invokes the `InstructionDeleter`.  When the
`InstructionDeleter` creates a new instruction, DCE marks it live.
This commit is contained in:
Nate Chandler
2023-11-15 14:28:42 -08:00
parent 7fd5cae61a
commit ae6868a296
4 changed files with 58 additions and 14 deletions

View File

@@ -25,6 +25,7 @@
#include "swift/SIL/SILBuilder.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SILOptimizer/Utils/InstModCallbacks.h"
#include "swift/SILOptimizer/Utils/InstructionDeleter.h"
namespace llvm {
@@ -36,7 +37,6 @@ namespace swift {
class DominanceInfo;
class SILLoop;
class SILLoopInfo;
struct InstModCallbacks;
/// Adds a new argument to an edge between a branch and a destination
/// block. Allows for user injected callbacks via \p callbacks.
@@ -65,14 +65,16 @@ TermInst *changeEdgeValue(TermInst *branch, SILBasicBlock *dest, size_t idx,
/// specified index. Asserts internally that the argument along the edge does
/// not have uses.
TermInst *deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
size_t argIndex, bool cleanupDeadPhiOp = true);
size_t argIndex, bool cleanupDeadPhiOp = true,
InstModCallbacks callbacks = InstModCallbacks());
/// Erase the \p argIndex phi argument from \p block. Asserts that the argument
/// is a /real/ phi argument. Removes all incoming values for the argument from
/// predecessor terminators. Asserts internally that it only ever is given
/// "true" phi argument.
void erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
bool cleanupDeadPhiOp = true);
bool cleanupDeadPhiOp = true,
InstModCallbacks callbacks = InstModCallbacks());
/// Check if the edge from the terminator is critical.
bool isCriticalEdge(TermInst *t, unsigned edgeIdx);

View File

@@ -658,7 +658,9 @@ bool DCE::removeDead() {
endLifetimeOfLiveValue(phiArg->getIncomingPhiValue(pred), insertPt);
}
erasePhiArgument(&BB, i);
erasePhiArgument(&BB, i, /*cleanupDeadPhiOps=*/true,
InstModCallbacks().onCreateNewInst(
[&](auto *inst) { markInstructionLive(inst); }));
Changed = true;
BranchesChanged = true;
}

View File

@@ -74,21 +74,22 @@ TermInst *swift::addNewEdgeValueToBranch(TermInst *branch, SILBasicBlock *dest,
return newBr;
}
static void
deleteTriviallyDeadOperandsOfDeadArgument(MutableArrayRef<Operand> termOperands,
unsigned deadArgIndex) {
static void deleteTriviallyDeadOperandsOfDeadArgument(
MutableArrayRef<Operand> termOperands, unsigned deadArgIndex,
InstModCallbacks callbacks = InstModCallbacks()) {
Operand &op = termOperands[deadArgIndex];
auto *i = op.get()->getDefiningInstruction();
if (!i)
return;
op.set(SILUndef::get(op.get()->getType(), *i->getFunction()));
eliminateDeadInstruction(i);
eliminateDeadInstruction(i, callbacks);
}
// Our implementation assumes that our caller is attempting to remove a dead
// SILPhiArgument from a SILBasicBlock and has already RAUWed the argument.
TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
size_t argIndex, bool cleanupDeadPhiOps) {
size_t argIndex, bool cleanupDeadPhiOps,
InstModCallbacks callbacks) {
if (auto *cbi = dyn_cast<CondBranchInst>(branch)) {
SmallVector<SILValue, 8> trueArgs;
SmallVector<SILValue, 8> falseArgs;
@@ -99,7 +100,7 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
if (destBlock == cbi->getTrueBB()) {
if (cleanupDeadPhiOps) {
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getTrueOperands(),
argIndex);
argIndex, callbacks);
}
trueArgs.erase(trueArgs.begin() + argIndex);
}
@@ -107,7 +108,7 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
if (destBlock == cbi->getFalseBB()) {
if (cleanupDeadPhiOps) {
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getFalseOperands(),
argIndex);
argIndex, callbacks);
}
falseArgs.erase(falseArgs.begin() + argIndex);
}
@@ -125,7 +126,8 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
SmallVector<SILValue, 8> args;
llvm::copy(bi->getArgs(), std::back_inserter(args));
if (cleanupDeadPhiOps) {
deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex);
deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex,
callbacks);
}
args.erase(args.begin() + argIndex);
auto *result = SILBuilderWithScope(bi).createBranch(bi->getLoc(),
@@ -138,7 +140,8 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
}
void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
bool cleanupDeadPhiOps) {
bool cleanupDeadPhiOps,
InstModCallbacks callbacks) {
assert(block->getArgument(argIndex)->isPhi()
&& "Only should be used on phi arguments");
block->eraseArgument(argIndex);
@@ -155,7 +158,8 @@ void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
predBlocks.insert(pred);
for (auto *pred : predBlocks)
deleteEdgeValue(pred->getTerminator(), block, argIndex, cleanupDeadPhiOps);
deleteEdgeValue(pred->getTerminator(), block, argIndex, cleanupDeadPhiOps,
callbacks);
}
/// Changes the edge value between a branch and destination basic block

View File

@@ -432,3 +432,39 @@ bb0(%0 : @owned $MO):
%63 = tuple ()
return %63 : $()
}
// The InstructionDeleter will delete the `load [take]` and insert a
// `destroy_addr`. Observe the creation of the new destroy_addr instruction
// that occurs when deleting the `load [take]` and mark it live. Prevents a
// leak.
// CHECK-LABEL: sil [ossa] @keep_new_destroy_addr : {{.*}} {
// CHECK: destroy_addr
// CHECK-LABEL: } // end sil function 'keep_new_destroy_addr'
sil [ossa] @keep_new_destroy_addr : $@convention(thin) () -> () {
bb0:
try_apply undef() : $@convention(thin) () -> @error any Error, normal bb1, error bb5
bb1(%4 : $()):
%err = alloc_stack $any Error
try_apply undef<any Error>(%err) : $@convention(method) <τ_1_1 where τ_1_1 : Error> () -> (@error_indirect τ_1_1), normal bb2, error bb6
bb2(%15 : $()):
dealloc_stack %err : $*any Error
br bb3
bb4(%24 : @owned $any Error):
destroy_value %24 : $any Error
br bb3
bb5(%30 : @owned $any Error):
br bb4(%30 : $any Error)
bb6:
%33 = load [take] %err : $*any Error
dealloc_stack %err : $*any Error
br bb4(%33 : $any Error)
bb3:
%22 = tuple ()
return %22 : $()
}