Skip to content

Commit cd98a4f

Browse files
committed
[move-only] Change type lowering to ensure that we use {retain,release}_value instead of strong_{retain,release} for move only enum/structs
This ensures that when we hit IRGen, if we have a type with trivial contents, we emit nothing and if we have a type with non-trivial contents, we just emit recursive retain/release as appropriate. This is the last bug holding up the buffer view prototype from Guilliume.
1 parent e000162 commit cd98a4f

File tree

3 files changed

+687
-6
lines changed

3 files changed

+687
-6
lines changed

lib/SIL/IR/TypeLowering.cpp

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,6 +1459,110 @@ namespace {
14591459
}
14601460
};
14611461

1462+
/// A lowering for loadable but non-trivial struct types.
1463+
class MoveOnlyLoadableStructTypeLowering final
1464+
: public LoadableAggTypeLowering<MoveOnlyLoadableStructTypeLowering,
1465+
VarDecl *> {
1466+
using Super =
1467+
LoadableAggTypeLowering<MoveOnlyLoadableStructTypeLowering, VarDecl *>;
1468+
1469+
public:
1470+
MoveOnlyLoadableStructTypeLowering(CanType type,
1471+
RecursiveProperties properties,
1472+
TypeExpansionContext forExpansion)
1473+
: LoadableAggTypeLowering(type, properties, forExpansion) {}
1474+
1475+
SILValue emitRValueProject(SILBuilder &B, SILLocation loc,
1476+
SILValue structValue, VarDecl *field,
1477+
const TypeLowering &fieldLowering) const {
1478+
return B.createStructExtract(loc, structValue, field,
1479+
fieldLowering.getLoweredType());
1480+
}
1481+
1482+
void destructureAggregate(
1483+
SILBuilder &B, SILLocation loc, SILValue aggValue, bool skipTrivial,
1484+
function_ref<void(unsigned childIndex, SILValue childValue,
1485+
const TypeLowering &childLowering)>
1486+
visitor) const {
1487+
if (!B.hasOwnership())
1488+
return Super::destructureAggregate(B, loc, aggValue, skipTrivial,
1489+
visitor);
1490+
1491+
auto *dsi = B.createDestructureStruct(loc, aggValue);
1492+
for (auto pair : llvm::enumerate(dsi->getResults())) {
1493+
SILValue childValue = pair.value();
1494+
auto &childLowering =
1495+
B.getFunction().getTypeLowering(childValue->getType());
1496+
if (skipTrivial && childLowering.isTrivial())
1497+
continue;
1498+
visitor(pair.index(), childValue, childLowering);
1499+
}
1500+
}
1501+
1502+
SILValue rebuildAggregate(SILBuilder &B, SILLocation loc,
1503+
ArrayRef<SILValue> values) const override {
1504+
return B.createStruct(loc, getLoweredType(), values);
1505+
}
1506+
1507+
private:
1508+
void lowerChildren(TypeConverter &TC,
1509+
SmallVectorImpl<Child> &children) const override {
1510+
auto silTy = getLoweredType();
1511+
auto structDecl = silTy.getStructOrBoundGenericStruct();
1512+
assert(structDecl);
1513+
1514+
for (auto prop : structDecl->getStoredProperties()) {
1515+
SILType propTy = silTy.getFieldType(prop, TC, getExpansionContext());
1516+
auto &propTL = TC.getTypeLowering(propTy, getExpansionContext());
1517+
children.push_back(Child{prop, propTL});
1518+
}
1519+
}
1520+
};
1521+
1522+
/// A lowering for loadable but non-trivial enum types.
1523+
class MoveOnlyLoadableEnumTypeLowering final
1524+
: public NonTrivialLoadableTypeLowering {
1525+
public:
1526+
MoveOnlyLoadableEnumTypeLowering(CanType type,
1527+
RecursiveProperties properties,
1528+
TypeExpansionContext forExpansion)
1529+
: NonTrivialLoadableTypeLowering(SILType::getPrimitiveObjectType(type),
1530+
properties, IsNotReferenceCounted,
1531+
forExpansion) {}
1532+
1533+
SILValue emitCopyValue(SILBuilder &B, SILLocation loc,
1534+
SILValue value) const override {
1535+
if (B.getFunction().hasOwnership())
1536+
return B.createCopyValue(loc, value);
1537+
B.createRetainValue(loc, value, B.getDefaultAtomicity());
1538+
return value;
1539+
}
1540+
1541+
SILValue emitLoweredCopyValue(SILBuilder &B, SILLocation loc,
1542+
SILValue value,
1543+
TypeExpansionKind style) const override {
1544+
if (B.getFunction().hasOwnership())
1545+
return B.createCopyValue(loc, value);
1546+
B.createRetainValue(loc, value, B.getDefaultAtomicity());
1547+
return value;
1548+
}
1549+
1550+
void emitDestroyValue(SILBuilder &B, SILLocation loc,
1551+
SILValue value) const override {
1552+
if (B.getFunction().hasOwnership()) {
1553+
B.createDestroyValue(loc, value);
1554+
return;
1555+
}
1556+
B.createReleaseValue(loc, value, B.getDefaultAtomicity());
1557+
}
1558+
1559+
void emitLoweredDestroyValue(SILBuilder &B, SILLocation loc, SILValue value,
1560+
TypeExpansionKind style) const override {
1561+
// Enums, we never want to expand.
1562+
return emitDestroyValue(B, loc, value);
1563+
}
1564+
};
1565+
14621566
/// A type lowering for `@differentiable(_linear)` function types.
14631567
class LinearDifferentiableSILFunctionTypeLowering final
14641568
: public LoadableAggTypeLowering<
@@ -2146,7 +2250,8 @@ namespace {
21462250
properties.setNonTrivial();
21472251
if (properties.isAddressOnly())
21482252
return handleMoveOnlyAddressOnly(structType, properties);
2149-
return handleMoveOnlyReference(structType, properties);
2253+
return new (TC) MoveOnlyLoadableStructTypeLowering(
2254+
structType, properties, Expansion);
21502255
}
21512256

