Skip to content

Commit eb4a25b

Browse files
committed
[MoveOnlyAddressChecker] Fix repr for reinits.
The address checker records instructions that reinit fields in its reinitInsts map. Previously, that map mapped from an instruction to a range of fields of the type. But an instruction can use multiple discontiguous fields of a single value. (Indeed an attempt to add a second range that was reinit'd by an already reinit'ing 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 reinit'd by the instruction. As in #66728 , a SmallBitVector is the representation chosen. rdar://111356251
1 parent aacb538 commit eb4a25b

File tree

2 files changed

+64
-10
lines changed

2 files changed

+64
-10
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

+21-9
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ struct UseState {
625625

626626
/// memInstMustReinitialize insts. Contains both insts like copy_addr/store
627627
/// [assign] that are reinits that we will convert to inits and true reinits.
628-
llvm::SmallMapVector<SILInstruction *, TypeTreeLeafTypeRange, 4> reinitInsts;
628+
llvm::SmallMapVector<SILInstruction *, SmallBitVector, 4> reinitInsts;
629629

630630
/// The set of drop_deinits of this mark_must_check
631631
SmallSetVector<SILInstruction *, 2> dropDeinitInsts;
@@ -670,6 +670,16 @@ struct UseState {
670670
range.setBits(bits);
671671
}
672672

673+
void recordReinitUse(SILInstruction *inst, TypeTreeLeafTypeRange range) {
674+
auto iter = reinitInsts.find(inst);
675+
if (iter == reinitInsts.end()) {
676+
iter =
677+
reinitInsts.insert({inst, SmallBitVector(getNumSubelements())}).first;
678+
}
679+
auto &bits = iter->second;
680+
range.setBits(bits);
681+
}
682+
673683
/// Returns true if this is a terminator instruction that although it doesn't
674684
/// use our inout argument directly is used by the pass to ensure that we
675685
/// reinit said argument if we consumed it in the body of the function.
@@ -777,6 +787,11 @@ struct UseState {
777787
range.setBits(consumingBits);
778788
}
779789

790+
void recordConsumingBlock(SILBasicBlock *block, SmallBitVector &bits) {
791+
auto &consumingBits = getOrCreateConsumingBlock(block);
792+
consumingBits |= bits;
793+
}
794+
780795
void
781796
initializeLiveness(FieldSensitiveMultiDefPrunedLiveRange &prunedLiveness);
782797

@@ -841,7 +856,7 @@ struct UseState {
841856
if (!isReinitToInitConvertibleInst(inst)) {
842857
auto iter = reinitInsts.find(inst);
843858
if (iter != reinitInsts.end()) {
844-
if (span.setIntersection(iter->second))
859+
if (span.intersects(iter->second))
845860
return true;
846861
}
847862
}
@@ -866,7 +881,7 @@ struct UseState {
866881
if (isReinitToInitConvertibleInst(inst)) {
867882
auto iter = reinitInsts.find(inst);
868883
if (iter != reinitInsts.end()) {
869-
if (span.setIntersection(iter->second))
884+
if (span.intersects(iter->second))
870885
return true;
871886
}
872887
}
@@ -1732,11 +1747,10 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17321747

17331748
if (::memInstMustReinitialize(op)) {
17341749
LLVM_DEBUG(llvm::dbgs() << "Found reinit: " << *user);
1735-
assert(!useState.reinitInsts.count(user));
17361750
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
17371751
if (!leafRange)
17381752
return false;
1739-
useState.reinitInsts.insert({user, *leafRange});
1753+
useState.recordReinitUse(user, *leafRange);
17401754
return true;
17411755
}
17421756

@@ -2721,9 +2735,7 @@ void MoveOnlyAddressCheckerPImpl::rewriteUses(
27212735
for (auto reinitPair : addressUseState.reinitInsts) {
27222736
if (!isReinitToInitConvertibleInst(reinitPair.first))
27232737
continue;
2724-
SmallBitVector bits(liveness.getNumSubElements());
2725-
reinitPair.second.setBits(bits);
2726-
if (!consumes.claimConsume(reinitPair.first, bits))
2738+
if (!consumes.claimConsume(reinitPair.first, reinitPair.second))
27272739
convertMemoryReinitToInitForm(reinitPair.first, debugVar);
27282740
}
27292741

@@ -2914,7 +2926,7 @@ void ExtendUnconsumedLiveness::run() {
29142926
}
29152927
}
29162928
for (auto pair : addressUseState.reinitInsts) {
2917-
if (pair.second.contains(element)) {
2929+
if (pair.second.test(element)) {
29182930
destroys[pair.first] = DestroyKind::Reinit;
29192931
}
29202932
}

test/SILOptimizer/moveonly_addresschecker_unmaximized.sil

+43-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ struct M4 {
1616
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) -> ()
19-
19+
sil @replace_2 : $@convention(thin) (@inout M, @inout M) -> ()
2020

2121
/// Two non-contiguous fields (#M4.s2, #M4.s4) are borrowed by @see_addr_2.
2222
/// Two non-contiguous fields (#M4.s1, #M$.s3) are consumed by @end_2.
@@ -65,3 +65,45 @@ bb0:
6565
return %22 : $()
6666
}
6767

68+
// CHECK-LABEL: sil [ossa] @rdar111356251 : $@convention(thin) () -> () {
69+
// CHECK: [[STACK:%[^,]+]] = alloc_stack $M4
70+
// CHECK: [[GET_M4:%[^,]+]] = function_ref @get_M4 : $@convention(thin) () -> @owned M4
71+
// CHECK: [[INSTANCE:%[^,]+]] = apply [[GET_M4]]() : $@convention(thin) () -> @owned M4
72+
// CHECK: store [[INSTANCE]] to [init] [[STACK]] : $*M4
73+
// CHECK: [[S2_ADDR:%[^,]+]] = struct_element_addr [[STACK]] : $*M4, #M4.s2
74+
// CHECK: [[S4_ADDR:%[^,]+]] = struct_element_addr [[STACK]] : $*M4, #M4.s4
75+
// CHECK: [[REPLACE_2:%[^,]+]] = function_ref @replace_2 : $@convention(thin) (@inout M, @inout M) -> ()
76+
// CHECK: apply [[REPLACE_2]]([[S2_ADDR]], [[S4_ADDR]]) : $@convention(thin) (@inout M, @inout M) -> ()
77+
// CHECK: [[S4_ADDR_2:%[^,]+]] = struct_element_addr [[STACK]] : $*M4, #M4.s4
78+
// CHECK: destroy_addr [[S4_ADDR_2]] : $*M
79+
// CHECK: [[S2_ADDR_2:%[^,]+]] = struct_element_addr [[STACK]] : $*M4, #M4.s2
80+
// CHECK: destroy_addr [[S2_ADDR_2]] : $*M
81+
// CHECK: [[S1_ADDR:%[^,]+]] = struct_element_addr [[STACK]] : $*M4, #M4.s1
82+
// CHECK: [[S1:%[^,]+]] = load [take] [[S1_ADDR]] : $*M
83+
// CHECK: [[S3_ADDR:%[^,]+]] = struct_element_addr [[STACK]] : $*M4, #M4.s3
84+
// CHECK: [[S3:%[^,]+]] = load [take] [[S3_ADDR]] : $*M
85+
// CHECK: [[END_2:%[^,]+]] = function_ref @end_2 : $@convention(thin) (@owned M, @owned M) -> ()
86+
// CHECK: apply [[END_2]]([[S1]], [[S3]]) : $@convention(thin) (@owned M, @owned M) -> ()
87+
// CHECK-LABEL: } // end sil function 'rdar111356251'
88+
sil [ossa] @rdar111356251 : $@convention(thin) () -> () {
89+
bb0:
90+
%stack_addr = alloc_stack $M4
91+
%stack = mark_must_check [consumable_and_assignable] %stack_addr : $*M4
92+
%get_M4 = function_ref @get_M4 : $@convention(thin) () -> @owned M4
93+
%instance = apply %get_M4() : $@convention(thin) () -> @owned M4
94+
store %instance to [init] %stack : $*M4
95+
%s2_addr = struct_element_addr %stack : $*M4, #M4.s2
96+
%s4_addr = struct_element_addr %stack : $*M4, #M4.s4
97+
%replace_2 = function_ref @replace_2 : $@convention(thin) (@inout M, @inout M) -> ()
98+
apply %replace_2(%s2_addr, %s4_addr) : $@convention(thin) (@inout M, @inout M) -> ()
99+
%12 = struct_element_addr %stack : $*M4, #M4.s1
100+
%13 = load [copy] %12 : $*M
101+
%14 = struct_element_addr %stack : $*M4, #M4.s3
102+
%15 = load [copy] %14 : $*M
103+
%16 = function_ref @end_2 : $@convention(thin) (@owned M, @owned M) -> ()
104+
%17 = apply %16(%13, %15) : $@convention(thin) (@owned M, @owned M) -> ()
105+
destroy_addr %stack : $*M4
106+
dealloc_stack %stack_addr : $*M4
107+
%22 = tuple ()
108+
return %22 : $()
109+
}

0 commit comments

Comments
 (0)