Skip to content

Commit 82c380b

Browse files
Merge pull request #77143 from nate-chandler/bug/20241021/1
[MoveAddrChecker] Look through mark_dependence to determine whether an address is initialized to begin with.
2 parents 159a78a + 9c887a1 commit 82c380b

File tree

2 files changed

+81
-56
lines changed

2 files changed

+81
-56
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 64 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -987,36 +987,21 @@ static bool findNonEscapingPartialApplyUses(PartialApplyInst *pai,
987987
return true;
988988
}
989989

990-
void UseState::initializeLiveness(
991-
FieldSensitiveMultiDefPrunedLiveRange &liveness) {
992-
assert(liveness.getNumSubElements() == getNumSubelements());
993-
// We begin by initializing all of our init uses.
994-
for (auto initInstAndValue : initInsts) {
995-
LLVM_DEBUG(llvm::dbgs() << "Found def: " << *initInstAndValue.first);
990+
static bool
991+
addressBeginsInitialized(MarkUnresolvedNonCopyableValueInst *address) {
992+
// FIXME: Whether the initial use is an initialization ought to be entirely
993+
// derivable from the CheckKind of the mark instruction.
996994

997-
liveness.initializeDef(initInstAndValue.first, initInstAndValue.second);
998-
}
995+
SILValue operand = address->getOperand();
999996

1000-
// If we have a reinitInstAndValue that we are going to be able to convert
1001-
// into a simple init, add it as an init. We are going to consider the rest of
1002-
// our reinit uses to be liveness uses.
1003-
for (auto reinitInstAndValue : reinitInsts) {
1004-
if (isReinitToInitConvertibleInst(reinitInstAndValue.first)) {
1005-
LLVM_DEBUG(llvm::dbgs() << "Found def: " << *reinitInstAndValue.first);
1006-
liveness.initializeDef(reinitInstAndValue.first,
1007-
reinitInstAndValue.second);
1008-
}
997+
if (auto *mdi = dyn_cast<MarkDependenceInst>(operand)) {
998+
operand = mdi->getValue();
1009999
}
1010-
1011-
// FIXME: Whether the initial use is an initialization ought to be entirely
1012-
// derivable from the CheckKind of the mark instruction.
10131000

1014-
// Then check if our markedValue is from an argument that is in,
1015-
// in_guaranteed, inout, or inout_aliasable, consider the marked address to be
1016-
// the initialization point.
1017-
bool beginsInitialized = false;
10181001
{
1019-
SILValue operand = address->getOperand();
1002+
// Then check if our markedValue is from an argument that is in,
1003+
// in_guaranteed, inout, or inout_aliasable, consider the marked address to
1004+
// be the initialization point.
10201005
if (auto *c = dyn_cast<CopyableToMoveOnlyWrapperAddrInst>(operand))
10211006
operand = c->getOperand();
10221007
if (auto *fArg = dyn_cast<SILFunctionArgument>(operand)) {
@@ -1034,7 +1019,7 @@ void UseState::initializeLiveness(
10341019
"an init... adding mark_unresolved_non_copyable_value as "
10351020
"init!\n");
10361021
// We cheat here slightly and use our address's operand.
1037-
beginsInitialized = true;
1022+
return true;
10381023
break;
10391024
case swift::SILArgumentConvention::Indirect_Out:
10401025
llvm_unreachable("Should never have out addresses here");
@@ -1051,15 +1036,15 @@ void UseState::initializeLiveness(
10511036
}
10521037

10531038
// A read or write access always begins on an initialized value.
1054-
if (auto access = dyn_cast<BeginAccessInst>(address->getOperand())) {
1039+
if (auto access = dyn_cast<BeginAccessInst>(operand)) {
10551040
switch (access->getAccessKind()) {
10561041
case SILAccessKind::Deinit:
10571042
case SILAccessKind::Read:
10581043
case SILAccessKind::Modify:
10591044
LLVM_DEBUG(llvm::dbgs()
10601045
<< "Found move only arg closure box use... "
10611046
"adding mark_unresolved_non_copyable_value as init!\n");
1062-
beginsInitialized = true;
1047+
return true;
10631048
break;
10641049
case SILAccessKind::Init:
10651050
break;
@@ -1069,7 +1054,8 @@ void UseState::initializeLiveness(
10691054
// See if our address is from a closure guaranteed box that we did not promote
10701055
// to an address. In such a case, just treat our
10711056
// mark_unresolved_non_copyable_value as the init of our value.
1072-
if (auto *projectBox = dyn_cast<ProjectBoxInst>(stripAccessMarkers(address->getOperand()))) {
1057+
if (auto *projectBox =
1058+
dyn_cast<ProjectBoxInst>(stripAccessMarkers(operand))) {
10731059
if (auto *fArg = dyn_cast<SILFunctionArgument>(projectBox->getOperand())) {
10741060
if (fArg->isClosureCapture()) {
10751061
assert(fArg->getArgumentConvention() ==
@@ -1081,91 +1067,115 @@ void UseState::initializeLiveness(
10811067
LLVM_DEBUG(llvm::dbgs()
10821068
<< "Found move only arg closure box use... "
10831069
"adding mark_unresolved_non_copyable_value as init!\n");
1084-
beginsInitialized = true;
1070+
return true;
10851071
}
10861072
} else if (auto *box = dyn_cast<AllocBoxInst>(
10871073
lookThroughOwnershipInsts(projectBox->getOperand()))) {
10881074
LLVM_DEBUG(llvm::dbgs()
10891075
<< "Found move only var allocbox use... "
10901076
"adding mark_unresolved_non_copyable_value as init!\n");
1091-
beginsInitialized = true;
1077+
return true;
10921078
}
10931079
}
10941080

10951081
// Check if our address is from a ref_element_addr. In such a case, we treat
10961082
// the mark_unresolved_non_copyable_value as the initialization.
1097-
if (auto *refEltAddr = dyn_cast<RefElementAddrInst>(
1098-
stripAccessMarkers(address->getOperand()))) {
1083+
if (auto *refEltAddr =
1084+
dyn_cast<RefElementAddrInst>(stripAccessMarkers(operand))) {
10991085
LLVM_DEBUG(llvm::dbgs()
11001086
<< "Found ref_element_addr use... "
11011087
"adding mark_unresolved_non_copyable_value as init!\n");
1102-
beginsInitialized = true;
1088+
return true;
11031089
}
11041090

11051091
// Check if our address is from a global_addr. In such a case, we treat the
11061092
// mark_unresolved_non_copyable_value as the initialization.
11071093
if (auto *globalAddr =
1108-
dyn_cast<GlobalAddrInst>(stripAccessMarkers(address->getOperand()))) {
1094+
dyn_cast<GlobalAddrInst>(stripAccessMarkers(operand))) {
11091095
LLVM_DEBUG(llvm::dbgs()
11101096
<< "Found global_addr use... "
11111097
"adding mark_unresolved_non_copyable_value as init!\n");
1112-
beginsInitialized = true;
1098+
return true;
11131099
}
11141100

1115-
if (auto *ptai = dyn_cast<PointerToAddressInst>(
1116-
stripAccessMarkers(address->getOperand()))) {
1101+
if (auto *ptai =
1102+
dyn_cast<PointerToAddressInst>(stripAccessMarkers(operand))) {
11171103
assert(ptai->isStrict());
11181104
LLVM_DEBUG(llvm::dbgs()
11191105
<< "Found pointer to address use... "
11201106
"adding mark_unresolved_non_copyable_value as init!\n");
1121-
beginsInitialized = true;
1107+
return true;
11221108
}
1123-
1109+
11241110
if (auto *bai = dyn_cast_or_null<BeginApplyInst>(
1125-
stripAccessMarkers(address->getOperand())->getDefiningInstruction())) {
1111+
stripAccessMarkers(operand)->getDefiningInstruction())) {
11261112
LLVM_DEBUG(llvm::dbgs()
11271113
<< "Adding accessor coroutine begin_apply as init!\n");
1128-
beginsInitialized = true;
1114+
return true;
11291115
}
1130-
1116+
11311117
if (auto *eai = dyn_cast<UncheckedTakeEnumDataAddrInst>(
1132-
stripAccessMarkers(address->getOperand()))) {
1118+
stripAccessMarkers(operand))) {
11331119
LLVM_DEBUG(llvm::dbgs()
11341120
<< "Adding enum projection as init!\n");
1135-
beginsInitialized = true;
1121+
return true;
11361122
}
11371123

11381124
// Assume a strict check of a temporary or formal access is initialized
11391125
// before the check.
1140-
if (auto *asi = dyn_cast<AllocStackInst>(
1141-
stripAccessMarkers(address->getOperand()));
1126+
if (auto *asi = dyn_cast<AllocStackInst>(stripAccessMarkers(operand));
11421127
asi && address->isStrict()) {
11431128
LLVM_DEBUG(llvm::dbgs()
11441129
<< "Adding strict-marked alloc_stack as init!\n");
1145-
beginsInitialized = true;
1130+
return true;
11461131
}
11471132

11481133
// Assume a strict-checked value initialized before the check.
11491134
if (address->isStrict()) {
11501135
LLVM_DEBUG(llvm::dbgs()
11511136
<< "Adding strict marker as init!\n");
1152-
beginsInitialized = true;
1137+
return true;
11531138
}
11541139

11551140
// Assume a value whose deinit has been dropped has been initialized.
1156-
if (auto *ddi = dyn_cast<DropDeinitInst>(address->getOperand())) {
1141+
if (auto *ddi = dyn_cast<DropDeinitInst>(operand)) {
11571142
LLVM_DEBUG(llvm::dbgs()
11581143
<< "Adding copyable_to_move_only_wrapper as init!\n");
1159-
beginsInitialized = true;
1144+
return true;
11601145
}
11611146

11621147
// Assume a value wrapped in a MoveOnlyWrapper is initialized.
1163-
if (auto *m2c = dyn_cast<CopyableToMoveOnlyWrapperAddrInst>(address->getOperand())) {
1148+
if (auto *m2c = dyn_cast<CopyableToMoveOnlyWrapperAddrInst>(operand)) {
11641149
LLVM_DEBUG(llvm::dbgs()
11651150
<< "Adding copyable_to_move_only_wrapper as init!\n");
1166-
beginsInitialized = true;
1151+
return true;
11671152
}
1168-
1153+
return false;
1154+
}
1155+
1156+
void UseState::initializeLiveness(
1157+
FieldSensitiveMultiDefPrunedLiveRange &liveness) {
1158+
assert(liveness.getNumSubElements() == getNumSubelements());
1159+
// We begin by initializing all of our init uses.
1160+
for (auto initInstAndValue : initInsts) {
1161+
LLVM_DEBUG(llvm::dbgs() << "Found def: " << *initInstAndValue.first);
1162+
1163+
liveness.initializeDef(initInstAndValue.first, initInstAndValue.second);
1164+
}
1165+
1166+
// If we have a reinitInstAndValue that we are going to be able to convert
1167+
// into a simple init, add it as an init. We are going to consider the rest of
1168+
// our reinit uses to be liveness uses.
1169+
for (auto reinitInstAndValue : reinitInsts) {
1170+
if (isReinitToInitConvertibleInst(reinitInstAndValue.first)) {
1171+
LLVM_DEBUG(llvm::dbgs() << "Found def: " << *reinitInstAndValue.first);
1172+
liveness.initializeDef(reinitInstAndValue.first,
1173+
reinitInstAndValue.second);
1174+
}
1175+
}
1176+
1177+
bool beginsInitialized = addressBeginsInitialized(address);
1178+
11691179
if (beginsInitialized) {
11701180
recordInitUse(address, address, liveness.getTopLevelSpan());
11711181
liveness.initializeDef(SILValue(address), liveness.getTopLevelSpan());

test/SILOptimizer/moveonly_addresschecker.swift

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s -Xllvm -sil-print-final-ossa-module | %FileCheck %s
2-
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature NonescapableTypes %s -Xllvm -sil-print-final-ossa-module | %FileCheck %s
2+
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature NonescapableTypes %s
33

44
// This file contains tests that used to crash due to verifier errors. It must
55
// be separate from moveonly_addresschecker_diagnostics since when we fail on
@@ -46,3 +46,18 @@ struct S
4646
fatalError()
4747
}
4848
}
49+
50+
struct TestCoroAccessorOfCoroAccessor<T : ~Escapable> : ~Copyable & ~Escapable {
51+
var t: T
52+
53+
var inner: TestCoroAccessorOfCoroAccessor<T> {
54+
_read {
55+
fatalError()
56+
}
57+
}
58+
var outer: TestCoroAccessorOfCoroAccessor<T> {
59+
_read {
60+
yield inner
61+
}
62+
}
63+
}

0 commit comments

Comments
 (0)