Skip to content

Commit 82f1ea2

Browse files
gottesmmgottesmm
authored
Merge pull request #67350 from gottesmm/pr-3ec27a1692f3f0da348c0327ac682ba616f46c6b
[move-only] Make sure that we mask out liveness bits and use |= when merging successors
2 parents 75ea47d + d7d7ab6 commit 82f1ea2

File tree

4 files changed

+111
-7
lines changed

4 files changed

+111
-7
lines changed

‎include/swift/SIL/FieldSensitivePrunedLiveness.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -724,17 +724,22 @@ class FieldSensitivePrunedLiveness {
724724

725725
/// Populates the provided vector with contiguous ranges of bits which are
726726
/// users of the same sort.
727+
///
728+
/// All bits not selected by \p selectedBits are assumed to be
729+
/// IsInterestingUser::NonUser.
727730
void getContiguousRanges(
728731
SmallVectorImpl<std::pair<TypeTreeLeafTypeRange, IsInterestingUser>>
729-
&ranges) const {
732+
&ranges,
733+
const SmallBitVector &selectedBits) const {
730734
if (liveBits.size() == 0)
731735
return;
732736

733737
assert(ranges.empty());
734738
llvm::Optional<std::pair<unsigned, IsInterestingUser>> current =
735739
llvm::None;
736740
for (unsigned bit = 0, size = liveBits.size(); bit < size; ++bit) {
737-
auto interesting = isInterestingUser(bit);
741+
auto interesting = selectedBits.test(bit) ? isInterestingUser(bit)
742+
: IsInterestingUser::NonUser;
738743
if (!current) {
739744
current = {bit, interesting};
740745
continue;

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,7 +2761,7 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
27612761
auto interestingUser = liveness.getInterestingUser(inst);
27622762
SmallVector<std::pair<TypeTreeLeafTypeRange, IsInterestingUser>, 4> ranges;
27632763
if (interestingUser) {
2764-
interestingUser->getContiguousRanges(ranges);
2764+
interestingUser->getContiguousRanges(ranges, bv);
27652765
}
27662766

27672767
for (auto rangePair : ranges) {
@@ -2799,12 +2799,14 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
27992799
auto *block = inst->getParent();
28002800
for (auto *succBlock : block->getSuccessorBlocks()) {
28012801
auto iter = mergeBlocks.find(succBlock);
2802-
if (iter == mergeBlocks.end())
2802+
if (iter == mergeBlocks.end()) {
28032803
iter = mergeBlocks.insert({succBlock, bits}).first;
2804-
else {
2804+
} else {
2805+
// NOTE: We use |= here so that different regions of the same
2806+
// terminator get updated appropriately.
28052807
SmallBitVector &alreadySetBits = iter->second;
28062808
bool hadCommon = alreadySetBits.anyCommon(bits);
2807-
alreadySetBits &= bits;
2809+
alreadySetBits |= bits;
28082810
if (hadCommon)
28092811
continue;
28102812
}

‎test/SILOptimizer/moveonly_addresschecker.sil‎

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,18 @@ sil @finalizeSingleTrivialFieldAndDeinit : $@convention(thin) (@owned SingleTriv
6969
sil_global hidden @$s23moveonly_addresschecker9varGlobalAA16NonTrivialStructVvp : $NonTrivialStruct
7070
sil_global hidden [let] @$s23moveonly_addresschecker9letGlobalAA16NonTrivialStructVvp : $NonTrivialStruct
7171

72+
struct NonCopyableNativeObjectIntPair : ~Copyable {
73+
var a: Builtin.NativeObject
74+
var currentPosition: Int
75+
}
76+
77+
struct NonCopyableNativeObjectPair : ~Copyable {
78+
var a: Builtin.NativeObject
79+
var currentPosition: Builtin.NativeObject
80+
}
81+
82+
sil @get_nativeobject : $@convention(thin) () -> @owned Builtin.NativeObject
83+
7284
///////////
7385
// Tests //
7486
///////////
@@ -602,7 +614,7 @@ sil @get_M2 : $@convention(thin) () -> @owned M2
602614
sil @end_addr_see_addr : $@convention(thin) (@in M, @in_guaranteed M) -> ()
603615

604616
/// A single instruction, apply @end_addr_see_addr, consumes one field and
605-
/// borrows another.
617+
/// borrows another.
606618

607619
/// Varify that the consumed value isn't destroyed twice and that the borrowed
608620
/// value isn't destroyed before it's used.
@@ -768,3 +780,67 @@ bb3:
768780
%9999 = tuple ()
769781
return %9999 : $()
770782
}
783+
784+
sil @testUseCorrectBitsClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectIntPair) -> Bool
785+
sil @testUseCorrectBitsClosureUser : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
786+
787+
// CHECK-LABEL: sil [ossa] @testUseCorrectBits : $@convention(method) (Int, @inout NonCopyableNativeObjectIntPair) -> () {
788+
// CHECK-NOT: destroy_addr
789+
// CHECK: } // end sil function 'testUseCorrectBits'
790+
sil [ossa] @testUseCorrectBits : $@convention(method) (Int, @inout NonCopyableNativeObjectIntPair) -> () {
791+
bb0(%0 : $Int, %1a : $*NonCopyableNativeObjectIntPair):
792+
debug_value %0 : $Int, let, name "index", argno 1
793+
debug_value %1a : $*NonCopyableNativeObjectIntPair, var, name "self", argno 2, implicit, expr op_deref
794+
%1 = mark_must_check [consumable_and_assignable] %1a : $*NonCopyableNativeObjectIntPair
795+
%4 = function_ref @testUseCorrectBitsClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectIntPair) -> Bool
796+
%5 = partial_apply [callee_guaranteed] [on_stack] %4(%0, %1) : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectIntPair) -> Bool
797+
%6 = mark_dependence %5 : $@noescape @callee_guaranteed () -> Bool on %1 : $*NonCopyableNativeObjectIntPair
798+
%7 = function_ref @testUseCorrectBitsClosureUser : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
799+
%8 = apply %7(%6) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
800+
%9 = struct_element_addr %1 : $*NonCopyableNativeObjectIntPair, #NonCopyableNativeObjectIntPair.currentPosition
801+
debug_value undef : $*NonCopyableNativeObjectIntPair, var, name "self", argno 2, implicit, expr op_deref
802+
destroy_value %6 : $@noescape @callee_guaranteed () -> Bool
803+
%12 = begin_access [modify] [static] %1 : $*NonCopyableNativeObjectIntPair
804+
%13 = struct_element_addr %12 : $*NonCopyableNativeObjectIntPair, #NonCopyableNativeObjectIntPair.currentPosition
805+
store %0 to [trivial] %13 : $*Int
806+
end_access %12 : $*NonCopyableNativeObjectIntPair
807+
%16 = tuple ()
808+
return %16 : $()
809+
}
810+
811+
sil @testUseCorrectBits2ClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectPair) -> Bool
812+
813+
// CHECK-LABEL: sil [ossa] @testUseCorrectBits2 : $@convention(method) (Int, @inout NonCopyableNativeObjectPair) -> () {
814+
// CHECK: bb0({{%.*}} : $Int, [[ARG:%.*]] : $*
815+
// CHECK-NOT: destroy_addr
816+
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] [[ARG]]
817+
// CHECK-NEXT: [[GEP:%.*]] = struct_element_addr [[ACCESS]]
818+
// CHECK-NEXT: function_ref get_nativeobject
819+
// CHECK-NEXT: function_ref @get_nativeobject
820+
// CHECK-NEXT: apply {{%.*}}
821+
// CHECK-NEXT: [[GEP2:%.*]] = struct_element_addr [[ARG]] : $*NonCopyableNativeObjectPair, #NonCopyableNativeObjectPair.currentPosition
822+
// CHECK-NEXT: destroy_addr [[GEP2]]
823+
// CHECK-NOT: destroy_addr
824+
// CHECK: } // end sil function 'testUseCorrectBits2'
825+
sil [ossa] @testUseCorrectBits2 : $@convention(method) (Int, @inout NonCopyableNativeObjectPair) -> () {
826+
bb0(%0 : $Int, %1a : $*NonCopyableNativeObjectPair):
827+
debug_value %0 : $Int, let, name "index", argno 1
828+
debug_value %1a : $*NonCopyableNativeObjectPair, var, name "self", argno 2, implicit, expr op_deref
829+
%1 = mark_must_check [consumable_and_assignable] %1a : $*NonCopyableNativeObjectPair
830+
%4 = function_ref @testUseCorrectBits2ClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectPair) -> Bool
831+
%5 = partial_apply [callee_guaranteed] [on_stack] %4(%0, %1) : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectPair) -> Bool
832+
%6 = mark_dependence %5 : $@noescape @callee_guaranteed () -> Bool on %1 : $*NonCopyableNativeObjectPair
833+
%7 = function_ref @testUseCorrectBitsClosureUser : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
834+
%8 = apply %7(%6) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
835+
%9 = struct_element_addr %1 : $*NonCopyableNativeObjectPair, #NonCopyableNativeObjectPair.currentPosition
836+
debug_value undef : $*NonCopyableNativeObjectPair, var, name "self", argno 2, implicit, expr op_deref
837+
destroy_value %6 : $@noescape @callee_guaranteed () -> Bool
838+
%12 = begin_access [modify] [static] %1 : $*NonCopyableNativeObjectPair
839+
%13 = struct_element_addr %12 : $*NonCopyableNativeObjectPair, #NonCopyableNativeObjectPair.currentPosition
840+
%f = function_ref @get_nativeobject : $@convention(thin) () -> @owned Builtin.NativeObject
841+
%14 = apply %f() : $@convention(thin) () -> @owned Builtin.NativeObject
842+
store %14 to [assign] %13 : $*Builtin.NativeObject
843+
end_access %12 : $*NonCopyableNativeObjectPair
844+
%16 = tuple ()
845+
return %16 : $()
846+
}

‎test/SILOptimizer/moveonly_addresschecker.swift‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s -Xllvm -sil-print-final-ossa-module | %FileCheck %s
12
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
23

34
// This file contains tests that used to crash due to verifier errors. It must
@@ -14,3 +15,23 @@ struct TestTrivialReturnValue : ~Copyable {
1415
return buffer
1516
}
1617
}
18+
19+
20+
//////////////////////
21+
// MARK: Misc Tests //
22+
//////////////////////
23+
24+
func testAssertLikeUseDifferentBits() {
25+
struct S : ~Copyable {
26+
var s: [Int] = []
27+
var currentPosition = 5
28+
29+
// CHECK-LABEL: sil private @$s23moveonly_addresschecker30testAssertLikeUseDifferentBitsyyF1SL_V6resume2atySi_tF : $@convention(method) (Int, @inout S) -> () {
30+
// CHECK-NOT: destroy_addr
31+
// CHECK: } // end sil function '$s23moveonly_addresschecker30testAssertLikeUseDifferentBitsyyF1SL_V6resume2atySi_tF'
32+
mutating func resume(at index: Int) {
33+
assert(index >= currentPosition)
34+
currentPosition = index
35+
}
36+
}
37+
}

0 commit comments

Comments
 (0)