21522257
return handleAggregateByProperties<LoadableStructTypeLowering>(structType,
@@ -2223,7 +2328,8 @@ namespace {
22232328
properties.setNonTrivial();
22242329
if (properties.isAddressOnly())
22252330
return handleMoveOnlyAddressOnly(enumType, properties);
2226-
return handleMoveOnlyReference(enumType, properties);
2331+
return new (TC)
2332+
MoveOnlyLoadableEnumTypeLowering(enumType, properties, Expansion);
22272333
}
22282334

22292335
return handleAggregateByProperties<LoadableEnumTypeLowering>(enumType,

test/Interpreter/moveonly_bufferview.swift

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@
88

99
@_moveOnly
1010
public struct BufferView<T> {
11-
var ptr: UnsafeMutableBufferPointer<T>
11+
var ptr: UnsafeBufferPointer<T>
1212

1313
deinit {}
1414
}
1515

1616
extension BufferView : Sequence {
17-
public typealias Iterator = UnsafeMutableBufferPointer<T>.Iterator
18-
public typealias Element = UnsafeMutableBufferPointer<T>.Element
17+
public typealias Iterator = UnsafeBufferPointer<T>.Iterator
18+
public typealias Element = UnsafeBufferPointer<T>.Element
1919

2020
public func makeIterator() -> Self.Iterator {
2121
return ptr.makeIterator()
@@ -24,7 +24,7 @@ extension BufferView : Sequence {
2424

2525
extension Array {
2626
public mutating func withBufferView<U>(_ f: (BufferView<Element>) -> U) -> U {
27-
return withUnsafeMutableBufferPointer {
27+
return withUnsafeBufferPointer {
2828
return f(BufferView(ptr: $0))
2929
}
3030
}
@@ -42,8 +42,26 @@ func testBufferView(_ x: __owned [Int]) {
4242
}
4343
}
4444

45+
@inline(never)
46+
func getBool() -> Bool { return true }
47+
48+
func testConditionalBufferView(_ x: __owned [Int]) {
49+
(_move x).withUnsafeBufferPointer {
50+
let y = BufferView(ptr: $0)
51+
// CHECK: 4
52+
// CHECK: 5
53+
// CHECK: 6
54+
if getBool() {
55+
for z in y {
56+
print(z)
57+
}
58+
}
59+
}
60+
}
61+
4562
func main() {
4663
testBufferView([1,2,3])
64+
testConditionalBufferView([4,5,6])
4765
}
4866

4967
main()

0 commit comments

Comments
 (0)