Skip to content

Commit 2b0e101

Browse files
meg-guptameg-gupta
committed
Fix OSSA Outliner when guaranteed values are scoped and end before their new uses in the outlined call
1 parent 2585a5b commit 2b0e101

File tree

2 files changed

+193
-10
lines changed

2 files changed

+193
-10
lines changed

‎lib/SILOptimizer/Transforms/Outliner.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,19 @@
1919
#include "swift/Demangling/Demangler.h"
2020
#include "swift/Demangling/ManglingMacros.h"
2121
#include "swift/SIL/ApplySite.h"
22+
#include "swift/SIL/BasicBlockDatastructures.h"
23+
#include "swift/SIL/BasicBlockUtils.h"
2224
#include "swift/SIL/DebugUtils.h"
2325
#include "swift/SIL/DynamicCasts.h"
2426
#include "swift/SIL/SILArgument.h"
2527
#include "swift/SIL/SILBuilder.h"
2628
#include "swift/SIL/SILFunction.h"
2729
#include "swift/SIL/SILInstruction.h"
2830
#include "swift/SIL/SILModule.h"
29-
#include "swift/SIL/BasicBlockDatastructures.h"
3031
#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
3132
#include "swift/SILOptimizer/PassManager/Passes.h"
3233
#include "swift/SILOptimizer/PassManager/Transforms.h"
34+
#include "swift/SILOptimizer/Utils/OwnershipOptUtils.h"
3335
#include "swift/SILOptimizer/Utils/SILOptFunctionBuilder.h"
3436
#include "llvm/ADT/BitVector.h"
3537
#include "llvm/Support/CommandLine.h"
@@ -958,7 +960,7 @@ void BridgedReturn::outline(SILFunction *Fun, ApplyInst *NewOutlinedCall) {
958960
if (deBlocks) {
959961
deBlocks->updateForNewBlock(NewTailBB);
960962
}
961-
auto Loc = switchInfo.SwitchEnum->getLoc();
963+
auto Loc = switchInfo.SwitchEnum->getLoc();
962964

963965
{
964966
SILBuilder Builder(StartBB);
@@ -1062,7 +1064,12 @@ ObjCMethodCall::outline(SILModule &M) {
10621064
for (auto Arg : BridgedCall->getArguments()) {
10631065
if (BridgedArgIdx < BridgedArguments.size() &&
10641066
BridgedArguments[BridgedArgIdx].Idx == OrigSigIdx) {
1065-
Args.push_back(BridgedArguments[BridgedArgIdx].bridgedValue());
1067+
auto bridgedArgValue = BridgedArguments[BridgedArgIdx].bridgedValue();
1068+
if (bridgedArgValue.getOwnershipKind() == OwnershipKind::Guaranteed) {
1069+
bridgedArgValue = makeGuaranteedValueAvailable(
1070+
bridgedArgValue, BridgedCall, *deBlocks);
1071+
}
1072+
Args.push_back(bridgedArgValue);
10661073
++BridgedArgIdx;
10671074
} else {
10681075
// Otherwise, use the original type convention.
@@ -1299,8 +1306,8 @@ class OutlinePatterns {
12991306
/// functions.
13001307
bool tryOutline(SILOptFunctionBuilder &FuncBuilder, SILFunction *Fun,
13011308
SmallVectorImpl<SILFunction *> &FunctionsAdded,
1302-
InstModCallbacks callbacks,
1303-
DeadEndBlocks *deBlocks) {
1309+
InstModCallbacks callbacks = InstModCallbacks(),
1310+
DeadEndBlocks *deBlocks = nullptr) {
13041311
BasicBlockWorklist Worklist(Fun->getEntryBlock());
13051312
OutlinePatterns patterns(FuncBuilder, callbacks, deBlocks);
13061313
bool changed = false;
@@ -1346,7 +1353,6 @@ class Outliner : public SILFunctionTransform {
13461353
if (!Fun->optimizeForSize())
13471354
return;
13481355

1349-
13501356
// Dump function if requested.
13511357
if (DumpFuncsBeforeOutliner.size() &&
13521358
Fun->getName().contains(DumpFuncsBeforeOutliner)) {
@@ -1363,7 +1369,8 @@ class Outliner : public SILFunctionTransform {
13631369
bool Changed = false;
13641370

13651371
if (Fun->hasOwnership()) {
1366-
Changed = tryOutline(FuncBuilder, Fun, FunctionsAdded, callbacks, deBlocks);
1372+
Changed =
1373+
tryOutline(FuncBuilder, Fun, FunctionsAdded, callbacks, deBlocks);
13671374
} else {
13681375
Changed = tryOutline(FuncBuilder, Fun, FunctionsAdded);
13691376
}

‎test/SILOptimizer/outliner_ossa.sil

Lines changed: 179 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,21 @@ import Foundation
1111

1212
@objc class MyObject {
1313
@objc static func take(arg: Data?) -> Data?
14+
@objc static func take_two(arg1: Data?, arg2: Data?) -> Data?
15+
}
16+
17+
struct DataWrapper {
18+
let data: Data
1419
}
1520

1621
sil @getData : $@convention(thin) () -> @owned Data
22+
sil @getDataWrapper : $@convention(thin) () -> @owned DataWrapper
1723
sil @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
1824
sil @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
1925

2026
// We used to have a use-after release failure.
2127

22-
// CHECK-LABEL: sil [Osize] [ossa] @test : $@convention(thin) (@owned MyObject) -> () {
28+
// CHECK-LABEL: sil [Osize] [ossa] @test1 : $@convention(thin) (@owned MyObject) -> () {
2329
// CHECK: bb0([[ARG:%.*]] : @owned $MyObject):
2430
// CHECK: [[META:%.*]] = metatype $@objc_metatype MyObject.Type
2531
// CHECK: [[FUN1:%.*]] = function_ref @getData : $@convention(thin) () -> @owned Data
@@ -33,8 +39,8 @@ sil @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ
3339
// CHECK: destroy_value [[RES]]
3440
// CHECK: [[T:%.*]] = tuple ()
3541
// CHECK: return [[T]] : $()
36-
// CHECK: } // end sil function 'test'
37-
sil [Osize] [ossa] @test : $@convention(thin) (@owned MyObject) -> () {
42+
// CHECK: } // end sil function 'test1'
43+
sil [Osize] [ossa] @test1 : $@convention(thin) (@owned MyObject) -> () {
3844
bb0(%0: @owned $MyObject):
3945
%35 = metatype $@objc_metatype MyObject.Type
4046
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
@@ -68,6 +74,132 @@ bb7(%64 : @owned $Optional<Data>):
6874
return %102 : $()
6975
}
7076

77+
// CHECK-LABEL: sil [Osize] [ossa] @test2 :
78+
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
79+
// CHECK: apply [[FUNC]]
80+
// CHECK-LABEL: } // end sil function 'test2'
81+
sil [Osize] [ossa] @test2 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
82+
bb0(%0: @owned $MyObject):
83+
%35 = metatype $@objc_metatype MyObject.Type
84+
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
85+
%43 = apply %41() : $@convention(thin) () -> @owned Data
86+
%b = begin_borrow %43 : $Data
87+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
88+
%45 = apply %44(%b) : $@convention(method) (@guaranteed Data) -> @owned NSData
89+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
90+
end_borrow %b : $Data
91+
destroy_value %0 : $MyObject
92+
destroy_value %43 : $Data
93+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
94+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
95+
destroy_value %46 : $Optional<NSData>
96+
return %51 : $Optional<NSData>
97+
}
98+
99+
// CHECK-LABEL: sil [Osize] [ossa] @test3 :
100+
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
101+
// CHECK: apply [[FUNC]]
102+
// CHECK-LABEL: } // end sil function 'test3'
103+
sil [Osize] [ossa] @test3 : $@convention(thin) (@owned MyObject, @in Data) -> @owned Optional<NSData> {
104+
bb0(%0: @owned $MyObject, %1 : $*Data):
105+
%35 = metatype $@objc_metatype MyObject.Type
106+
%b = load_borrow %1 : $*Data
107+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
108+
%45 = apply %44(%b) : $@convention(method) (@guaranteed Data) -> @owned NSData
109+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
110+
end_borrow %b : $Data
111+
destroy_value %0 : $MyObject
112+
destroy_addr %1 : $*Data
113+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
114+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
115+
destroy_value %46 : $Optional<NSData>
116+
return %51 : $Optional<NSData>
117+
}
118+
119+
// CHECK-LABEL: sil [Osize] [ossa] @test4 :
120+
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC8take_two4arg14arg210Foundation4DataVSgAJ_AJtFZToTembgbgnn_ :
121+
// CHECK: apply [[FUNC]]
122+
// CHECK-LABEL: } // end sil function 'test4'
123+
sil [Osize] [ossa] @test4 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
124+
bb0(%0: @owned $MyObject):
125+
%35 = metatype $@objc_metatype MyObject.Type
126+
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
127+
%43 = apply %41() : $@convention(thin) () -> @owned Data
128+
%b1 = begin_borrow %43 : $Data
129+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
130+
%45 = apply %44(%b1) : $@convention(method) (@guaranteed Data) -> @owned NSData
131+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
132+
end_borrow %b1 : $Data
133+
%41a = function_ref @getData : $@convention(thin) () -> @owned Data
134+
%43a = apply %41a() : $@convention(thin) () -> @owned Data
135+
%b2 = begin_borrow %43a : $Data
136+
%44a = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
137+
%45a = apply %44a(%b2) : $@convention(method) (@guaranteed Data) -> @owned NSData
138+
%46a = enum $Optional<NSData>, #Optional.some!enumelt, %45a : $NSData
139+
end_borrow %b2 : $Data
140+
destroy_value %0 : $MyObject
141+
destroy_value %43 : $Data
142+
destroy_value %43a : $Data
143+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take_two!foreign : (MyObject.Type) -> (Data?, Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
144+
%51 = apply %50(%46, %46a, %35) : $@convention(objc_method) (Optional<NSData>, Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
145+
destroy_value %46 : $Optional<NSData>
146+
destroy_value %46a : $Optional<NSData>
147+
return %51 : $Optional<NSData>
148+
}
149+
150+
// This is not optimized currently
151+
sil [Osize] [ossa] @test5 : $@convention(thin) (@owned MyObject, @in Data, @in Data) -> @owned Optional<NSData> {
152+
bb0(%0: @owned $MyObject, %1 : $*Data, %2 : $*Data):
153+
%35 = metatype $@objc_metatype MyObject.Type
154+
%b = load_borrow %1 : $*Data
155+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
156+
%45 = apply %44(%b) : $@convention(method) (@guaranteed Data) -> @owned NSData
157+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
158+
end_borrow %b : $Data
159+
destroy_addr %1 : $*Data
160+
br bb1
161+
162+
bb1:
163+
%b1 = load_borrow %2 : $*Data
164+
%44a = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
165+
%45a = apply %44a(%b1) : $@convention(method) (@guaranteed Data) -> @owned NSData
166+
%46a = enum $Optional<NSData>, #Optional.some!enumelt, %45a : $NSData
167+
end_borrow %b1 : $Data
168+
destroy_addr %2 : $*Data
169+
br bb3
170+
171+
bb3:
172+
destroy_value %0 : $MyObject
173+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take_two!foreign : (MyObject.Type) -> (Data?, Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
174+
%51 = apply %50(%46, %46a, %35) : $@convention(objc_method) (Optional<NSData>, Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
175+
destroy_value %46 : $Optional<NSData>
176+
destroy_value %46a : $Optional<NSData>
177+
return %51 : $Optional<NSData>
178+
}
179+
180+
// CHECK-LABEL: sil [Osize] [ossa] @test6 :
181+
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
182+
// CHECK: apply [[FUNC]]
183+
// CHECK-LABEL: } // end sil function 'test6'
184+
sil [Osize] [ossa] @test6 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
185+
bb0(%0: @owned $MyObject):
186+
%35 = metatype $@objc_metatype MyObject.Type
187+
%41 = function_ref @getDataWrapper : $@convention(thin) () -> @owned DataWrapper
188+
%43 = apply %41() : $@convention(thin) () -> @owned DataWrapper
189+
%b = begin_borrow %43 : $DataWrapper
190+
%ex = struct_extract %b : $DataWrapper, #DataWrapper.data
191+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
192+
%45 = apply %44(%ex) : $@convention(method) (@guaranteed Data) -> @owned NSData
193+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
194+
end_borrow %b : $DataWrapper
195+
destroy_value %0 : $MyObject
196+
destroy_value %43 : $DataWrapper
197+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
198+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
199+
destroy_value %46 : $Optional<NSData>
200+
return %51 : $Optional<NSData>
201+
}
202+
71203
sil [Osize] [ossa] @test_dont_crash : $@convention(thin) (@owned MyObject) -> () {
72204
bb0(%0: @owned $MyObject):
73205
%35 = metatype $@objc_metatype MyObject.Type
@@ -105,3 +237,47 @@ bb7(%64 : @owned $Optional<Data>):
105237
return %102 : $()
106238
}
107239

240+
sil [Osize] [ossa] @test7 : $@convention(thin) (@in Data, @in Data) -> () {
241+
bb0(%0 : $*Data, %1 : $*Data):
242+
%35 = metatype $@objc_metatype MyObject.Type
243+
%b1 = load_borrow %0 : $*Data
244+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
245+
%45 = apply %44(%b1) : $@convention(method) (@guaranteed Data) -> @owned NSData
246+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
247+
end_borrow %b1 : $Data
248+
destroy_addr %0 : $*Data
249+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
250+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
251+
destroy_value %46 : $Optional<NSData>
252+
switch_enum %51 : $Optional<NSData>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
253+
254+
bb1(%54 : @owned $NSData):
255+
%55 = function_ref @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
256+
%56 = enum $Optional<NSData>, #Optional.some!enumelt, %54 : $NSData
257+
%57 = metatype $@thin Data.Type
258+
%58 = apply %55(%56, %57) : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
259+
%59 = enum $Optional<Data>, #Optional.some!enumelt, %58 : $Data
260+
destroy_value %56 : $Optional<NSData>
261+
br bb3(%59 : $Optional<Data>)
262+
263+
bb2:
264+
%62 = enum $Optional<Data>, #Optional.none!enumelt
265+
br bb3(%62 : $Optional<Data>)
266+
267+
bb3(%64 : @owned $Optional<Data>):
268+
destroy_value %64 : $Optional<Data>
269+
%35a = metatype $@objc_metatype MyObject.Type
270+
%b2 = load_borrow %1 : $*Data
271+
%44a = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
272+
%45a = apply %44a(%b2) : $@convention(method) (@guaranteed Data) -> @owned NSData
273+
%46a = enum $Optional<NSData>, #Optional.some!enumelt, %45a : $NSData
274+
end_borrow %b2 : $Data
275+
destroy_addr %1 : $*Data
276+
%50a = objc_method %35a : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
277+
%51a = apply %50a(%46a, %35a) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
278+
destroy_value %46a : $Optional<NSData>
279+
destroy_value %51a : $Optional<NSData>
280+
%102 = tuple ()
281+
return %102 : $()
282+
}
283+

0 commit comments

Comments
 (0)