Skip to content

Commit 4a5009a

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 0b30bd7 commit 4a5009a

File tree

2 files changed

+64
-10
lines changed

2 files changed

+64
-10
lines changed

Diff for: lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

+21-9
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ struct UseState {
626626

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

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

674+
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);
682+
}
683+
674684
/// Returns true if this is a terminator instruction that although it doesn't
675685
/// use our inout argument directly is used by the pass to ensure that we
676686
/// reinit said argument if we consumed it in the body of the function.
@@ -778,6 +788,11 @@ struct UseState {
778788
range.setBits(consumingBits);
779789
}
780790

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

@@ -842,7 +857,7 @@ struct UseState {
842857
if (!isReinitToInitConvertibleInst(inst)) {
843858
auto iter = reinitInsts.find(inst);
844859
if (iter != reinitInsts.end()) {
845-
if (span.setIntersection(iter->second))
860+
if (span.intersects(iter->second))
846861
return true;
847862
}
848863
}
@@ -867,7 +882,7 @@ struct UseState {
867882
if (isReinitToInitConvertibleInst(inst)) {
868883
auto iter = reinitInsts.find(inst);
869884
if (iter != reinitInsts.end()) {
870-
if (span.setIntersection(iter->second))
885+
if (span.intersects(iter->second))
871886
return true;
872887
}
873888
}
@@ -1744,11 +1759,10 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17441759

17451760
if (::memInstMustReinitialize(op)) {
17461761
LLVM_DEBUG(llvm::dbgs() << "Found reinit: " << *user);
1747-
assert(!useState.reinitInsts.count(user));
17481762
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
17491763
if (!leafRange)
17501764
return false;
1751-
useState.reinitInsts.insert({user, *leafRange});
1765+
useState.recordReinitUse(user, *leafRange);
17521766
return true;
17531767
}
17541768

@@ -2733,9 +2747,7 @@ void MoveOnlyAddressCheckerPImpl::rewriteUses(
27332747
for (auto reinitPair : addressUseState.reinitInsts) {
27342748
if (!isReinitToInitConvertibleInst(reinitPair.first))
27352749
continue;
2736-
SmallBitVector bits(liveness.getNumSubElements());
2737-
reinitPair.second.setBits(bits);
2738-
if (!consumes.claimConsume(reinitPair.first, bits))
2750+
if (!consumes.claimConsume(reinitPair.first, reinitPair.second))
27392751
convertMemoryReinitToInitForm(reinitPair.first, debugVar);
27402752
}
27412753

@@ -2926,7 +2938,7 @@ void ExtendUnconsumedLiveness::run() {
29262938
}
29272939
}
29282940
for (auto pair : addressUseState.reinitInsts) {
2929-
if (pair.second.contains(element)) {
2941+
if (pair.second.test(element)) {
29302942
destroys[pair.first] = DestroyKind::Reinit;
29312943
}
29322944
}

Diff for: 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)