Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 3662698

Browse files
committedJun 27, 2023
[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 f4268a9 commit 3662698

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
@@ -624,7 +624,7 @@ struct UseState {
624624

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

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

689+
void recordInitUse(SILInstruction *inst, TypeTreeLeafTypeRange range) {
690+
setAffectedBits(inst, range, initInsts);
691+
}
692+
689693
/// Returns true if this is a terminator instruction that although it doesn't
690694
/// use our inout argument directly is used by the pass to ensure that we
691695
/// reinit said argument if we consumed it in the body of the function.
@@ -880,7 +884,7 @@ struct UseState {
880884
{
881885
auto iter = initInsts.find(inst);
882886
if (iter != initInsts.end()) {
883-
if (span.setIntersection(iter->second))
887+
if (span.intersects(iter->second))
884888
return true;
885889
}
886890
}
@@ -1021,7 +1025,7 @@ void UseState::initializeLiveness(
10211025
llvm::dbgs()
10221026
<< "Found in/in_guaranteed/inout/inout_aliasable argument as "
10231027
"an init... adding mark_must_check as init!\n");
1024-
initInsts.insert({address, liveness.getTopLevelSpan()});
1028+
recordInitUse(address, liveness.getTopLevelSpan());
10251029
liveness.initializeDef(address, liveness.getTopLevelSpan());
10261030
break;
10271031
case swift::SILArgumentConvention::Indirect_Out:
@@ -1052,14 +1056,14 @@ void UseState::initializeLiveness(
10521056
// later invariants that we assert upon remain true.
10531057
LLVM_DEBUG(llvm::dbgs() << "Found move only arg closure box use... "
10541058
"adding mark_must_check as init!\n");
1055-
initInsts.insert({address, liveness.getTopLevelSpan()});
1059+
recordInitUse(address, liveness.getTopLevelSpan());
10561060
liveness.initializeDef(address, liveness.getTopLevelSpan());
10571061
}
10581062
} else if (auto *box = dyn_cast<AllocBoxInst>(
10591063
lookThroughOwnershipInsts(projectBox->getOperand()))) {
10601064
LLVM_DEBUG(llvm::dbgs() << "Found move only var allocbox use... "
10611065
"adding mark_must_check as init!\n");
1062-
initInsts.insert({address, liveness.getTopLevelSpan()});
1066+
recordInitUse(address, liveness.getTopLevelSpan());
10631067
liveness.initializeDef(address, liveness.getTopLevelSpan());
10641068
}
10651069
}
@@ -1070,7 +1074,7 @@ void UseState::initializeLiveness(
10701074
stripAccessMarkers(address->getOperand()))) {
10711075
LLVM_DEBUG(llvm::dbgs() << "Found ref_element_addr use... "
10721076
"adding mark_must_check as init!\n");
1073-
initInsts.insert({address, liveness.getTopLevelSpan()});
1077+
recordInitUse(address, liveness.getTopLevelSpan());
10741078
liveness.initializeDef(address, liveness.getTopLevelSpan());
10751079
}
10761080

@@ -1080,7 +1084,7 @@ void UseState::initializeLiveness(
10801084
dyn_cast<GlobalAddrInst>(stripAccessMarkers(address->getOperand()))) {
10811085
LLVM_DEBUG(llvm::dbgs() << "Found global_addr use... "
10821086
"adding mark_must_check as init!\n");
1083-
initInsts.insert({address, liveness.getTopLevelSpan()});
1087+
recordInitUse(address, liveness.getTopLevelSpan());
10841088
liveness.initializeDef(address, liveness.getTopLevelSpan());
10851089
}
10861090

@@ -1746,8 +1750,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17461750
if (!leafRange)
17471751
return false;
17481752

1749-
assert(!useState.initInsts.count(user));
1750-
useState.initInsts.insert({user, *leafRange});
1753+
useState.recordInitUse(user, *leafRange);
17511754
return true;
17521755
}
17531756

‎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)
Please sign in to comment.