Skip to content

Commit 3088fe1

Browse files
gottesmmgottesmm
committed
[move-only] Add support in SILGen for unwrapping moveonlywrapped values before materializing for a function argument.
I discovered this as I began to write some Interpreter tests for move only. This was triggered by SILGen attempting to pass a non-trivial type to StringBuilder which is generic, so we needed to materialize. I also discovered a small bug where we were not properly ignoring class_method in the move only type eliminator. I just folded the small fix + a test for that into this commit.
1 parent 2f9d67d commit 3088fe1

File tree

6 files changed

+124
-2
lines changed

6 files changed

+124
-2
lines changed

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3082,6 +3082,18 @@ class ArgEmitter {
30823082

30833083
// If it's not already in memory, put it there.
30843084
if (!result.getType().isAddress()) {
3085+
// If we have a move only wrapped type, we need to unwrap before we
3086+
// materialize. We will forward as appropriate so it will show up as a
3087+
// consuming use or a guaranteed use as appropriate.
3088+
if (result.getType().isMoveOnlyWrapped()) {
3089+
if (result.isPlusOne(SGF)) {
3090+
result =
3091+
SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(loc, result);
3092+
} else {
3093+
result = SGF.B.createGuaranteedMoveOnlyWrapperToCopyableValue(
3094+
loc, result);
3095+
}
3096+
}
30853097
result = result.materialize(SGF, loc);
30863098
}
30873099

‎lib/SILGen/SILGenBuilder.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,5 +918,6 @@ ManagedValue SILGenBuilder::createGuaranteedMoveOnlyWrapperToCopyableValue(
918918
SILLocation loc, ManagedValue value) {
919919
auto *mdi =
920920
createGuaranteedMoveOnlyWrapperToCopyableValue(loc, value.getValue());
921+
assert(mdi->getOperand()->getType().isObject() && "Expected an object?!");
921922
return ManagedValue::forUnmanaged(mdi);
922923
}

‎lib/SILOptimizer/Mandatory/MoveOnlyTypeEliminator.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ struct SILMoveOnlyTypeEliminatorVisitor
182182
NO_UPDATE_NEEDED(RefToBridgeObject)
183183
NO_UPDATE_NEEDED(BridgeObjectToRef)
184184
NO_UPDATE_NEEDED(UnconditionalCheckedCast)
185+
NO_UPDATE_NEEDED(ClassMethod)
185186
#undef NO_UPDATE_NEEDED
186187
};
187188

‎test/Interpreter/noimplicitcopy.swift‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// RUN: %target-run-simple-swift( -Xfrontend -enable-experimental-move-only ) | %FileCheck %s
2+
3+
// A small sanity check.
4+
5+
// REQUIRES: executable_test
6+
7+
import StdlibUnittest
8+
9+
var Tests = TestSuite("TrivialMoveOnly")
10+
defer { runAllTests() }
11+
12+
func print2(_ x: Int) {
13+
print("printInt: \(x + 1)")
14+
}
15+
16+
// CHECK: printInt: 11
17+
// CHECK: printInt: 5
18+
Tests.test("printInt") {
19+
@_noImplicitCopy let x: Int = 5
20+
// Infix operators where we pass values as guaranteed parameters.
21+
print2(x + x)
22+
print("printInt: \(x)")
23+
}
24+
25+
class Klass {
26+
var i = 8
27+
func increment() { i += 1 }
28+
}
29+
30+
// CHECK: printKlass: main.Klass
31+
// CHECK: printKlass: 10
32+
Tests.test("printKlass") {
33+
@_noImplicitCopy let x = Klass()
34+
x.increment()
35+
x.increment()
36+
print("printKlass: \(x)")
37+
print("printKlass: \(x.i)")
38+
}

