Skip to content

Commit a4737d8

Browse files
committed
[move-function] Move addressesToCheck out of MoveKillsCopyableAddressesObjectChecker and into the users of said checker.
This ensures that MoveKillsCopyableAddressesObjectChecker is only ever processing a single address rather than maintaining this worklist. This will enable me to reuse parts of it in a simpler way for defer checking. (cherry picked from commit 5b1ec61)
1 parent 1a57035 commit a4737d8

File tree

1 file changed

+101
-110
lines changed

1 file changed

+101
-110
lines changed

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

Lines changed: 101 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,6 @@ static bool upwardScanForInit(SILInstruction *inst, UseState &useState) {
482482
namespace {
483483

484484
struct MoveKillsCopyableAddressesObjectChecker {
485-
SmallSetVector<SILValue, 32> addressesToCheck;
486485
SILFunction *fn;
487486
UseState useState;
488487
llvm::DenseMap<SILBasicBlock *, SILInstruction *> useBlocks;
@@ -496,7 +495,7 @@ struct MoveKillsCopyableAddressesObjectChecker {
496495
bool performSingleBasicBlockAnalysisForAllMarkMoves(SILValue address);
497496
bool performGlobalDataflow(SILValue address);
498497

499-
bool check();
498+
bool check(SILValue address);
500499

501500
void emitDiagnosticForMove(SILValue borrowedValue,
502501
StringRef borrowedValueName, MoveValueInst *mvi);
@@ -850,130 +849,112 @@ bool MoveKillsCopyableAddressesObjectChecker::performGlobalDataflow(
850849
return madeChange;
851850
}
852851

853-
bool MoveKillsCopyableAddressesObjectChecker::check() {
854-
if (addressesToCheck.empty())
852+
bool MoveKillsCopyableAddressesObjectChecker::check(SILValue address) {
853+
auto accessPathWithBase = AccessPathWithBase::compute(address);
854+
auto accessPath = accessPathWithBase.accessPath;
855+
856+
// Bail on an invalid AccessPath.
857+
//
858+
// AccessPath completeness is verified independently--it may be invalid in
859+
// extraordinary situations. When AccessPath is valid, we know all its uses
860+
// are recognizable.
861+
//
862+
// NOTE: If due to an invalid access path we fail here, we will just error
863+
// on the _move since the _move would not have been handled.
864+
if (!accessPath.isValid())
855865
return false;
856866

857-
LLVM_DEBUG(llvm::dbgs() << "Visiting Function: " << fn->getName() << "\n");
858-
auto addressToProcess =
859-
llvm::makeArrayRef(addressesToCheck.begin(), addressesToCheck.end());
860-
861-
bool madeChange = false;
862-
863-
while (!addressToProcess.empty()) {
864-
auto address = addressToProcess.front();
865-
addressToProcess = addressToProcess.drop_front(1);
866-
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *address);
867-
868-
auto accessPathWithBase = AccessPathWithBase::compute(address);
869-
auto accessPath = accessPathWithBase.accessPath;
870-
871-
// Bail on an invalid AccessPath.
872-
//
873-
// AccessPath completeness is verified independently--it may be invalid in
874-
// extraordinary situations. When AccessPath is valid, we know all its uses
875-
// are recognizable.
876-
//
877-
// NOTE: If due to an invalid access path we fail here, we will just error
878-
// on the _move since the _move would not have been handled.
879-
if (!accessPath.isValid())
880-
continue;
881-
882-
GatherLexicalLifetimeUseVisitor visitor(useState);
883-
SWIFT_DEFER { visitor.clear(); };
884-
visitor.reset(address);
885-
if (!visitAccessPathUses(visitor, accessPath, fn))
886-
continue;
867+
GatherLexicalLifetimeUseVisitor visitor(useState);
868+
SWIFT_DEFER { visitor.clear(); };
869+
visitor.reset(address);
870+
if (!visitAccessPathUses(visitor, accessPath, fn))
871+
return false;
887872

888-
// See if our base address is an inout. If we found any moves, add as a
889-
// liveness use all function terminators.
890-
if (auto *fArg = dyn_cast<SILFunctionArgument>(address)) {
891-
if (fArg->hasConvention(SILArgumentConvention::Indirect_Inout)) {
892-
if (visitor.useState.markMoves.size()) {
893-
SmallVector<SILBasicBlock *, 2> exitingBlocks;
894-
fn->findExitingBlocks(exitingBlocks);
895-
for (auto *block : exitingBlocks) {
896-
visitor.useState.livenessUses.insert(block->getTerminator());
897-
}
873+
// See if our base address is an inout. If we found any moves, add as a
874+
// liveness use all function terminators.
875+
if (auto *fArg = dyn_cast<SILFunctionArgument>(address)) {
876+
if (fArg->hasConvention(SILArgumentConvention::Indirect_Inout)) {
877+
if (visitor.useState.markMoves.size()) {
878+
SmallVector<SILBasicBlock *, 2> exitingBlocks;
879+
fn->findExitingBlocks(exitingBlocks);
880+
for (auto *block : exitingBlocks) {
881+
visitor.useState.livenessUses.insert(block->getTerminator());
898882
}
899883
}
900884
}
885+
}
901886

902-
// Now initialize our data structures.
903-
SWIFT_DEFER {
904-
useBlocks.clear();
905-
initBlocks.clear();
906-
destroyBlocks.clear();
907-
reinitBlocks.clear();
908-
markMovesToDataflow.clear();
909-
};
910-
911-
// Perform the single basic block analysis emitting a diagnostic/pairing
912-
// mark_unresolved_move_addr and destroys if needed. If we discover a
913-
// mark_move that propagates its state out of the current block, this
914-
// routine also prepares the pass for running the multi-basic block
915-
// diagnostic.
916-
if (performSingleBasicBlockAnalysisForAllMarkMoves(address)) {
917-
LLVM_DEBUG(llvm::dbgs() << "Performed single block analysis!\n");
918-
madeChange = true;
919-
continue;
920-
}
887+
// Now initialize our data structures.
888+
SWIFT_DEFER {
889+
useBlocks.clear();
890+
initBlocks.clear();
891+
destroyBlocks.clear();
892+
reinitBlocks.clear();
893+
markMovesToDataflow.clear();
894+
};
921895

922-
// Go through all init uses and if we don't see any other of our uses, then
923-
// mark this as an "init block".
924-
for (auto *init : useState.inits) {
925-
if (upwardScanForInit(init, useState)) {
926-
LLVM_DEBUG(llvm::dbgs() << " Found use block at: " << *init);
927-
initBlocks.insert(init->getParent());
928-
}
896+
// Perform the single basic block analysis emitting a diagnostic/pairing
897+
// mark_unresolved_move_addr and destroys if needed. If we discover a
898+
// mark_move that propagates its state out of the current block, this
899+
// routine also prepares the pass for running the multi-basic block
900+
// diagnostic.
901+
if (performSingleBasicBlockAnalysisForAllMarkMoves(address)) {
902+
LLVM_DEBUG(llvm::dbgs() << "Performed single block analysis!\n");
903+
return true;
904+
}
905+
906+
// Go through all init uses and if we don't see any other of our uses, then
907+
// mark this as an "init block".
908+
for (auto *init : useState.inits) {
909+
if (upwardScanForInit(init, useState)) {
910+
LLVM_DEBUG(llvm::dbgs() << " Found use block at: " << *init);
911+
initBlocks.insert(init->getParent());
929912
}
913+
}
930914

931-
// Then go through all normal uses and do upwardScanForUseOut.
932-
for (auto *user : useState.livenessUses) {
933-
if (upwardScanForUseOut(user, useState)) {
934-
LLVM_DEBUG(llvm::dbgs() << " Found liveness block at: " << *user);
935-
useBlocks[user->getParent()] = user;
936-
}
915+
// Then go through all normal uses and do upwardScanForUseOut.
916+
for (auto *user : useState.livenessUses) {
917+
if (upwardScanForUseOut(user, useState)) {
918+
LLVM_DEBUG(llvm::dbgs() << " Found liveness block at: " << *user);
919+
useBlocks[user->getParent()] = user;
937920
}
921+
}
938922

939-
for (auto destroyOpt : useState.destroys) {
940-
// Any destroys we eliminated when processing single basic blocks will be
941-
// nullptr. Skip them!
942-
if (!destroyOpt)
943-
continue;
923+
for (auto destroyOpt : useState.destroys) {
924+
// Any destroys we eliminated when processing single basic blocks will be
925+
// nullptr. Skip them!
926+
if (!destroyOpt)
927+
continue;
944928

945-
auto *destroy = *destroyOpt;
929+
auto *destroy = *destroyOpt;
946930

947-
auto iter = useState.destroyToIndexMap.find(destroy);
948-
assert(iter != useState.destroyToIndexMap.end());
931+
auto iter = useState.destroyToIndexMap.find(destroy);
932+
assert(iter != useState.destroyToIndexMap.end());
949933

950-
if (upwardScanForDestroys(destroy, useState)) {
951-
LLVM_DEBUG(llvm::dbgs() << " Found destroy block at: " << *destroy);
952-
destroyBlocks[destroy->getParent()] = destroy;
953-
}
934+
if (upwardScanForDestroys(destroy, useState)) {
935+
LLVM_DEBUG(llvm::dbgs() << " Found destroy block at: " << *destroy);
936+
destroyBlocks[destroy->getParent()] = destroy;
954937
}
938+
}
955939

956-
for (auto reinitOpt : useState.reinits) {
957-
// Any destroys we eliminated when processing single basic blocks will be
958-
// nullptr. Skip them!
959-
if (!reinitOpt)
960-
continue;
940+
for (auto reinitOpt : useState.reinits) {
941+
// Any destroys we eliminated when processing single basic blocks will be
942+
// nullptr. Skip them!
943+
if (!reinitOpt)
944+
continue;
961945

962-
auto *reinit = *reinitOpt;
963-
auto iter = useState.reinitToIndexMap.find(reinit);
964-
assert(iter != useState.reinitToIndexMap.end());
946+
auto *reinit = *reinitOpt;
947+
auto iter = useState.reinitToIndexMap.find(reinit);
948+
assert(iter != useState.reinitToIndexMap.end());
965949

966-
if (upwardScanForDestroys(reinit, useState)) {
967-
LLVM_DEBUG(llvm::dbgs() << " Found reinit block at: " << *reinit);
968-
reinitBlocks[reinit->getParent()] = reinit;
969-
}
950+
if (upwardScanForDestroys(reinit, useState)) {
951+
LLVM_DEBUG(llvm::dbgs() << " Found reinit block at: " << *reinit);
952+
reinitBlocks[reinit->getParent()] = reinit;
970953
}
971-
972-
// Ok, we are setup. Perform the global dataflow!
973-
madeChange |= performGlobalDataflow(address);
974954
}
975955

976-
return madeChange;
956+
// Ok, we are setup. Perform the global dataflow!
957+
return performGlobalDataflow(address);
977958
}
978959

979960
//===----------------------------------------------------------------------===//
@@ -991,18 +972,16 @@ class MoveKillsCopyableAddressesCheckerPass : public SILFunctionTransform {
991972
if (getFunction()->wasDeserializedCanonical())
992973
return;
993974

994-
bool madeChange = false;
995-
996975
assert(fn->getModule().getStage() == SILStage::Raw &&
997976
"Should only run on Raw SIL");
998977

999-
MoveKillsCopyableAddressesObjectChecker checker(getFunction());
978+
SmallSetVector<SILValue, 32> addressesToCheck;
1000979

1001980
for (auto *arg : fn->front().getSILFunctionArguments()) {
1002981
if (arg->getType().isAddress() &&
1003982
(arg->hasConvention(SILArgumentConvention::Indirect_In) ||
1004983
arg->hasConvention(SILArgumentConvention::Indirect_Inout)))
1005-
checker.addressesToCheck.insert(arg);
984+
addressesToCheck.insert(arg);
1006985
}
1007986

1008987
for (auto &block : *fn) {
@@ -1014,14 +993,26 @@ class MoveKillsCopyableAddressesCheckerPass : public SILFunctionTransform {
1014993
// Only check lexical alloc_stack that were not emitted as vars.
1015994
if (asi->isLexical()) {
1016995
LLVM_DEBUG(llvm::dbgs() << "Found lexical alloc_stack: " << *asi);
1017-
checker.addressesToCheck.insert(asi);
996+
addressesToCheck.insert(asi);
1018997
continue;
1019998
}
1020999
}
10211000
}
10221001
}
10231002

1024-
madeChange |= checker.check();
1003+
LLVM_DEBUG(llvm::dbgs() << "Visiting Function: " << fn->getName() << "\n");
1004+
auto addressToProcess =
1005+
llvm::makeArrayRef(addressesToCheck.begin(), addressesToCheck.end());
1006+
1007+
MoveKillsCopyableAddressesObjectChecker checker(getFunction());
1008+
bool madeChange = false;
1009+
1010+
while (!addressToProcess.empty()) {
1011+
auto address = addressToProcess.front();
1012+
addressToProcess = addressToProcess.drop_front(1);
1013+
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *address);
1014+
madeChange |= checker.check(address);
1015+
}
10251016

10261017
if (madeChange) {
10271018
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);

0 commit comments

Comments
 (0)