Skip to content

Commit da4f27e

Browse files
committed
ManualOwnership: adjust CopyExpr emission
We were getting an address-based emission for loadable types, if the result of a LoadExpr was explicit-copied. This emission had extra hidden copies that could not be silenced.
1 parent c20ed75 commit da4f27e

File tree

3 files changed

+68
-7
lines changed

3 files changed

+68
-7
lines changed

lib/SILGen/SILGenExpr.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7279,6 +7279,19 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) {
72797279

72807280
if (auto *li = dyn_cast<LoadExpr>(subExpr)) {
72817281
FormalEvaluationScope writeback(SGF);
7282+
7283+
// If we're relying on ManualOwnership for explicit-copies enforcement,
7284+
// avoid doing address-based emission for loadable types.
7285+
if (subType.isLoadableOrOpaque(SGF.F) &&
7286+
SGF.F.getPerfConstraints() == PerformanceConstraints::ManualOwnership) {
7287+
// Interpret this 'load' as a borrow + copy instead.
7288+
LValue lv =
7289+
SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedObjectRead);
7290+
auto value = SGF.emitBorrowedLValue(li, std::move(lv));
7291+
ManagedValue copy = SGF.B.createExplicitCopyValue(E, value);
7292+
return RValue(SGF, {copy}, subType.getASTType());
7293+
}
7294+
72827295
LValue lv =
72837296
SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedAddressRead);
72847297
auto address = SGF.emitAddressOfLValue(subExpr, std::move(lv));

test/SIL/manual_ownership.swift

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,8 @@ public func basic_loop_trivial_values_fixed(_ t: Triangle, _ xs: [Triangle]) {
172172
// FIXME: the only reason for so many copies below is because
173173
// `Triangle.nontrivial` only exposes get/set rather than read/modify by default
174174
//
175-
// We should figure out when the coroutine accessors are generated, and ensure
176-
// that when it is available, it is used without copying the result, rather than
177-
// calling the get/set
175+
// There's complexity in auto-generating a read accessor for classes, but if it's provided
176+
// we could then allow someone to elide the copy with a `borrow x` expression.
178177

179178
@_manualOwnership
180179
public func basic_loop_nontrivial_values(_ t: Triangle, _ xs: [Triangle]) {
@@ -185,14 +184,35 @@ public func basic_loop_nontrivial_values(_ t: Triangle, _ xs: [Triangle]) {
185184
t.nontrivial.a = p // expected-error {{accessing 't.nontrivial' produces a copy of it}}
186185
}
187186

188-
// FIXME: there should be no copies required in the below, other than what's already written.
189187
@_manualOwnership
190188
public func basic_loop_nontrivial_values_fixed(_ t: Triangle, _ xs: [Triangle]) {
191-
var p: Pair = (copy t.nontrivial).a // expected-error {{accessing 't.nontrivial' produces a copy of it}}
189+
var p: Pair = (copy t.nontrivial).a
192190
for x in copy xs {
193-
p = p.midpoint((copy x.nontrivial).a) // expected-error {{accessing 'x.nontrivial' produces a copy of it}}
191+
p = p.midpoint((copy x.nontrivial).a)
194192
}
195-
(copy t.nontrivial).a = p // expected-error {{accessing 't.nontrivial' produces a copy of it}}
193+
(copy t.nontrivial).a = p
194+
}
195+
196+
@_manualOwnership
197+
public func basic_loop_nontrivial_values_reduced_copies(_ t: Triangle, _ xs: [Triangle]) {
198+
// FIXME: confusing variable names are chosen
199+
let nt = t.nontrivial // expected-error {{accessing 'nt' produces a copy of it}}
200+
var p: Pair = nt.a
201+
for x in copy xs {
202+
let xnt = x.nontrivial // expected-error {{accessing 'xnt' produces a copy of it}}
203+
p = p.midpoint(xnt.a)
204+
}
205+
nt.a = p
206+
}
207+
@_manualOwnership
208+
public func basic_loop_nontrivial_values_reduced_copies_fixed(_ t: Triangle, _ xs: [Triangle]) {
209+
let nt = copy t.nontrivial
210+
var p: Pair = nt.a
211+
for x in copy xs {
212+
let xnt = copy x.nontrivial
213+
p = p.midpoint(xnt.a)
214+
}
215+
nt.a = p
196216
}
197217

198218

test/SILGen/manual_ownership.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-swift-frontend %s -emit-silgen -verify \
2+
// RUN: -enable-experimental-feature ManualOwnership -o %t.silgen
3+
4+
// RUN: %FileCheck %s --input-file %t.silgen
5+
6+
// REQUIRES: swift_feature_ManualOwnership
7+
8+
class ShapeClass {}
9+
class TriangleClass {
10+
var shape = ShapeClass()
11+
}
12+
13+
// CHECK-LABEL: sil {{.*}} @basic_access_of_loadable
14+
// CHECK: bb0(%0 : @guaranteed $TriangleClass):
15+
// CHECK-NEXT: debug_value %0
16+
// CHECK-NEXT: [[M:%.*]] = class_method %0, #TriangleClass.shape!getter
17+
// CHECK-NEXT: [[SHAPE:%.*]] = apply [[M]](%0)
18+
// CHECK-NEXT: [[B:%.*]] = begin_borrow [[SHAPE]]
19+
// CHECK-NEXT: [[COPY:%.*]] = explicit_copy_value [[B]]
20+
// CHECK-NEXT: end_borrow [[B]]
21+
// CHECK-NEXT: destroy_value [[SHAPE]]
22+
// CHECK-NEXT: return [[COPY]]
23+
// CHECK-NEXT: } // end sil function 'basic_access_of_loadable'
24+
@_manualOwnership
25+
@_silgen_name("basic_access_of_loadable")
26+
func basic_access_of_loadable(_ t: TriangleClass) -> ShapeClass {
27+
return copy t.shape
28+
}

0 commit comments

Comments
 (0)