Skip to content

Commit f4e3fce

Browse files
committed
[MoveOnlyAddressChecker] Fix repr for initInsts.
The address checker records instructions that initialize fields in its initInsts map. Previously, that map mapped from an instruction to a range of fields of the type. But an instruction can initialize multiple discontiguous fields of a single value. (Indeed an attempt to add a second range that was initialized by an already initializing instruction--even if it were overlapping or adjacent--would have no effect and the map wouldn't be updated.) Here, this is fixed by fixing the representation and updating the storage whenver a new range is seen to be initialized by the instruction. As in #66728 , a SmallBitVector is the representation chosen. rdar://111391893
1 parent 69c5ae3 commit f4e3fce

File tree

2 files changed

+47
-9
lines changed

2 files changed

+47
-9
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ struct UseState {
625625

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

630630
/// memInstMustReinitialize insts. Contains both insts like copy_addr/store
631631
/// [assign] that are reinits that we will convert to inits and true reinits.
@@ -687,6 +687,10 @@ struct UseState {
687687
setAffectedBits(inst, range, reinitInsts);
688688
}
689689

690+
void recordInitUse(SILInstruction *inst, TypeTreeLeafTypeRange range) {
691+
setAffectedBits(inst, range, initInsts);
692+
}
693+
690694
/// Returns true if this is a terminator instruction that although it doesn't
691695
/// use our inout argument directly is used by the pass to ensure that we
692696
/// reinit said argument if we consumed it in the body of the function.
@@ -881,7 +885,7 @@ struct UseState {
881885
{
882886
auto iter = initInsts.find(inst);
883887
if (iter != initInsts.end()) {
884-
if (span.setIntersection(iter->second))
888+
if (span.intersects(iter->second))
885889
return true;
886890
}
887891
}
@@ -1026,7 +1030,7 @@ void UseState::initializeLiveness(
10261030
llvm::dbgs()
10271031
<< "Found in/in_guaranteed/inout/inout_aliasable argument as "
10281032
"an init... adding mark_must_check as init!\n");
1029-
initInsts.insert({address, liveness.getTopLevelSpan()});
1033+
recordInitUse(address, liveness.getTopLevelSpan());
10301034
liveness.initializeDef(address, liveness.getTopLevelSpan());
10311035
break;
10321036
case swift::SILArgumentConvention::Indirect_Out:
@@ -1057,14 +1061,14 @@ void UseState::initializeLiveness(
10571061
// later invariants that we assert upon remain true.
10581062
LLVM_DEBUG(llvm::dbgs() << "Found move only arg closure box use... "
10591063
"adding mark_must_check as init!\n");
1060-
initInsts.insert({address, liveness.getTopLevelSpan()});
1064+
recordInitUse(address, liveness.getTopLevelSpan());
10611065
liveness.initializeDef(address, liveness.getTopLevelSpan());
10621066
}
10631067
} else if (auto *box = dyn_cast<AllocBoxInst>(
10641068
lookThroughOwnershipInsts(projectBox->getOperand()))) {
10651069
LLVM_DEBUG(llvm::dbgs() << "Found move only var allocbox use... "
10661070
"adding mark_must_check as init!\n");
1067-
initInsts.insert({address, liveness.getTopLevelSpan()});
1071+
recordInitUse(address, liveness.getTopLevelSpan());
10681072
liveness.initializeDef(address, liveness.getTopLevelSpan());
10691073
}
10701074
}
@@ -1075,7 +1079,7 @@ void UseState::initializeLiveness(
10751079
stripAccessMarkers(address->getOperand()))) {
10761080
LLVM_DEBUG(llvm::dbgs() << "Found ref_element_addr use... "
10771081
"adding mark_must_check as init!\n");
1078-
initInsts.insert({address, liveness.getTopLevelSpan()});
1082+
recordInitUse(address, liveness.getTopLevelSpan());
10791083
liveness.initializeDef(address, liveness.getTopLevelSpan());
10801084
}
10811085

@@ -1085,7 +1089,7 @@ void UseState::initializeLiveness(
10851089
dyn_cast<GlobalAddrInst>(stripAccessMarkers(address->getOperand()))) {
10861090
LLVM_DEBUG(llvm::dbgs() << "Found global_addr use... "
10871091
"adding mark_must_check as init!\n");
1088-
initInsts.insert({address, liveness.getTopLevelSpan()});
1092+
recordInitUse(address, liveness.getTopLevelSpan());
10891093
liveness.initializeDef(address, liveness.getTopLevelSpan());
10901094
}
10911095

@@ -1758,8 +1762,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17581762
if (!leafRange)
17591763
return false;
17601764

1761-
assert(!useState.initInsts.count(user));
1762-
useState.initInsts.insert({user, *leafRange});
1765+
useState.recordInitUse(user, *leafRange);
17631766
return true;
17641767
}
17651768

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)