Skip to content

Commit a7ca167

Browse files
swift-ciswift-ci
authored
Merge pull request swiftlang#34549 from meg-gupta/arrayelempropossa
2 parents f36f014 + 483321c commit a7ca167

File tree

7 files changed

+599
-127
lines changed

7 files changed

+599
-127
lines changed

‎include/swift/SILOptimizer/Utils/InstOptUtils.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ NullablePtr<SILInstruction> createIncrementBefore(SILValue ptr,
5656
NullablePtr<SILInstruction> createDecrementBefore(SILValue ptr,
5757
SILInstruction *insertpt);
5858

59+
/// Get the insertion point after \p val.
60+
Optional<SILBasicBlock::iterator> getInsertAfterPoint(SILValue val);
61+
5962
/// A utility for deleting one or more instructions belonging to a function, and
6063
/// cleaning up any dead code resulting from deleting those instructions. Use
6164
/// this utility instead of

‎lib/SILOptimizer/Analysis/ArraySemantic.cpp‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -701,30 +701,29 @@ bool swift::ArraySemanticsCall::replaceByValue(SILValue V) {
701701
if (!V->getType().isLoadable(*SemanticsCall->getFunction()))
702702
return false;
703703

704+
if (!hasGetElementDirectResult())
705+
return false;
706+
704707
// Expect a check_subscript call or the empty dependence.
705708
auto SubscriptCheck = getSubscriptCheckArgument();
706709
ArraySemanticsCall Check(SubscriptCheck, "array.check_subscript");
707710
auto *EmptyDep = dyn_cast<StructInst>(SubscriptCheck);
708711
if (!Check && (!EmptyDep || !EmptyDep->getElements().empty()))
709712
return false;
710713

711-
SILBuilderWithScope Builder(SemanticsCall);
712-
auto &ValLowering = Builder.getTypeLowering(V->getType());
713-
if (hasGetElementDirectResult()) {
714-
ValLowering.emitCopyValue(Builder, SemanticsCall->getLoc(), V);
715-
SemanticsCall->replaceAllUsesWith(V);
716-
} else {
717-
auto Dest = SemanticsCall->getArgument(0);
714+
// In OSSA, the InsertPt is after V's definition and not before SemanticsCall
715+
// Because we are creating copy_value in ossa, and the source may have been
716+
// taken previously. So our insert point for copy_value is immediately after
717+
// V, where we can be sure it is live.
718+
auto InsertPt = V->getFunction()->hasOwnership()
719+
? getInsertAfterPoint(V)
720+
: SemanticsCall->getIterator();
721+
assert(InsertPt.hasValue());
718722

719-
// Expect an alloc_stack initialization.
720-
auto *ASI = dyn_cast<AllocStackInst>(Dest);
721-
if (!ASI)
722-
return false;
723+
SILValue CopiedVal = SILBuilderWithScope(InsertPt.getValue())
724+
.emitCopyValueOperation(SemanticsCall->getLoc(), V);
725+
SemanticsCall->replaceAllUsesWith(CopiedVal);
723726

724-
ValLowering.emitCopyValue(Builder, SemanticsCall->getLoc(), V);
725-
ValLowering.emitStoreOfCopy(Builder, SemanticsCall->getLoc(), V, Dest,
726-
IsInitialization_t::IsInitialization);
727-
}
728727
removeCall();
729728
return true;
730729
}
@@ -777,9 +776,16 @@ bool swift::ArraySemanticsCall::replaceByAppendingValues(
777776
for (SILValue V : Vals) {
778777
auto SubTy = V->getType();
779778
auto &ValLowering = Builder.getTypeLowering(SubTy);
780-
auto CopiedVal = ValLowering.emitCopyValue(Builder, Loc, V);
779+
// In OSSA, the InsertPt is after V's definition and not before
780+
// SemanticsCall. Because we are creating copy_value in ossa, and the source
781+
// may have been taken previously. So our insert point for copy_value is
782+
// immediately after V, where we can be sure it is live.
783+
auto InsertPt = F->hasOwnership() ? getInsertAfterPoint(V)
784+
: SemanticsCall->getIterator();
785+
assert(InsertPt.hasValue());
786+
SILValue CopiedVal = SILBuilderWithScope(InsertPt.getValue())
787+
.emitCopyValueOperation(V.getLoc(), V);
781788
auto *AllocStackInst = Builder.createAllocStack(Loc, SubTy);
782-
783789
ValLowering.emitStoreOfCopy(Builder, Loc, CopiedVal, AllocStackInst,
784790
IsInitialization_t::IsInitialization);
785791

@@ -795,8 +801,7 @@ bool swift::ArraySemanticsCall::replaceByAppendingValues(
795801
if (AppendContentsOfFnTy->getParameters()[0].getConvention() ==
796802
ParameterConvention::Direct_Owned) {
797803
SILValue SrcArray = SemanticsCall->getArgument(0);
798-
Builder.createReleaseValue(SemanticsCall->getLoc(), SrcArray,
799-
Builder.getDefaultAtomicity());
804+
Builder.emitDestroyValueOperation(SemanticsCall->getLoc(), SrcArray);
800805
}
801806

802807
removeCall();

‎lib/SILOptimizer/Transforms/ArrayElementValuePropagation.cpp‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,22 @@ bool ArrayAllocation::recursivelyCollectUses(ValueBase *Def) {
125125
for (auto *Opd : Def->getUses()) {
126126
auto *User = Opd->getUser();
127127
// Ignore reference counting and debug instructions.
128-
if (isa<RefCountingInst>(User) ||
129-
isa<DebugValueInst>(User))
128+
if (isa<RefCountingInst>(User) || isa<DebugValueInst>(User) ||
129+
isa<DestroyValueInst>(User) || isa<EndBorrowInst>(User))
130130
continue;
131131

132+
if (auto *CVI = dyn_cast<CopyValueInst>(User)) {
133+
if (!recursivelyCollectUses(CVI))
134+
return false;
135+
continue;
136+
}
137+
138+
if (auto *BBI = dyn_cast<BeginBorrowInst>(User)) {
139+
if (!recursivelyCollectUses(BBI))
140+
return false;
141+
continue;
142+
}
143+
132144
// Array value projection.
133145
if (auto *SEI = dyn_cast<StructExtractInst>(User)) {
134146
if (!recursivelyCollectUses(SEI))
@@ -312,11 +324,6 @@ class ArrayElementPropagation : public SILFunctionTransform {
312324

313325
void run() override {
314326
auto &Fn = *getFunction();
315-
316-
// FIXME: Update for ownership.
317-
if (Fn.hasOwnership())
318-
return;
319-
320327
bool Changed = false;
321328

322329
for (auto &BB :Fn) {
@@ -326,7 +333,7 @@ class ArrayElementPropagation : public SILFunctionTransform {
326333
if (!ALit.analyze(Apply))
327334
continue;
328335

329-
// First optimization: replace getElemente calls.
336+
// First optimization: replace getElement calls.
330337
if (ALit.replaceGetElements()) {
331338
Changed = true;
332339
// Re-do the analysis if the SIL changed.

‎lib/SILOptimizer/Utils/InstOptUtils.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ const std::function<void(SingleValueInstruction *, SILValue)>
6969
i->eraseFromParent();
7070
};
7171

72+
Optional<SILBasicBlock::iterator> swift::getInsertAfterPoint(SILValue val) {
73+
if (isa<SingleValueInstruction>(val)) {
74+
return std::next(cast<SingleValueInstruction>(val)->getIterator());
75+
}
76+
if (isa<SILArgument>(val)) {
77+
return cast<SILArgument>(val)->getParentBlock()->begin();
78+
}
79+
return None;
80+
}
81+
7282
/// Creates an increment on \p Ptr before insertion point \p InsertPt that
7383
/// creates a strong_retain if \p Ptr has reference semantics itself or a
7484
/// retain_value if \p Ptr is a non-trivial value without reference-semantics.

‎test/SILOptimizer/array_element_propagation.sil‎

Lines changed: 10 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -29,106 +29,13 @@ sil @swift_bufferAllocate : $@convention(thin)() -> @owned AnyObject
2929
sil [_semantics "array.uninitialized"] @adoptStorage : $@convention(thin) (@owned AnyObject, MyInt, @thin MyArray<MyInt>.Type) -> @owned (MyArray<MyInt>, UnsafeMutablePointer<MyInt>)
3030
sil [_semantics "array.props.isNativeTypeChecked"] @hoistableIsNativeTypeChecked : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
3131
sil [_semantics "array.check_subscript"] @checkSubscript : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
32-
sil [_semantics "array.get_element"] @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
33-
sil [_semantics "array.get_element"] @getElement2 : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
32+
sil [_semantics "array.get_element"] @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
3433
sil @unknown_array_use : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
3534
sil [_semantics "array.uninitialized"] @arrayAdoptStorage : $@convention(thin) (@owned AnyObject, MyInt, @thin Array<MyInt>.Type) -> @owned (Array<MyInt>, UnsafeMutablePointer<MyInt>)
3635
sil @arrayInit : $@convention(method) (@thin Array<MyInt>.Type) -> @owned Array<MyInt>
3736
sil [_semantics "array.finalize_intrinsic"] @finalize : $@convention(thin) (@owned MyArray<MyInt>) -> @owned MyArray<MyInt>
3837
sil [_semantics "array.append_contentsOf"] @arrayAppendContentsOf : $@convention(method) (@owned Array<MyInt>, @inout Array<MyInt>) -> ()
3938

40-
// CHECK-LABEL: sil @propagate01
41-
// CHECK: struct $MyInt
42-
// CHECK: [[V0:%.*]] = integer_literal $Builtin.Int64, 0
43-
// CHECK: [[I0:%.*]] = struct $MyInt ([[V0]] : $Builtin.Int64)
44-
// CHECK: [[V1:%.*]] = integer_literal $Builtin.Int64, 1
45-
// CHECK: [[I1:%.*]] = struct $MyInt ([[V1]] : $Builtin.Int64)
46-
// CHECK: [[V2:%.*]] = integer_literal $Builtin.Int64, 2
47-
// CHECK: [[I2:%.*]] = struct $MyInt ([[V2]] : $Builtin.Int64)
48-
// CHECK: [[S0:%.*]] = alloc_stack $MyInt
49-
// CHECK: [[HFUN:%.*]] = function_ref @hoistableIsNativeTypeChecked
50-
// CHECK-NOT: apply [[HFUN]]
51-
// CHECK: [[CFUN:%.*]] = function_ref @checkSubscript
52-
// CHECK-NOT: apply [[CFUN]]
53-
// CHECK: [[GFUN:%.*]] = function_ref @getElement
54-
// CHECK-NOT: apply [[GFUN]]
55-
// CHECK-NOT: apply [[HFUN]]
56-
// CHECK-NOT: apply [[CFUN]]
57-
// CHECK-NOT: apply [[GFUN]]
58-
// CHECK: store [[I0]] to [[S0]]
59-
// CHECK: [[S1:%.*]] = alloc_stack $MyInt
60-
// CHECK: store [[I1]] to [[S1]]
61-
// CHECK: [[S2:%.*]] = alloc_stack $MyInt
62-
// CHECK: store [[I2]] to [[S2]]
63-
// CHECK: return
64-
65-
sil @propagate01 : $@convention(thin) () -> () {
66-
%0 = function_ref @swift_bufferAllocate : $@convention(thin) () -> @owned AnyObject
67-
%1 = integer_literal $Builtin.Int64, 3
68-
%2 = struct $MyInt (%1 : $Builtin.Int64)
69-
%3 = apply %0() : $@convention(thin) () -> @owned AnyObject
70-
%4 = metatype $@thin MyArray<MyInt>.Type
71-
%5 = function_ref @adoptStorage : $@convention(thin) (@owned AnyObject, MyInt, @thin MyArray<MyInt>.Type) -> @owned (MyArray<MyInt>, UnsafeMutablePointer<MyInt>)
72-
%6 = apply %5(%3, %2, %4) : $@convention(thin) (@owned AnyObject, MyInt, @thin MyArray<MyInt>.Type) -> @owned (MyArray<MyInt>, UnsafeMutablePointer<MyInt>)
73-
%7 = tuple_extract %6 : $(MyArray<MyInt>, UnsafeMutablePointer<MyInt>), 0
74-
%8 = tuple_extract %6 : $(MyArray<MyInt>, UnsafeMutablePointer<MyInt>), 1
75-
debug_value %7 : $MyArray<MyInt>
76-
debug_value %8 : $UnsafeMutablePointer<MyInt>
77-
%9 = struct_extract %8 : $UnsafeMutablePointer<MyInt>, #UnsafeMutablePointer._rawValue
78-
%10 = pointer_to_address %9 : $Builtin.RawPointer to [strict] $*MyInt
79-
%11 = integer_literal $Builtin.Int64, 0
80-
%12 = struct $MyInt (%11 : $Builtin.Int64)
81-
store %12 to %10 : $*MyInt
82-
%13 = integer_literal $Builtin.Word, 1
83-
%14 = index_addr %10 : $*MyInt, %13 : $Builtin.Word
84-
%15 = integer_literal $Builtin.Int64, 1
85-
%16 = struct $MyInt (%15 : $Builtin.Int64)
86-
store %16 to %14 : $*MyInt
87-
%17 = integer_literal $Builtin.Word, 2
88-
%18 = index_addr %10 : $*MyInt, %17 : $Builtin.Word
89-
%19 = integer_literal $Builtin.Int64, 2
90-
%20 = struct $MyInt (%19 : $Builtin.Int64)
91-
store %20 to %18 : $*MyInt
92-
%f = function_ref @finalize : $@convention(thin) (@owned MyArray<MyInt>) -> @owned MyArray<MyInt>
93-
%a = apply %f(%7) : $@convention(thin) (@owned MyArray<MyInt>) -> @owned MyArray<MyInt>
94-
%23 = struct_extract %a : $MyArray<MyInt>, #MyArray._buffer
95-
%24 = struct_extract %23 : $_MyArrayBuffer<MyInt>, #_MyArrayBuffer._storage
96-
%25 = struct_extract %24 : $_MyBridgeStorage, #_MyBridgeStorage.rawValue
97-
%26 = alloc_stack $MyInt
98-
debug_value %a : $MyArray<MyInt>
99-
%27 = function_ref @hoistableIsNativeTypeChecked : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
100-
%28 = apply %27(%a) : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
101-
debug_value %28 : $MyBool // id: %104
102-
%29 = function_ref @checkSubscript : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
103-
%30 = apply %29(%12, %28, %a) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
104-
debug_value %30 : $_MyDependenceToken
105-
%31 = function_ref @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
106-
%32 = apply %31(%26, %12, %28, %30, %a) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
107-
%35 = alloc_stack $MyInt
108-
debug_value %16 : $MyInt
109-
debug_value %a : $MyArray<MyInt>
110-
debug_value %28 : $MyBool
111-
strong_retain %25 : $Builtin.BridgeObject
112-
%36 = apply %29(%16, %28, %a) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
113-
debug_value %36 : $_MyDependenceToken
114-
%37 = apply %31(%35, %16, %28, %36, %a) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
115-
strong_release %25 : $Builtin.BridgeObject
116-
%44 = alloc_stack $MyInt
117-
debug_value %a : $MyArray<MyInt>
118-
debug_value %28 : $MyBool
119-
strong_retain %25 : $Builtin.BridgeObject
120-
%45 = apply %29(%20, %28, %a) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
121-
debug_value %45 : $_MyDependenceToken
122-
%46 = apply %31(%44, %20, %28, %45, %a) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
123-
strong_release %25 : $Builtin.BridgeObject
124-
%52 = tuple ()
125-
dealloc_stack %44 : $*MyInt
126-
dealloc_stack %35 : $*MyInt
127-
dealloc_stack %26 : $*MyInt
128-
strong_release %25 : $Builtin.BridgeObject
129-
return %52 : $()
130-
}
131-
13239
// CHECK-LABEL: sil @propagate_with_get_element_returning_direct_result
13340
// CHECK: struct $MyInt
13441
// CHECK: [[V0:%.*]] = integer_literal $Builtin.Int64, 0
@@ -192,7 +99,7 @@ sil @propagate_with_get_element_returning_direct_result : $@convention(thin) ()
19299
%29 = function_ref @checkSubscript : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
193100
%30 = apply %29(%12, %28, %7) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
194101
debug_value %30 : $_MyDependenceToken
195-
%31 = function_ref @getElement2 : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
102+
%31 = function_ref @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
196103
%32 = apply %31(%12, %28, %30, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
197104
store %32 to %26 : $*MyInt
198105
%35 = alloc_stack $MyInt
@@ -256,12 +163,14 @@ sil @repeated_initialization : $@convention(thin) () -> () {
256163
%28 = apply %27(%7) : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
257164
%29 = function_ref @checkSubscript : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
258165
%30 = apply %29(%12, %28, %7) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
259-
%31 = function_ref @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
260-
%32 = apply %31(%26, %12, %28, %30, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
166+
%31 = function_ref @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
167+
%32 = apply %31(%12, %28, %30, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
168+
store %32 to %26 : $*MyInt
261169
%35 = alloc_stack $MyInt
262170
strong_retain %25 : $Builtin.BridgeObject
263171
%36 = apply %29(%16, %28, %7) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
264-
%37 = apply %31(%35, %16, %28, %36, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
172+
%37 = apply %31(%16, %28, %36, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
173+
store %37 to %35 : $*MyInt
265174
strong_release %25 : $Builtin.BridgeObject
266175
%52 = tuple ()
267176
dealloc_stack %35 : $*MyInt
@@ -298,8 +207,9 @@ sil @unknown_use : $@convention(thin) () -> () {
298207
%28 = apply %27(%7) : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
299208
%29 = function_ref @checkSubscript : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
300209
%30 = apply %29(%12, %28, %7) : $@convention(method) (MyInt, MyBool, @guaranteed MyArray<MyInt>) -> _MyDependenceToken
301-
%31 = function_ref @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
302-
%32 = apply %31(%26, %12, %28, %30, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> @out MyInt
210+
%31 = function_ref @getElement : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
211+
%32 = apply %31(%12, %28, %30, %7) : $@convention(method) (MyInt, MyBool, _MyDependenceToken, @guaranteed MyArray<MyInt>) -> MyInt
212+
store %32 to %26 : $*MyInt
303213
%33 = function_ref @unknown_array_use : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
304214
%34 = apply %33(%7) : $@convention(method) (@guaranteed MyArray<MyInt>) -> MyBool
305215
%52 = tuple ()

0 commit comments

Comments
 (0)