‎test/SILGen/noimplicitcopy.swift‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// RUN: %target-swift-frontend -emit-silgen -enable-experimental-move-only %s | %FileCheck %s
2+
3+
//////////////////
4+
// Declarations //
5+
//////////////////
6+
7+
class Klass {
8+
var i = 8
9+
func increment() { i += 1 }
10+
}
11+
12+
///////////
13+
// Tests //
14+
///////////
15+
func print2(_ x: Int) {
16+
print("printInt: \(x + 1)")
17+
}
18+
19+
// CHECK: sil hidden [ossa] @$s14noimplicitcopy8printIntyyF : $@convention(thin) () -> () {
20+
// CHECK: [[X_MOVEONLY:%.*]] = copyable_to_moveonlywrapper {{%.*}} : $Int
21+
// CHECK: [[X:%.*]] = begin_borrow [lexical] [[X_MOVEONLY]]
22+
// CHECK: [[X_COPY:%.*]] = explicit_copy_value [[X]]
23+
// CHECK: [[X_MOVEONLYWRAPPED_MARKED:%.*]] = mark_must_check [no_implicit_copy] [[X_COPY]]
24+
// CHECK: [[BORROWED_X_MOVEONLYWRAPPED_MARKED_1:%.*]] = begin_borrow [[X_MOVEONLYWRAPPED_MARKED]]
25+
// CHECK: [[BORROWED_X_MOVEONLYWRAPPED_MARKED_2:%.*]] = begin_borrow [[X_MOVEONLYWRAPPED_MARKED]]
26+
// CHECK: [[FUNC:%.*]] = function_ref @$sSi1poiyS2i_SitFZ : $@convention(method) (Int, Int, @thin Int.Type) -> Int // user: %15
27+
// CHECK: [[GUARANTEED_ESCAPED_X_1:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[BORROWED_X_MOVEONLYWRAPPED_MARKED_1]]
28+
// CHECK: [[GUARANTEED_ESCAPED_X_2:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[BORROWED_X_MOVEONLYWRAPPED_MARKED_2]]
29+
// CHECK: apply [[FUNC]]([[GUARANTEED_ESCAPED_X_1]], [[GUARANTEED_ESCAPED_X_2]], {{.*}})
30+
// CHECK: [[BORROWED_X_MOVEONLYWRAPPED_MARKED:%.*]] = begin_borrow [[X_MOVEONLYWRAPPED_MARKED]]
31+
// CHECK: [[GUARANTEED_ESCAPED_X:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[BORROWED_X_MOVEONLYWRAPPED_MARKED]]
32+
// CHECK: store [[GUARANTEED_ESCAPED_X]] to [trivial] {{%.*}} : $*Int
33+
// CHECK: } // end sil function '$s14noimplicitcopy8printIntyyF'
34+
func printInt() {
35+
@_noImplicitCopy let x: Int = 5
36+
print2(x + x)
37+
print("printInt: \(x)")
38+
}
39+
40+
// NOTE: MOW expands to MOVEONLYWRAPPED
41+
//
42+
// CHECK-LABEL: sil hidden [ossa] @$s14noimplicitcopy10printKlassyyF : $@convention(thin) () -> () {
43+
// CHECK: [[X:%.*]] = begin_borrow [lexical] {{%[0-9]+}} : $Klass
44+
// CHECK: [[X_COPY:%.*]] = copy_value [[X]]
45+
// CHECK: [[X_MOVEONLYWRAPPED:%.*]] = copyable_to_moveonlywrapper [[X_COPY]]
46+
// CHECK: [[X_MOVEONLYWRAPPED_MARKED:%.*]] = mark_must_check [no_implicit_copy] [[X_MOVEONLYWRAPPED]]
47+
// CHECK: [[BORROWED_X_MOVEONLYWRAPPED_MARKED:%.*]] = begin_borrow [[X_MOVEONLYWRAPPED_MARKED]]
48+
// CHECK: [[FUNC:%.*]] = class_method [[BORROWED_X_MOVEONLYWRAPPED_MARKED]]
49+
// CHECK: [[GUARANTEED_ESCAPED_X:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[BORROWED_X_MOVEONLYWRAPPED_MARKED]]
50+
// CHECK: apply [[FUNC]]([[GUARANTEED_ESCAPED_X]])
51+
//
52+
// CHECK: [[BORROWED_X_MOVEONLYWRAPPED_MARKED:%.*]] = begin_borrow [[X_MOVEONLYWRAPPED_MARKED]]
53+
// CHECK: [[FUNC:%.*]] = class_method [[BORROWED_X_MOVEONLYWRAPPED_MARKED]] :
54+
// CHECK: [[GUARANTEED_ESCAPED_X:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[BORROWED_X_MOVEONLYWRAPPED_MARKED]]
55+
// CHECK: apply [[FUNC]]([[GUARANTEED_ESCAPED_X]])
56+
//
57+
// CHECK: [[BORROWED_X_MOVEONLYWRAPPED_MARKED:%.*]] = begin_borrow [[X_MOVEONLYWRAPPED_MARKED]]
58+
// CHECK: [[GUARANTEED_ESCAPED_X:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[BORROWED_X_MOVEONLYWRAPPED_MARKED]]
59+
// CHECK: store_borrow [[GUARANTEED_ESCAPED_X]] to
60+
// CHECK: } // end sil function '$s14noimplicitcopy10printKlassyyF'
61+
func printKlass() {
62+
@_noImplicitCopy let x = Klass()
63+
x.increment()
64+
x.increment()
65+
print("printKlass: \(x)")
66+
print("printKlass: \(x.i)")
67+
}

‎test/SILOptimizer/move_only_type_eliminator.sil‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ struct TrivialPair {
2525

2626
sil @use_trivial : $@convention(thin) (Trivial) -> ()
2727

28-
class Klass {}
28+
class Klass {
29+
func foo()
30+
}
31+
2932
struct KlassPair {
3033
var lhs: Klass
3134
var rhs: Klass
@@ -540,9 +543,9 @@ bb0(%0 : @guaranteed $Klass):
540543
%f = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
541544
%0aa = copy_value %0 : $Klass
542545
%0a = copyable_to_moveonlywrapper %0aa : $Klass
546+
%notUsed = class_method %0a : $@moveOnly Klass, #Klass.foo : (Klass) -> () -> (), $@convention(method) (@guaranteed Klass) -> ()
543547
debug_value %0a : $@moveOnly Klass, let, name "foo"
544548
%2 = enum $@moveOnly FakeOptional<Klass>, #FakeOptional.some!enumelt, %0a : $@moveOnly Klass
545-
546549
switch_enum %2 : $@moveOnly FakeOptional<Klass>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
547550

548551
bb1(%2Payload : @owned $@moveOnly Klass):

0 commit comments

Comments
 (0)