Skip to content

Commit dd5efc7

Browse files
committed
[NFC] DestroyAddrHoisting: Extract transformation.
Separate the hoisting of destroys from the setup and cleanup. Facilitates bailing after completing N subpasses.
1 parent 6e2f09f commit dd5efc7

File tree

1 file changed

+62
-49
lines changed

1 file changed

+62
-49
lines changed

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

Lines changed: 62 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -927,9 +927,70 @@ bool hoistDestroys(SILValue root, bool ignoreDeinitBarriers,
927927
namespace {
928928
class DestroyAddrHoisting : public swift::SILFunctionTransform {
929929
void run() override;
930+
void hoistDestroys(bool &changed, ArrayRef<BeginAccessInst *> bais,
931+
ArrayRef<AllocStackInst *> asis,
932+
SmallPtrSetImpl<SILInstruction *> &remainingDestroyAddrs,
933+
InstructionDeleter &deleter);
930934
};
931935
} // end anonymous namespace
932936

937+
void DestroyAddrHoisting::hoistDestroys(
938+
bool &changed, ArrayRef<BeginAccessInst *> bais,
939+
ArrayRef<AllocStackInst *> asis,
940+
SmallPtrSetImpl<SILInstruction *> &remainingDestroyAddrs,
941+
InstructionDeleter &deleter) {
942+
auto *calleeAnalysis = getAnalysis<BasicCalleeAnalysis>();
943+
944+
// We assume that the function is in reverse post order so visiting the
945+
// blocks and pushing begin_access as we see them and then popping them off
946+
// the end will result in hoisting inner begin_access' destroy_addrs first.
947+
for (auto *bai : llvm::reverse(bais)) {
948+
// [exclusive_modify_scope_hoisting] Hoisting within modify access scopes
949+
// doesn't respect deinit barriers because
950+
//
951+
// Mutable variable lifetimes that are formally modified in the middle of
952+
// a lexical scope are anchored to the beginning of the lexical scope
953+
// rather than to the end.
954+
//
955+
// TODO: If the performance issues associated with failing to hoist
956+
// destroys within an exclusive modify scope are otherwise addressed,
957+
// it may be less confusing not to make use of the above rule and
958+
// respect deinit barriers here also ( rdar://116335154 ).
959+
changed |= ::hoistDestroys(bai, /*ignoreDeinitBarriers=*/true,
960+
remainingDestroyAddrs, deleter, calleeAnalysis);
961+
}
962+
// Alloc stacks always enclose their accesses.
963+
for (auto *asi : asis) {
964+
changed |= ::hoistDestroys(asi,
965+
/*ignoreDeinitBarriers=*/!asi->isLexical(),
966+
remainingDestroyAddrs, deleter, calleeAnalysis);
967+
}
968+
// Arguments enclose everything.
969+
for (auto *uncastArg : getFunction()->getArguments()) {
970+
auto *arg = cast<SILFunctionArgument>(uncastArg);
971+
if (arg->getType().isAddress()) {
972+
auto convention = arg->getArgumentConvention();
973+
// This is equivalent to writing
974+
//
975+
// convention == SILArgumentConvention::Indirect_Inout
976+
//
977+
// but communicates the rationale: in order to ignore deinit barriers, the
978+
// address must be exclusively accessed and be a modification.
979+
//
980+
// The situation with inout parameters is analogous to that with
981+
// mutable exclusive access scopes [exclusive_modify_scope_hoisting], so
982+
// deinit barriers are not respected.
983+
bool ignoredByConvention = convention.isInoutConvention() &&
984+
convention.isExclusiveIndirectParameter();
985+
auto lifetime = arg->getLifetime();
986+
bool ignoreDeinitBarriers = ignoredByConvention || lifetime.isEagerMove();
987+
changed |=
988+
::hoistDestroys(arg, ignoreDeinitBarriers, remainingDestroyAddrs,
989+
deleter, calleeAnalysis);
990+
}
991+
}
992+
}
993+
933994
// TODO: Handle alloc_box the same way, as long as the box doesn't escape.
934995
//
935996
// TODO: Handle address and boxes that are captured in no-escape closures.
@@ -1025,55 +1086,7 @@ void DestroyAddrHoisting::run() {
10251086
++splitDestroys;
10261087
}
10271088

1028-
auto *calleeAnalysis = getAnalysis<BasicCalleeAnalysis>();
1029-
1030-
// We assume that the function is in reverse post order so visiting the
1031-
// blocks and pushing begin_access as we see them and then popping them off
1032-
// the end will result in hoisting inner begin_access' destroy_addrs first.
1033-
for (auto *bai : llvm::reverse(bais)) {
1034-
// [exclusive_modify_scope_hoisting] Hoisting within modify access scopes
1035-
// doesn't respect deinit barriers because
1036-
//
1037-
// Mutable variable lifetimes that are formally modified in the middle of
1038-
// a lexical scope are anchored to the beginning of the lexical scope
1039-
// rather than to the end.
1040-
//
1041-
// TODO: If the performance issues associated with failing to hoist
1042-
// destroys within an exclusive modify scope are otherwise addressed,
1043-
// it may be less confusing not to make use of the above rule and
1044-
// respect deinit barriers here also ( rdar://116335154 ).
1045-
changed |= hoistDestroys(bai, /*ignoreDeinitBarriers=*/true,
1046-
remainingDestroyAddrs, deleter, calleeAnalysis);
1047-
}
1048-
// Alloc stacks always enclose their accesses.
1049-
for (auto *asi : asis) {
1050-
changed |= hoistDestroys(asi,
1051-
/*ignoreDeinitBarriers=*/!asi->isLexical(),
1052-
remainingDestroyAddrs, deleter, calleeAnalysis);
1053-
}
1054-
// Arguments enclose everything.
1055-
for (auto *uncastArg : getFunction()->getArguments()) {
1056-
auto *arg = cast<SILFunctionArgument>(uncastArg);
1057-
if (arg->getType().isAddress()) {
1058-
auto convention = arg->getArgumentConvention();
1059-
// This is equivalent to writing
1060-
//
1061-
// convention == SILArgumentConvention::Indirect_Inout
1062-
//
1063-
// but communicates the rationale: in order to ignore deinit barriers, the
1064-
// address must be exclusively accessed and be a modification.
1065-
//
1066-
// The situation with inout parameters is analogous to that with
1067-
// mutable exclusive access scopes [exclusive_modify_scope_hoisting], so
1068-
// deinit barriers are not respected.
1069-
bool ignoredByConvention = convention.isInoutConvention() &&
1070-
convention.isExclusiveIndirectParameter();
1071-
auto lifetime = arg->getLifetime();
1072-
bool ignoreDeinitBarriers = ignoredByConvention || lifetime.isEagerMove();
1073-
changed |= hoistDestroys(arg, ignoreDeinitBarriers, remainingDestroyAddrs,
1074-
deleter, calleeAnalysis);
1075-
}
1076-
}
1089+
hoistDestroys(changed, bais, asis, remainingDestroyAddrs, deleter);
10771090

10781091
for (auto pair : splitDestroysAndStores) {
10791092
auto *dai = pair.first;

0 commit comments

Comments
 (0)