Skip to content

Commit 925a211

Browse files
gottesmmgottesmm
authored
Merge pull request #60989 from gottesmm/pr-d5933fd70a08a0acd36e29c39312cf34cc50f904
[move-only] Fix a few small issues around mark must check.
2 parents bea8a3d + 03986db commit 925a211

File tree

8 files changed

+141
-9
lines changed

8 files changed

+141
-9
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Utilities/WalkUtils.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ extension ValueDefUseWalker {
294294
case is InitExistentialRefInst, is OpenExistentialRefInst,
295295
is BeginBorrowInst, is CopyValueInst,
296296
is UpcastInst, is UncheckedRefCastInst, is EndCOWMutationInst,
297-
is RefToBridgeObjectInst, is BridgeObjectToRefInst:
297+
is RefToBridgeObjectInst, is BridgeObjectToRefInst, is MarkMustCheckInst:
298298
return walkDownUses(ofValue: (instruction as! SingleValueInstruction), path: path)
299299
case let mdi as MarkDependenceInst:
300300
if operand.index == 0 {
@@ -419,7 +419,7 @@ extension AddressDefUseWalker {
419419
return unmatchedPath(address: operand, path: path)
420420
}
421421
case is InitExistentialAddrInst, is OpenExistentialAddrInst, is BeginAccessInst,
422-
is IndexAddrInst:
422+
is IndexAddrInst, is MarkMustCheckInst:
423423
// FIXME: for now `index_addr` is treated as a forwarding instruction since
424424
// SmallProjectionPath does not track indices.
425425
// This is ok since `index_addr` is eventually preceeded by a `tail_addr`
@@ -558,7 +558,7 @@ extension ValueUseDefWalker {
558558
case is InitExistentialRefInst, is OpenExistentialRefInst,
559559
is BeginBorrowInst, is CopyValueInst,
560560
is UpcastInst, is UncheckedRefCastInst, is EndCOWMutationInst,
561-
is RefToBridgeObjectInst, is BridgeObjectToRefInst:
561+
is RefToBridgeObjectInst, is BridgeObjectToRefInst, is MarkMustCheckInst:
562562
return walkUp(value: (def as! Instruction).operands[0].value, path: path)
563563
case let arg as BlockArgument:
564564
if arg.isPhiArgument {
@@ -643,7 +643,8 @@ extension AddressUseDefWalker {
643643
case is InitEnumDataAddrInst, is UncheckedTakeEnumDataAddrInst:
644644
return walkUp(address: (def as! UnaryInstruction).operand,
645645
path: path.push(.enumCase, index: (def as! EnumInstruction).caseIndex))
646-
case is InitExistentialAddrInst, is OpenExistentialAddrInst, is BeginAccessInst, is IndexAddrInst:
646+
case is InitExistentialAddrInst, is OpenExistentialAddrInst, is BeginAccessInst, is IndexAddrInst,
647+
is MarkMustCheckInst:
647648
// FIXME: for now `index_addr` is treated as a forwarding instruction since
648649
// SmallProjectionPath does not track indices.
649650
// This is ok since `index_addr` is eventually preceeded by a `tail_addr`

‎SwiftCompilerSources/Sources/SIL/Instruction.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,8 @@ final public class IsUniqueInst : SingleValueInstruction, UnaryInstruction {}
639639

640640
final public class IsEscapingClosureInst : SingleValueInstruction, UnaryInstruction {}
641641

642+
final public
643+
class MarkMustCheckInst : SingleValueInstruction, UnaryInstruction {}
642644

643645
//===----------------------------------------------------------------------===//
644646
// single-value allocation instructions

‎SwiftCompilerSources/Sources/SIL/Registration.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public func registerSILClasses() {
6666
register(BuiltinInst.self)
6767
register(UpcastInst.self)
6868
register(UncheckedRefCastInst.self)
69+
register(MarkMustCheckInst.self)
6970
register(RawPointerToRefInst.self)
7071
register(AddressToPointerInst.self)
7172
register(PointerToAddressInst.self)

‎include/swift/SIL/MemAccessUtils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1603,6 +1603,7 @@ inline bool isAccessStorageIdentityCast(SingleValueInstruction *svi) {
16031603

16041604
// Simply pass-thru the incoming address.
16051605
case SILInstructionKind::MarkUninitializedInst:
1606+
case SILInstructionKind::MarkMustCheckInst:
16061607
case SILInstructionKind::MarkDependenceInst:
16071608
case SILInstructionKind::CopyValueInst:
16081609
return true;

‎lib/SIL/Parser/ParseSIL.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3434,11 +3434,6 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
34343434
}
34353435

34363436
case SILInstructionKind::MarkMustCheckInst: {
3437-
if (parseTypedValueRef(Val, B))
3438-
return true;
3439-
if (parseSILDebugLocation(InstLoc, B))
3440-
return true;
3441-
34423437
StringRef AttrName;
34433438
if (!parseSILOptional(AttrName, *this)) {
34443439
auto diag = diag::sil_markmustcheck_requires_attribute;
@@ -3457,6 +3452,12 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
34573452
P.diagnose(InstLoc.getSourceLoc(), diag, AttrName);
34583453
return true;
34593454
}
3455+
3456+
if (parseTypedValueRef(Val, B))
3457+
return true;
3458+
if (parseSILDebugLocation(InstLoc, B))
3459+
return true;
3460+
34603461
auto *MVI = B.createMarkMustCheckInst(InstLoc, Val, CKind);
34613462
ResultVal = MVI;
34623463
break;

‎test/SIL/Parser/basic2_moveonly.sil

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// RUN: %target-sil-opt -enable-experimental-move-only %s | %target-sil-opt -enable-experimental-move-only | %FileCheck %s
2+
3+
// Once move only is no longer behind a feature flag, merge this into basic2.
4+
5+
sil_stage raw
6+
7+
class Klass {}
8+
@_moveOnly struct MoveOnlyPair {
9+
var lhs: Klass
10+
var rhs: Klass
11+
}
12+
13+
// CHECK-LABEL: sil [ossa] @testMarkMustCheckInst : $@convention(thin) (@guaranteed Klass) -> () {
14+
// CHECK: mark_must_check [no_implicit_copy] %{{[0-9]+}} : $*MoveOnlyPair
15+
// CHECK: } // end sil function 'testMarkMustCheckInst'
16+
sil [ossa] @testMarkMustCheckInst : $@convention(thin) (@guaranteed Klass) -> () {
17+
bb0(%0 : @guaranteed $Klass):
18+
%1 = alloc_box ${ var MoveOnlyPair }
19+
%2 = project_box %1 : ${ var MoveOnlyPair }, 0
20+
%3 = mark_must_check [no_implicit_copy] %2 : $*MoveOnlyPair
21+
%3c = begin_access [modify] [static] %3 : $*MoveOnlyPair
22+
%3a = struct_element_addr %3c : $*MoveOnlyPair, #MoveOnlyPair.lhs
23+
%3b = struct_element_addr %3c : $*MoveOnlyPair, #MoveOnlyPair.rhs
24+
%0a = copy_value %0 : $Klass
25+
%0b = copy_value %0 : $Klass
26+
store %0a to [init] %3a : $*Klass
27+
store %0b to [init] %3b : $*Klass
28+
end_access %3c : $*MoveOnlyPair
29+
30+
destroy_value %1 : ${ var MoveOnlyPair }
31+
32+
%9999 = tuple()
33+
return %9999 : $()
34+
}

‎test/SIL/Serialization/basic2_moveonly.sil

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-sil-opt -enable-experimental-move-only %s -emit-sib -o %t/tmp.sib -module-name basic2
3+
// RUN: %target-sil-opt -enable-experimental-move-only %t/tmp.sib -o %t/tmp.2.sib -module-name basic2
4+
// RUN: %target-sil-opt -enable-experimental-move-only %t/tmp.2.sib -module-name basic2 -emit-sorted-sil | %FileCheck %s
5+
6+
// Once move only is no longer behind a feature flag, merge this into basic2.
7+
8+
sil_stage raw
9+
10+
class Klass {}
11+
@_moveOnly struct MoveOnlyPair {
12+
var lhs: Klass
13+
var rhs: Klass
14+
}
15+
16+
// CHECK-LABEL: sil [ossa] @testMarkMustCheckInst : $@convention(thin) (@guaranteed Klass) -> () {
17+
// CHECK: mark_must_check [no_implicit_copy] %{{[0-9]+}} : $*MoveOnlyPair
18+
// CHECK: } // end sil function 'testMarkMustCheckInst'
19+
sil [ossa] @testMarkMustCheckInst : $@convention(thin) (@guaranteed Klass) -> () {
20+
bb0(%0 : @guaranteed $Klass):
21+
%1 = alloc_box ${ var MoveOnlyPair }
22+
%2 = project_box %1 : ${ var MoveOnlyPair }, 0
23+
%3 = mark_must_check [no_implicit_copy] %2 : $*MoveOnlyPair
24+
%3c = begin_access [modify] [static] %3 : $*MoveOnlyPair
25+
%3a = struct_element_addr %3c : $*MoveOnlyPair, #MoveOnlyPair.lhs
26+
%3b = struct_element_addr %3c : $*MoveOnlyPair, #MoveOnlyPair.rhs
27+
%0a = copy_value %0 : $Klass
28+
%0b = copy_value %0 : $Klass
29+
store %0a to [init] %3a : $*Klass
30+
store %0b to [init] %3b : $*Klass
31+
end_access %3c : $*MoveOnlyPair
32+
33+
destroy_value %1 : ${ var MoveOnlyPair }
34+
35+
%9999 = tuple()
36+
return %9999 : $()
37+
}

‎test/SILOptimizer/accessutils_raw.sil

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %target-sil-opt -enable-experimental-move-only %s -dump-access -o /dev/null | %FileCheck %s
2+
3+
// REQUIRES: swift_in_compiler
4+
5+
// Test is failing when targeting ARMv7k/ARM64_32. rdar://98669547
6+
// UNSUPPORTED: CPU=armv7k || CPU=arm64_32
7+
8+
sil_stage raw
9+
10+
import Builtin
11+
12+
class List {
13+
var x: Builtin.Int64
14+
let next: List
15+
}
16+
17+
@_moveOnly
18+
struct MoveOnlyPair {
19+
var lhs: List
20+
var rhs: List
21+
}
22+
23+
// CHECK-LABEL: Accesses for testMarkMustCheckInst
24+
// CHECK-NEXT: Value: %5 = struct_element_addr %4 : $*MoveOnlyPair, #MoveOnlyPair.lhs
25+
// CHECK-NEXT: Scope: %4 = begin_access [modify] [static] %3 : $*MoveOnlyPair
26+
// CHECK-NEXT: Base: box - %2 = project_box %1 : ${ var MoveOnlyPair }, 0
27+
// CHECK-NEXT: Path: "s0"
28+
// CHECK-NEXT: Storage: %1 = alloc_box ${ var MoveOnlyPair }
29+
// CHECK-NEXT: Path: "c0.s0"
30+
// CHECK-NEXT: Value: %6 = struct_element_addr %4 : $*MoveOnlyPair, #MoveOnlyPair.rhs
31+
// CHECK-NEXT: Scope: %4 = begin_access [modify] [static] %3 : $*MoveOnlyPair
32+
// CHECK-NEXT: Base: box - %2 = project_box %1 : ${ var MoveOnlyPair }, 0
33+
// CHECK-NEXT: Path: "s1"
34+
// CHECK-NEXT: Storage: %1 = alloc_box ${ var MoveOnlyPair }
35+
// CHECK-NEXT: Path: "c0.s1"
36+
// CHECK-NEXT: End accesses for testMarkMustCheckInst
37+
sil [ossa] @testMarkMustCheckInst : $@convention(thin) (@guaranteed List) -> () {
38+
bb0(%0 : @guaranteed $List):
39+
%1 = alloc_box ${ var MoveOnlyPair }
40+
%2 = project_box %1 : ${ var MoveOnlyPair }, 0
41+
%3 = mark_must_check [no_implicit_copy] %2 : $*MoveOnlyPair
42+
%3c = begin_access [modify] [static] %3 : $*MoveOnlyPair
43+
%3a = struct_element_addr %3c : $*MoveOnlyPair, #MoveOnlyPair.lhs
44+
%3b = struct_element_addr %3c : $*MoveOnlyPair, #MoveOnlyPair.rhs
45+
%0a = copy_value %0 : $List
46+
%0b = copy_value %0 : $List
47+
store %0a to [init] %3a : $*List
48+
store %0b to [init] %3b : $*List
49+
end_access %3c : $*MoveOnlyPair
50+
51+
destroy_value %1 : ${ var MoveOnlyPair }
52+
53+
%9999 = tuple()
54+
return %9999 : $()
55+
}

0 commit comments

Comments
 (0)