Skip to content

Commit 06f53fe

Browse files
Merge pull request swiftlang#66955 from nate-chandler/rdar111391893
[MoveOnlyAddressChecker] Fix representation for initialized fields.
2 parents 1259125 + d00da4a commit 06f53fe

File tree

2 files changed

+70
-26
lines changed

2 files changed

+70
-26
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,9 @@ namespace {
563563
struct UseState {
564564
MarkMustCheckInst *address;
565565

566+
using InstToBitMap =
567+
llvm::SmallMapVector<SILInstruction *, SmallBitVector, 4>;
568+
566569
llvm::Optional<unsigned> cachedNumSubelements;
567570

568571
/// The blocks that consume fields of the value.
@@ -576,7 +579,7 @@ struct UseState {
576579

577580
/// A map from a liveness requiring use to the part of the type that it
578581
/// requires liveness for.
579-
llvm::SmallMapVector<SILInstruction *, SmallBitVector, 4> livenessUses;
582+
InstToBitMap livenessUses;
580583

581584
/// A map from a load [copy] or load [take] that we determined must be
582585
/// converted to a load_borrow to the part of the type tree that it needs to
@@ -622,11 +625,11 @@ struct UseState {
622625

623626
/// A map from an instruction that initializes memory to the description of
624627
/// the part of the type tree that it initializes.
625-
llvm::SmallMapVector<SILInstruction *, TypeTreeLeafTypeRange, 4> initInsts;
628+
InstToBitMap initInsts;
626629

627630
/// memInstMustReinitialize insts. Contains both insts like copy_addr/store
628631
/// [assign] that are reinits that we will convert to inits and true reinits.
629-
llvm::SmallMapVector<SILInstruction *, SmallBitVector, 4> reinitInsts;
632+
InstToBitMap reinitInsts;
630633

631634
/// The set of drop_deinits of this mark_must_check
632635
SmallSetVector<SILInstruction *, 2> dropDeinitInsts;
@@ -653,32 +656,39 @@ struct UseState {
653656
return *cachedNumSubelements;
654657
}
655658

656-
SmallBitVector &getOrCreateLivenessUse(SILInstruction *inst) {
657-
auto iter = livenessUses.find(inst);
658-
if (iter == livenessUses.end()) {
659-
iter = livenessUses.insert({inst, SmallBitVector(getNumSubelements())})
660-
.first;
659+
SmallBitVector &getOrCreateAffectedBits(SILInstruction *inst,
660+
InstToBitMap &map) {
661+
auto iter = map.find(inst);
662+
if (iter == map.end()) {
663+
iter = map.insert({inst, SmallBitVector(getNumSubelements())}).first;
661664
}
662665
return iter->second;
663666
}
664667

668+
void setAffectedBits(SILInstruction *inst, SmallBitVector const &bits,
669+
InstToBitMap &map) {
670+
getOrCreateAffectedBits(inst, map) |= bits;
671+
}
672+
673+
void setAffectedBits(SILInstruction *inst, TypeTreeLeafTypeRange range,
674+
InstToBitMap &map) {
675+
range.setBits(getOrCreateAffectedBits(inst, map));
676+
}
677+
665678
void recordLivenessUse(SILInstruction *inst, SmallBitVector const &bits) {
666-
getOrCreateLivenessUse(inst) |= bits;
679+
setAffectedBits(inst, bits, livenessUses);
667680
}
668681

669682
void recordLivenessUse(SILInstruction *inst, TypeTreeLeafTypeRange range) {
670-
auto &bits = getOrCreateLivenessUse(inst);
671-
range.setBits(bits);
683+
setAffectedBits(inst, range, livenessUses);
672684
}
673685

674686
void recordReinitUse(SILInstruction *inst, TypeTreeLeafTypeRange range) {
675-
auto iter = reinitInsts.find(inst);
676-
if (iter == reinitInsts.end()) {
677-
iter =
678-
reinitInsts.insert({inst, SmallBitVector(getNumSubelements())}).first;
679-
}
680-
auto &bits = iter->second;
681-
range.setBits(bits);
687+
setAffectedBits(inst, range, reinitInsts);
688+
}
689+
690+
void recordInitUse(SILInstruction *inst, TypeTreeLeafTypeRange range) {
691+
setAffectedBits(inst, range, initInsts);
682692
}
683693

684694
/// Returns true if this is a terminator instruction that although it doesn't
@@ -875,7 +885,7 @@ struct UseState {
875885
{
876886
auto iter = initInsts.find(inst);
877887
if (iter != initInsts.end()) {
878-
if (span.setIntersection(iter->second))
888+
if (span.intersects(iter->second))
879889
return true;
880890
}
881891
}
@@ -1020,7 +1030,7 @@ void UseState::initializeLiveness(
10201030
llvm::dbgs()
10211031
<< "Found in/in_guaranteed/inout/inout_aliasable argument as "
10221032
"an init... adding mark_must_check as init!\n");
1023-
initInsts.insert({address, liveness.getTopLevelSpan()});
1033+
recordInitUse(address, liveness.getTopLevelSpan());
10241034
liveness.initializeDef(address, liveness.getTopLevelSpan());
10251035
break;
10261036
case swift::SILArgumentConvention::Indirect_Out:
@@ -1051,14 +1061,14 @@ void UseState::initializeLiveness(
10511061
// later invariants that we assert upon remain true.
10521062
LLVM_DEBUG(llvm::dbgs() << "Found move only arg closure box use... "
10531063
"adding mark_must_check as init!\n");
1054-
initInsts.insert({address, liveness.getTopLevelSpan()});
1064+
recordInitUse(address, liveness.getTopLevelSpan());
10551065
liveness.initializeDef(address, liveness.getTopLevelSpan());
10561066
}
10571067
} else if (auto *box = dyn_cast<AllocBoxInst>(
10581068
lookThroughOwnershipInsts(projectBox->getOperand()))) {
10591069
LLVM_DEBUG(llvm::dbgs() << "Found move only var allocbox use... "
10601070
"adding mark_must_check as init!\n");
1061-
initInsts.insert({address, liveness.getTopLevelSpan()});
1071+
recordInitUse(address, liveness.getTopLevelSpan());
10621072
liveness.initializeDef(address, liveness.getTopLevelSpan());
10631073
}
10641074
}
@@ -1069,7 +1079,7 @@ void UseState::initializeLiveness(
10691079
stripAccessMarkers(address->getOperand()))) {
10701080
LLVM_DEBUG(llvm::dbgs() << "Found ref_element_addr use... "
10711081
"adding mark_must_check as init!\n");
1072-
initInsts.insert({address, liveness.getTopLevelSpan()});
1082+
recordInitUse(address, liveness.getTopLevelSpan());
10731083
liveness.initializeDef(address, liveness.getTopLevelSpan());
10741084
}
10751085

@@ -1079,7 +1089,7 @@ void UseState::initializeLiveness(
10791089
dyn_cast<GlobalAddrInst>(stripAccessMarkers(address->getOperand()))) {
10801090
LLVM_DEBUG(llvm::dbgs() << "Found global_addr use... "
10811091
"adding mark_must_check as init!\n");
1082-
initInsts.insert({address, liveness.getTopLevelSpan()});
1092+
recordInitUse(address, liveness.getTopLevelSpan());
10831093
liveness.initializeDef(address, liveness.getTopLevelSpan());
10841094
}
10851095

@@ -1752,8 +1762,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17521762
if (!leafRange)
17531763
return false;
17541764

1755-
assert(!useState.initInsts.count(user));
1756-
useState.initInsts.insert({user, *leafRange});
1765+
useState.recordInitUse(user, *leafRange);
17571766
return true;
17581767
}
17591768

test/SILOptimizer/moveonly_addresschecker_unmaximized.sil

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ sil @get_M4 : $@convention(thin) () -> @owned M4
1717
sil @end_2 : $@convention(thin) (@owned M, @owned M) -> ()
1818
sil @see_addr_2 : $@convention(thin) (@in_guaranteed M, @in_guaranteed M) -> ()
1919
sil @replace_2 : $@convention(thin) (@inout M, @inout M) -> ()
20+
sil @get_out_2 : $@convention(thin) () -> (@out M, @out M)
2021

2122
/// Two non-contiguous fields (#M4.s2, #M4.s4) are borrowed by @see_addr_2.
2223
/// Two non-contiguous fields (#M4.s1, #M$.s3) are consumed by @end_2.
@@ -107,3 +108,37 @@ bb0:
107108
%22 = tuple ()
108109
return %22 : $()
109110
}
111+
112+
// Verify that M4.s4 is not leaked after it is set.
113+
// CHECK-LABEL: sil [ossa] @rdar111391893 : $@convention(thin) () -> () {
114+
// CHECK: [[GET_OUT_2:%[^,]+]] = function_ref @get_out_2
115+
// CHECK: [[STACK:%[^,]+]] = alloc_stack
116+
// CHECK: [[S2_ADDR_1:%[^,]+]] = struct_element_addr [[STACK]]
117+
// CHECK: [[S4_ADDR_1:%[^,]+]] = struct_element_addr [[STACK]]
118+
// CHECK: apply [[GET_OUT_2]]([[S2_ADDR_1]], [[S4_ADDR_1]])
119+
// CHECK: [[S2_ADDR_2:%[^,]+]] = struct_element_addr [[STACK]]
120+
// CHECK: [[S4_ADDR_4:%[^,]+]] = struct_element_addr [[STACK]]
121+
// CHECK: destroy_addr [[S2_ADDR_2]]
122+
// CHECK: destroy_addr [[S4_ADDR_4]]
123+
// CHECK-LABEL: } // end sil function 'rdar111391893'
124+
sil [ossa] @rdar111391893 : $@convention(thin) () -> () {
125+
%get_out_2 = function_ref @get_out_2 : $@convention(thin) () -> (@out M, @out M)
126+
%end_2 = function_ref @end_2 : $@convention(thin) (@owned M, @owned M) -> ()
127+
%stack_addr = alloc_stack $M4
128+
%stack = mark_must_check [consumable_and_assignable] %stack_addr : $*M4
129+
%s2_addr = struct_element_addr %stack : $*M4, #M4.s2
130+
%s4_addr = struct_element_addr %stack : $*M4, #M4.s4
131+
apply %get_out_2(%s2_addr, %s4_addr) : $@convention(thin) () -> (@out M, @out M)
132+
%s1_addr = struct_element_addr %stack : $*M4, #M4.s1
133+
%s3_addr = struct_element_addr %stack : $*M4, #M4.s3
134+
apply %get_out_2(%s1_addr, %s3_addr) : $@convention(thin) () -> (@out M, @out M)
135+
%12 = struct_element_addr %stack : $*M4, #M4.s1
136+
%13 = load [copy] %12 : $*M
137+
%14 = struct_element_addr %stack : $*M4, #M4.s3
138+
%15 = load [copy] %14 : $*M
139+
%17 = apply %end_2(%13, %15) : $@convention(thin) (@owned M, @owned M) -> ()
140+
destroy_addr %stack : $*M4
141+
dealloc_stack %stack_addr : $*M4
142+
%22 = tuple ()
143+
return %22 : $()
144+
}

0 commit comments

Comments
 (0)