Skip to content

Commit adb11ec

Browse files
meg-guptameg-gupta
authored
Merge pull request #39670 from meg-gupta/migrateoutliner
Migrate Outliner to OSSA
2 parents 5b2f36a + 5d6b6b7 commit adb11ec

File tree

2 files changed

+190
-42
lines changed

2 files changed

+190
-42
lines changed

‎lib/SILOptimizer/Transforms/Outliner.cpp‎

Lines changed: 83 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ class BridgedProperty : public OutlinePattern {
224224
SILBasicBlock *StartBB;
225225
SwitchInfo switchInfo;
226226
ObjCMethodInst *ObjCMethod;
227-
StrongReleaseInst *Release;
227+
SILInstruction *Release;
228228
ApplyInst *PropApply;
229229

230230
public:
@@ -505,11 +505,18 @@ static bool matchSwitch(SwitchInfo &SI, SILInstruction *Inst,
505505
return false;
506506

507507
if (numSomeEnumUses == 2) {
508-
// release_value %38 : $Optional<NSString>
508+
// [release_value | destroy_value] %38 : $Optional<NSString>
509509
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
510-
auto *RVI = dyn_cast<ReleaseValueInst>(It);
511-
if (!RVI || RVI->getOperand() != SomeEnum)
512-
return false;
510+
bool hasOwnership = It->getFunction()->hasOwnership();
511+
if (hasOwnership) {
512+
auto *DVI = dyn_cast<DestroyValueInst>(It);
513+
if (!DVI || DVI->getOperand() != SomeEnum)
514+
return false;
515+
} else {
516+
auto *RVI = dyn_cast<ReleaseValueInst>(It);
517+
if (!RVI || RVI->getOperand() != SomeEnum)
518+
return false;
519+
}
513520
}
514521

515522
// br bb10(%41 : $Optional<String>)
@@ -578,7 +585,7 @@ bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It) {
578585
bool BridgedProperty::matchInstSequence(SILBasicBlock::iterator It) {
579586
// Matches:
580587
// [ optionally:
581-
// %31 = load %30 : $*UITextField
588+
// %31 = load %30 : $*UITextField or %31 = load [copy] %30 : $*UITextField
582589
// strong_retain %31 : $UITextField
583590
// ]
584591
// %33 = objc_method %31 : $UITextField, #UITextField.text!getter.foreign : (UITextField) -> () -> String?, $@convention(objc_method) (UITextField) -> @autoreleased Optional<NSString>
@@ -619,25 +626,39 @@ bool BridgedProperty::matchInstSequence(SILBasicBlock::iterator It) {
619626
StartBB = FirstInst->getParent();
620627

621628
if (Load) {
622-
// strong_retain %31 : $UITextField
623-
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
624-
auto *Retain = dyn_cast<StrongRetainInst>(It);
625-
if (!Retain || Retain->getOperand() != Load)
626-
return false;
627-
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
629+
if (Load->getFunction()->hasOwnership()) {
630+
if (Load->getOwnershipQualifier() != LoadOwnershipQualifier::Copy)
631+
return false;
632+
} else {
633+
// strong_retain %31 : $UITextField
634+
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
635+
auto *Retain = dyn_cast<StrongRetainInst>(It);
636+
if (!Retain || Retain->getOperand() != Load)
637+
return false;
638+
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
639+
}
628640
}
629641

630642
if (!matchMethodCall(It))
631643
return false;
632644

633645
if (Load) {
634-
// There will be a release matching the earlier retain. The only user of the
635-
// retained value is the unowned objective-c method consumer.
646+
// In OSSA, there will be a destroy_value matching the earlier load [copy].
647+
// In non-ossa, there will be a release matching the earlier retain. The
648+
// only user of the retained value is the unowned objective-c method
649+
// consumer.
636650
unsigned NumUses = 0;
637651
Release = nullptr;
652+
bool hasOwnership = Load->getFunction()->hasOwnership();
638653
for (auto *Use : Load->getUses()) {
639654
++NumUses;
640-
if (auto *R = dyn_cast<StrongReleaseInst>(Use->getUser())) {
655+
SILInstruction *R;
656+
if (hasOwnership) {
657+
R = dyn_cast<DestroyValueInst>(Use->getUser());
658+
} else {
659+
R = dyn_cast<StrongReleaseInst>(Use->getUser());
660+
}
661+
if (R) {
641662
if (!Release) {
642663
Release = R;
643664
} else {
@@ -646,8 +667,15 @@ bool BridgedProperty::matchInstSequence(SILBasicBlock::iterator It) {
646667
}
647668
}
648669
}
649-
if (!Release || NumUses != 4)
670+
if (!Release)
650671
return false;
672+
if (hasOwnership) {
673+
if (NumUses != 3)
674+
return false;
675+
} else {
676+
if (NumUses != 4)
677+
return false;
678+
}
651679
}
652680
return true;
653681
}
@@ -676,16 +704,18 @@ class BridgedArgument {
676704
ApplyInst *BridgeCall;
677705
EnumInst *OptionalResult;
678706
SILValue BridgedValue;
679-
ReleaseValueInst *ReleaseAfterBridge;
680-
ReleaseValueInst *ReleaseArgAfterCall;
707+
SILInstruction *ReleaseAfterBridge;
708+
SILInstruction *ReleaseArgAfterCall;
681709
unsigned Idx = 0;
682710

683711
// Matched bridged argument.
684712
BridgedArgument(unsigned Idx, FunctionRefInst *F, ApplyInst *A, EnumInst *E,
685-
ReleaseValueInst *R0, ReleaseValueInst *R1)
713+
SILInstruction *R0, SILInstruction *R1)
686714
: BridgeFun(F), BridgeCall(A), OptionalResult(E),
687715
BridgedValue(FullApplySite(A).getSelfArgument()),
688-
ReleaseAfterBridge(R0), ReleaseArgAfterCall(R1), Idx(Idx) {}
716+
ReleaseAfterBridge(R0), ReleaseArgAfterCall(R1), Idx(Idx) {
717+
assert(!R0 || isa<ReleaseValueInst>(R0) || isa<DestroyValueInst>(R0));
718+
}
689719

690720
/// Invalid argument constructor.
691721
BridgedArgument()
@@ -726,7 +756,7 @@ void BridgedArgument::transferTo(SILValue BridgedValue,
726756
DestBB->moveTo(SILBasicBlock::iterator(BridgedCall), OptionalResult);
727757
if (ReleaseAfterBridge) {
728758
DestBB->moveTo(SILBasicBlock::iterator(BridgedCall), ReleaseAfterBridge);
729-
ReleaseAfterBridge->setOperand(BridgedValue);
759+
ReleaseAfterBridge->setOperand(0, BridgedValue);
730760
}
731761
auto AfterCall = std::next(SILBasicBlock::iterator(BridgedCall));
732762
DestBB->moveTo(SILBasicBlock::iterator(AfterCall), ReleaseArgAfterCall);
@@ -741,13 +771,20 @@ void BridgedArgument::eraseFromParent() {
741771
BridgeFun->eraseFromParent();
742772
}
743773

744-
static ReleaseValueInst *findReleaseOf(SILValue releasedValue,
745-
SILBasicBlock::iterator from,
746-
SILBasicBlock::iterator to) {
774+
static SILInstruction *findReleaseOf(SILValue releasedValue,
775+
SILBasicBlock::iterator from,
776+
SILBasicBlock::iterator to) {
777+
bool hasOwnership = releasedValue->getFunction()->hasOwnership();
747778
while (from != to) {
748-
auto release = dyn_cast<ReleaseValueInst>(&*from);
749-
if (release && release->getOperand() == releasedValue)
750-
return release;
779+
if (hasOwnership) {
780+
auto destroy = dyn_cast<DestroyValueInst>(&*from);
781+
if (destroy && destroy->getOperand() == releasedValue)
782+
return destroy;
783+
} else {
784+
auto release = dyn_cast<ReleaseValueInst>(&*from);
785+
if (release && release->getOperand() == releasedValue)
786+
return release;
787+
}
751788
++from;
752789
}
753790
return nullptr;
@@ -757,14 +794,13 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
757794
ApplyInst *AI) {
758795
// Match
759796
// %15 = function_ref @$SSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF
760-
// %16 = apply %15(%14) :
761-
// $@convention(method) (@guaranteed String) -> @owned NSString
797+
// %16 = apply %15(%14) : $@convention(method) (@guaranteed String) -> @owned NSString
762798
// %17 = enum $Optional<NSString>, #Optional.some!enumelt, %16 : $NSString
763-
// release_value %14 : $String
799+
// [release_value | destroy_value] %14 : $String
764800
// ...
765-
// apply %objcMethod(%17, ...) : $@convention(objc_method) (Optional<NSString> ...) ->
801+
// apply %objcMethod(%17, ...) : $@convention(objc_method) (Optional<NSString>...) ->
766802
// release_value ...
767-
// release_value %17 : $Optional<NSString>
803+
// [release_value | destroy_value] %17 : $Optional<NSString>
768804
//
769805
auto *Enum = dyn_cast<EnumInst>(Arg);
770806
if (!Enum || !Enum->hasOperand())
@@ -795,10 +831,15 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
795831
// value.
796832
if (Enum->getParent() != AI->getParent())
797833
return BridgedArgument();
798-
auto *BridgedValueRelease = dyn_cast_or_null<ReleaseValueInst>(
834+
835+
bool hasOwnership = AI->getFunction()->hasOwnership();
836+
auto *BridgedValueRelease =
799837
findReleaseOf(BridgedValue, std::next(SILBasicBlock::iterator(Enum)),
800-
SILBasicBlock::iterator(AI)));
801-
if (BridgedValueRelease && BridgedValueRelease->getOperand() != BridgedValue)
838+
SILBasicBlock::iterator(AI));
839+
assert(!BridgedValueRelease ||
840+
(hasOwnership && isa<DestroyValueInst>(BridgedValueRelease)) ||
841+
isa<ReleaseValueInst>(BridgedValueRelease));
842+
if (BridgedValueRelease && BridgedValueRelease->getOperand(0) != BridgedValue)
802843
return BridgedArgument();
803844

804845
if (SILBasicBlock::iterator(BridgeCall) == BridgeCall->getParent()->begin())
@@ -808,7 +849,7 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
808849
if (!FunRef || !FunRef->hasOneUse() || BridgeCall->getCallee() != FunRef)
809850
return BridgedArgument();
810851

811-
ReleaseValueInst *ReleaseAfter = nullptr;
852+
SILInstruction *ReleaseAfter = nullptr;
812853
for (auto *Use : Enum->getUses()) {
813854
if (Use->getUser() == AI)
814855
continue;
@@ -817,7 +858,11 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
817858
if (ReleaseAfter)
818859
return BridgedArgument();
819860

820-
ReleaseAfter = dyn_cast<ReleaseValueInst>(Use->getUser());
861+
if (hasOwnership) {
862+
ReleaseAfter = dyn_cast<DestroyValueInst>(Use->getUser());
863+
} else {
864+
ReleaseAfter = dyn_cast<ReleaseValueInst>(Use->getUser());
865+
}
821866
if (!ReleaseAfter)
822867
return BridgedArgument();
823868
}
@@ -838,7 +883,7 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
838883
}
839884

840885
namespace {
841-
// Match the return value briding pattern.
886+
// Match the return value bridging pattern.
842887
// switch_enum %20 : $Optional<NSString>, case #O.some: bb1, case #O.none: bb2
843888
//
844889
// bb1(%23 : $NSString):
@@ -1274,10 +1319,6 @@ class Outliner : public SILFunctionTransform {
12741319
void run() override {
12751320
auto *Fun = getFunction();
12761321

1277-
// We do not support [ossa] now.
1278-
if (Fun->hasOwnership())
1279-
return;
1280-
12811322
// Only outline if we optimize for size.
12821323
if (!Fun->optimizeForSize())
12831324
return;

‎test/SILOptimizer/outliner_ossa.sil‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// RUN: %target-sil-opt -outliner %s -enable-sil-verify-all | %FileCheck %s
2+
3+
// REQUIRES: OS=macosx
4+
5+
sil_stage canonical
6+
7+
import Builtin
8+
import Swift
9+
import Foundation
10+
11+
12+
@objc class MyObject {
13+
@objc static func take(arg: Data?) -> Data?
14+
}
15+
16+
sil @getData : $@convention(thin) () -> @owned Data
17+
sil @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
18+
sil @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
19+
20+
// We used to have a use-after release failure.
21+
22+
// CHECK-LABEL: sil [Osize] [ossa] @test : $@convention(thin) (@owned MyObject) -> () {
23+
// CHECK: bb0([[ARG:%.*]] : @owned $MyObject):
24+
// CHECK: [[META:%.*]] = metatype $@objc_metatype MyObject.Type
25+
// CHECK: [[FUN1:%.*]] = function_ref @getData : $@convention(thin) () -> @owned Data
26+
// CHECK: [[DATA:%.*]] = apply [[FUN1]]() : $@convention(thin) () -> @owned Data
27+
// CHECK: destroy_value [[ARG]] : $MyObject
28+
// CHECK: [[OUTLINED:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembnb_ : $@convention(thin) (@owned Data, @objc_metatype MyObject.Type) -> @owned Optional<Data>
29+
// CHECK: [[RES:%.*]] = apply [[OUTLINED]]([[DATA]], [[META]]) : $@convention(thin) (@owned Data, @objc_metatype MyObject.Type) -> @owned Optional<Data>
30+
// CHECK: br bb1
31+
32+
// CHECK: bb1:
33+
// CHECK: destroy_value [[RES]]
34+
// CHECK: [[T:%.*]] = tuple ()
35+
// CHECK: return [[T]] : $()
36+
// CHECK: } // end sil function 'test'
37+
sil [Osize] [ossa] @test : $@convention(thin) (@owned MyObject) -> () {
38+
bb0(%0: @owned $MyObject):
39+
%35 = metatype $@objc_metatype MyObject.Type
40+
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
41+
%43 = apply %41() : $@convention(thin) () -> @owned Data
42+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
43+
%45 = apply %44(%43) : $@convention(method) (@guaranteed Data) -> @owned NSData
44+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
45+
destroy_value %0 : $MyObject
46+
destroy_value %43 : $Data
47+
%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>
48+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
49+
destroy_value %46 : $Optional<NSData>
50+
switch_enum %51 : $Optional<NSData>, case #Optional.some!enumelt: bb5, case #Optional.none!enumelt: bb6
51+
52+
bb5(%54 : @owned $NSData):
53+
%55 = function_ref @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
54+
%56 = enum $Optional<NSData>, #Optional.some!enumelt, %54 : $NSData
55+
%57 = metatype $@thin Data.Type
56+
%58 = apply %55(%56, %57) : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
57+
%59 = enum $Optional<Data>, #Optional.some!enumelt, %58 : $Data
58+
destroy_value %56 : $Optional<NSData>
59+
br bb7(%59 : $Optional<Data>)
60+
61+
bb6:
62+
%62 = enum $Optional<Data>, #Optional.none!enumelt
63+
br bb7(%62 : $Optional<Data>)
64+
65+
bb7(%64 : @owned $Optional<Data>):
66+
destroy_value %64 : $Optional<Data>
67+
%102 = tuple ()
68+
return %102 : $()
69+
}
70+
71+
sil [Osize] [ossa] @test_dont_crash : $@convention(thin) (@owned MyObject) -> () {
72+
bb0(%0: @owned $MyObject):
73+
%35 = metatype $@objc_metatype MyObject.Type
74+
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
75+
%43 = apply %41() : $@convention(thin) () -> @owned Data
76+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
77+
%45 = apply %44(%43) : $@convention(method) (@guaranteed Data) -> @owned NSData
78+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
79+
br bb4
80+
81+
bb4:
82+
destroy_value %0 : $MyObject
83+
destroy_value %43 : $Data
84+
%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>
85+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
86+
destroy_value %46 : $Optional<NSData>
87+
switch_enum %51 : $Optional<NSData>, case #Optional.some!enumelt: bb5, case #Optional.none!enumelt: bb6
88+
89+
bb5(%54 : @owned $NSData):
90+
%55 = function_ref @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
91+
%56 = enum $Optional<NSData>, #Optional.some!enumelt, %54 : $NSData
92+
%57 = metatype $@thin Data.Type
93+
%58 = apply %55(%56, %57) : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
94+
%59 = enum $Optional<Data>, #Optional.some!enumelt, %58 : $Data
95+
destroy_value %56 : $Optional<NSData>
96+
br bb7(%59 : $Optional<Data>)
97+
98+
bb6:
99+
%62 = enum $Optional<Data>, #Optional.none!enumelt
100+
br bb7(%62 : $Optional<Data>)
101+
102+
bb7(%64 : @owned $Optional<Data>):
103+
destroy_value %64 : $Optional<Data>
104+
%102 = tuple ()
105+
return %102 : $()
106+
}
107+

0 commit comments

Comments
 (0)