Skip to content

Commit 6c66b8f

Browse files
zoecarverzoecarver
committed
Update let properties opts to OSSA
1 parent b4771c2 commit 6c66b8f

File tree

5 files changed

+147
-41
lines changed

5 files changed

+147
-41
lines changed

‎lib/SIL/SILVerifier.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4733,6 +4733,12 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
47334733
if (term->isFunctionExiting()) {
47344734
require(state.Stack.empty(),
47354735
"return with stack allocs that haven't been deallocated");
4736+
if (!state.ActiveOps.empty()) {
4737+
for (auto *i : state.ActiveOps) {
4738+
i->dump();
4739+
}
4740+
}
4741+
47364742
require(state.ActiveOps.empty(),
47374743
"return with operations still active");
47384744

‎lib/SILOptimizer/IPO/LetPropertiesOpts.cpp

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ void LetPropertiesOpt::optimizeLetPropertyAccess(VarDecl *Property,
204204

205205
// Look for any instructions accessing let properties.
206206
if (isa<RefElementAddrInst>(Load) || isa<StructElementAddrInst>(Load)
207-
|| isa<BeginAccessInst>(Load)) {
207+
|| isa<BeginAccessInst>(Load) || isa<BeginBorrowInst>(Load)) {
208208
auto proj = cast<SingleValueInstruction>(Load);
209209

210210
// Copy the initializer into the function
@@ -216,7 +216,7 @@ void LetPropertiesOpt::optimizeLetPropertyAccess(VarDecl *Property,
216216
++UI;
217217

218218
// A nested begin_access will be mapped as a separate "Load".
219-
if (isa<BeginAccessInst>(User))
219+
if (isa<BeginAccessInst>(User) || isa<BeginBorrowInst>(User))
220220
continue;
221221

222222
if (!canReplaceLoadSequence(User))
@@ -372,18 +372,6 @@ bool LetPropertiesOpt::isConstantLetProperty(VarDecl *Property) {
372372
LLVM_DEBUG(llvm::dbgs() << "Property '" << *Property
373373
<< "' has no unknown uses\n");
374374

375-
// Only properties of simple types can be optimized.
376-
377-
// FIXME: Expansion
378-
auto &TL = Module->Types.getTypeLowering(Property->getType(),
379-
TypeExpansionContext::minimal());
380-
if (!TL.isTrivial()) {
381-
LLVM_DEBUG(llvm::dbgs() << "Property '" << *Property
382-
<< "' is not of trivial type\n");
383-
SkipProcessing.insert(Property);
384-
return false;
385-
}
386-
387375
PotentialConstantLetProperty.insert(Property);
388376

389377
return true;
@@ -412,6 +400,10 @@ LetPropertiesOpt::analyzeInitValue(SILInstruction *I, VarDecl *Property) {
412400
&& "Store instruction should store into a proper let property");
413401
(void) Dest;
414402
value = SI->getSrc();
403+
} else if (auto *copyValue = dyn_cast<CopyValueInst>(I)) {
404+
value = copyValue->getOperand();
405+
} else if (auto *copyAddr = dyn_cast<CopyAddrInst>(I)) {
406+
value = copyAddr->getSrc();
415407
}
416408

417409
// Check if it's just a copy from another instance of the struct.
@@ -531,7 +523,8 @@ void LetPropertiesOpt::collectPropertyAccess(SILInstruction *I,
531523
llvm::dbgs() << "The instructions are:\n"; I->dumpInContext());
532524

533525
if (isa<RefElementAddrInst>(I) || isa<StructElementAddrInst>(I)
534-
|| isa<BeginAccessInst>(I)) {
526+
|| isa<BeginAccessInst>(I) || isa<CopyAddrInst>(I) ||
527+
isa<CopyValueInst>(I) || isa<BeginBorrowInst>(I)) {
535528
// Check if there is a store to this property.
536529
auto projection = cast<SingleValueInstruction>(I);
537530
for (auto Use : getNonDebugUses(projection)) {
@@ -541,9 +534,26 @@ void LetPropertiesOpt::collectPropertyAccess(SILInstruction *I,
541534

542535
// Each begin_access is analyzed as a separate property access. Do not
543536
// consider a begin_access a use of the current projection.
544-
if (isa<BeginAccessInst>(User))
537+
if (isa<BeginAccessInst>(User) || isa<BeginBorrowInst>(I))
545538
continue;
546539

540+
if (auto *copyAddr = dyn_cast<CopyAddrInst>(User)) {
541+
if (copyAddr->getSrc() != projection ||
542+
!analyzeInitValue(copyAddr, Property)) {
543+
SkipProcessing.insert(Property);
544+
return;
545+
}
546+
continue;
547+
}
548+
549+
if (auto *copyVal = dyn_cast<CopyValueInst>(User)) {
550+
if (copyVal != projection || !analyzeInitValue(copyVal, Property)) {
551+
SkipProcessing.insert(Property);
552+
return;
553+
}
554+
continue;
555+
}
556+
547557
if (auto *SI = dyn_cast<StoreInst>(User)) {
548558
// There is a store into this property.
549559
// Analyze the assigned value and check if it is a constant
@@ -582,8 +592,8 @@ void LetPropertiesOpt::run(SILModuleTransform *T) {
582592
// properties.
583593
bool NonRemovable = !F.shouldOptimize();
584594

585-
// FIXME: We should be able to handle ownership.
586-
NonRemovable &= !F.hasOwnership();
595+
// // FIXME: We should be able to handle ownership.
596+
// NonRemovable &= !F.hasOwnership();
587597

588598
for (auto &BB : F) {
589599
for (auto &I : BB)

‎lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -956,9 +956,9 @@ class SILMem2Reg : public SILFunctionTransform {
956956
void run() override {
957957
SILFunction *F = getFunction();
958958

959-
// FIXME: We should be able to support ownership.
960-
if (F->hasOwnership())
961-
return;
959+
// // FIXME: We should be able to support ownership.
960+
// if (F->hasOwnership())
961+
// return;
962962

963963
LLVM_DEBUG(llvm::dbgs() << "** Mem2Reg on function: " << F->getName()
964964
<< " **\n");

‎test/SILOptimizer/let_properties_opts.sil

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,20 @@ struct Point {
1212
var x: Int64
1313
var y: Int64
1414
}
15-
class HasCenter {
1615

16+
class HasCenter {
1717
let centerPoint: Point = Point(x: 0, y: 0)
18-
1918
public func getCenter() -> Int64
2019
}
2120

22-
sil hidden @$s19let_properties_opts5PointV1x1yACSi_SitcfC : $@convention(method) (Int64, Int64, @thin Point.Type) -> Point {
21+
sil hidden [ossa] @$s19let_properties_opts5PointV1x1yACSi_SitcfC : $@convention(method) (Int64, Int64, @thin Point.Type) -> Point {
2322
bb0(%0 : $Int64, %1 : $Int64, %2 : $@thin Point.Type):
2423
%3 = struct $Point (%0 : $Int64, %1 : $Int64)
2524
return %3 : $Point
2625
}
2726

2827
// variable initialization expression of HasCenter.centerPoint
29-
sil hidden [transparent] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvpfi : $@convention(thin) () -> Point {
28+
sil hidden [transparent] [ossa] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvpfi : $@convention(thin) () -> Point {
3029
bb0:
3130
%0 = integer_literal $Builtin.Int64, 0
3231
%1 = struct $Int64 (%0 : $Builtin.Int64)
@@ -35,43 +34,47 @@ bb0:
3534
}
3635

3736
// HasCenter.centerPoint.getter
38-
// CHECK-LABEL: sil hidden [transparent] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg : $@convention(method) (@guaranteed HasCenter) -> Point {
39-
// CHECK: bb0(%0 : $HasCenter):
37+
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg : $@convention(method) (@guaranteed HasCenter) -> Point {
38+
// CHECK: bb0(%0 : @guaranteed $HasCenter):
4039
// CHECK: [[LIT:%.*]] = integer_literal $Builtin.Int64, 0
4140
// CHECK: [[INT:%.*]] = struct $Int64 ([[LIT]] : $Builtin.Int64)
4241
// CHECK: [[PNT:%.*]] = struct $Point ([[INT]] : $Int64, [[INT]] : $Int64)
4342
// CHECK: return [[PNT]] : $Point
4443
// CHECK-LABEL: } // end sil function '$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg'
45-
sil hidden [transparent] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg : $@convention(method) (@guaranteed HasCenter) -> Point {
46-
bb0(%0 : $HasCenter):
44+
sil hidden [transparent] [ossa] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg : $@convention(method) (@guaranteed HasCenter) -> Point {
45+
bb0(%0 : @guaranteed $HasCenter):
4746
%1 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint
48-
%2 = load %1 : $*Point
47+
%2 = load [trivial] %1 : $*Point
4948
return %2 : $Point
5049
}
5150

5251
// HasCenter.getCenter()
53-
// CHECK-LABEL: sil hidden @$s19let_properties_opts9HasCenterC9getCenterSiyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
52+
// CHECK-LABEL: sil hidden [ossa] @$s19let_properties_opts9HasCenterC9getCenterSiyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
5453
// CHECK: [[LIT:%.*]] = integer_literal $Builtin.Int64, 0
5554
// CHECK: [[INT:%.*]] = struct $Int64 ([[LIT]] : $Builtin.Int64)
5655
// CHECK: [[PNT:%.*]] = struct $Point ([[INT]] : $Int64, [[INT]] : $Int64)
5756
// CHECK: [[X:%.*]] = struct_extract [[PNT]] : $Point, #Point.x
5857
// CHECK: return [[X]] : $Int64
5958
// CHECK-LABEL: } // end sil function '$s19let_properties_opts9HasCenterC9getCenterSiyF'
60-
sil hidden @$s19let_properties_opts9HasCenterC9getCenterSiyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
61-
bb0(%0 : $HasCenter):
59+
sil hidden [ossa] @$s19let_properties_opts9HasCenterC9getCenterSiyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
60+
bb0(%0 : @guaranteed $HasCenter):
6261
%1 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint
6362
%2 = struct_element_addr %1 : $*Point, #Point.x
64-
%3 = load %2 : $*Int64
63+
%3 = load [trivial] %2 : $*Int64
6564
return %3 : $Int64
6665
}
6766

6867
// HasCenter.init()
69-
sil hidden @$s19let_properties_opts9HasCenterCACycfc : $@convention(method) (@owned HasCenter) -> @owned HasCenter {
70-
bb0(%0 : $HasCenter):
68+
sil hidden [ossa] @$s19let_properties_opts9HasCenterCACycfc : $@convention(method) (@owned HasCenter) -> @owned HasCenter {
69+
bb0(%0 : @owned $HasCenter):
7170
%1 = integer_literal $Builtin.Int64, 0
72-
%2 = struct $Int64 (%1 : $Builtin.Int64)
73-
%3 = struct $Point (%2 : $Int64, %2 : $Int64)
74-
%4 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint
75-
store %3 to %4 : $*Point
76-
return %0 : $HasCenter
71+
%3 = struct $Int64 (%1 : $Builtin.Int64)
72+
%4 = struct $Point (%3 : $Int64, %3 : $Int64)
73+
%5 = begin_borrow %0 : $HasCenter
74+
%6 = ref_element_addr %5 : $HasCenter, #HasCenter.centerPoint
75+
store %4 to [trivial] %6 : $*Point
76+
end_borrow %5 : $HasCenter
77+
%9 = copy_value %0 : $HasCenter
78+
destroy_value %0 : $HasCenter
79+
return %9 : $HasCenter
7780
}

‎test/SILOptimizer/let_properties_opts_non_trivial.sil

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// RUN: %target-sil-opt -let-properties-opt -enable-sil-verify-all %s | %FileCheck %s
2+
3+
sil_stage canonical
4+
5+
import Builtin
6+
import Swift
7+
8+
class Point {
9+
let x: Int64 = 0
10+
let y: Int64 = 0
11+
}
12+
13+
class HasCenter {
14+
let centerPoint: Point = Point()
15+
public func getCenter() -> Int64
16+
}
17+
18+
// Point.init()
19+
sil hidden [ossa] @$s19let_properties_opts5PointCACycfc : $@convention(method) (@owned Point) -> @owned Point {
20+
bb0(%0 : @owned $Point):
21+
%1 = integer_literal $Builtin.Int64, 0
22+
%2 = struct $Int64 (%1 : $Builtin.Int64)
23+
24+
%3 = begin_borrow %0 : $Point
25+
%6 = ref_element_addr %3 : $Point, #Point.x
26+
store %2 to [trivial] %6 : $*Int64
27+
end_borrow %3 : $Point
28+
29+
%9 = begin_borrow %0 : $Point
30+
%12 = ref_element_addr %9 : $Point, #Point.y
31+
store %2 to [trivial] %12 : $*Int64
32+
end_borrow %9 : $Point
33+
34+
%15 = copy_value %0 : $Point
35+
destroy_value %0 : $Point
36+
return %15 : $Point
37+
}
38+
39+
// HasCenter.init()
40+
sil hidden [ossa] @$s19let_properties_opts9HasCenterCACycfc : $@convention(method) (@owned HasCenter) -> @owned HasCenter {
41+
// %0 // users: %2, %1
42+
bb0(%0 : @owned $HasCenter):
43+
%1 = alloc_ref $Point
44+
// function_ref Point.init()
45+
%2 = function_ref @$s19let_properties_opts5PointCACycfc : $@convention(method) (@owned Point) -> @owned Point
46+
%3 = apply %2(%1) : $@convention(method) (@owned Point) -> @owned Point
47+
48+
%5 = begin_borrow %0 : $HasCenter // users: %8, %6
49+
%6 = ref_element_addr %5 : $HasCenter, #HasCenter.centerPoint // user: %7
50+
51+
store %3 to [init] %6 : $*Point // id: %7
52+
end_borrow %5 : $HasCenter // id: %8
53+
54+
%9 = copy_value %0 : $HasCenter // user: %11
55+
destroy_value %0 : $HasCenter // id: %10
56+
return %9 : $HasCenter // id: %11
57+
} // end sil function '$s19let_properties_opts9HasCenterCACycfc'
58+
59+
// HasCenter.centerPoint.getter
60+
sil hidden [transparent] [ossa] @$s19let_properties_opts9HasCenterC11centerPointAA0G0Cvg : $@convention(method) (@guaranteed HasCenter) -> @owned Point {
61+
// %0 // users: %2, %1
62+
bb0(%0 : @guaranteed $HasCenter):
63+
debug_value %0 : $HasCenter, let, name "self", argno 1 // id: %1
64+
%2 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint // user: %3
65+
%3 = load [copy] %2 : $*Point // user: %4
66+
return %3 : $Point // id: %4
67+
} // end sil function '$s19let_properties_opts9HasCenterC11centerPointAA0G0Cvg'
68+
69+
// HasCenter.getCenter()
70+
// CHECK-LABEL: sil hidden [ossa] @$s19let_properties_opts9HasCenterC03getE0s5Int64VyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
71+
// CHECK: [[LIT:%.*]] = integer_literal $Builtin.Int64, 0
72+
// CHECK: [[INT:%.*]] = struct $Int64 ([[LIT]] : $Builtin.Int64)
73+
// CHECK: return [[INT]] : $Int64
74+
// CHECK-LABEL: } // end sil function '$s19let_properties_opts9HasCenterC03getE0s5Int64VyF'
75+
sil hidden [ossa] @$s19let_properties_opts9HasCenterC03getE0s5Int64VyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
76+
// %0 // users: %2, %1
77+
bb0(%0 : @guaranteed $HasCenter):
78+
debug_value %0 : $HasCenter, let, name "self", argno 1 // id: %1
79+
%2 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint // user: %3
80+
%3 = load [copy] %2 : $*Point // users: %8, %4
81+
%4 = begin_borrow %3 : $Point // users: %7, %5
82+
%5 = ref_element_addr %4 : $Point, #Point.x // user: %6
83+
%6 = load [trivial] %5 : $*Int64 // user: %9
84+
end_borrow %4 : $Point // id: %7
85+
destroy_value %3 : $Point // id: %8
86+
return %6 : $Int64 // id: %9
87+
} // end sil function '$s19let_properties_opts9HasCenterC03getE0s5Int64VyF'

0 commit comments

Comments
 (0)