Skip to content

Commit dcea147

Browse files
nate-chandlernate-chandler
authored
Merge pull request #41943 from nate-chandler/copy_propagation/canonicalize-all
[CopyPropagation] Canonicalize all values.
2 parents dd40e14 + a903805 commit dcea147

File tree

5 files changed

+115
-61
lines changed

5 files changed

+115
-61
lines changed

‎lib/Demangling/Demangler.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2800,7 +2800,7 @@ std::string Demangler::demangleBridgedMethodParams() {
28002800
switch (kind) {
28012801
default:
28022802
return std::string();
2803-
case 'p': case 'a': case 'm':
2803+
case 'o': case 'p': case 'a': case 'm':
28042804
Str.push_back(kind);
28052805
}
28062806

‎lib/SILOptimizer/Transforms/CopyPropagation.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ namespace {
375375
class CopyPropagation : public SILFunctionTransform {
376376
/// True if debug_value instructions should be pruned.
377377
bool pruneDebug;
378-
/// True of all values should be canonicalized.
378+
/// True if all values should be canonicalized.
379379
bool canonicalizeAll;
380380
/// If true, then borrow scopes will be canonicalized, allowing copies of
381381
/// guaranteed values to be optimized. Does *not* shrink the borrow scope.
@@ -588,7 +588,7 @@ SILTransform *swift::createMandatoryCopyPropagation() {
588588
}
589589

590590
SILTransform *swift::createCopyPropagation() {
591-
return new CopyPropagation(/*pruneDebug*/ true, /*canonicalizeAll*/ false,
591+
return new CopyPropagation(/*pruneDebug*/ true, /*canonicalizeAll*/ true,
592592
/*canonicalizeBorrows*/ EnableRewriteBorrows,
593593
/*poisonRefs*/ false);
594594
}

‎lib/SILOptimizer/Transforms/Outliner.cpp

Lines changed: 77 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class OutlinerMangler : public Mangle::ASTMangler {
5050
/// The kind of method bridged.
5151
enum MethodKind : unsigned {
5252
BridgedProperty,
53+
BridgedProperty_Consuming,
5354
BridgedPropertyAddress,
5455
BridgedMethod,
5556
};
@@ -69,10 +70,12 @@ class OutlinerMangler : public Mangle::ASTMangler {
6970
Kind(BridgedMethod), IsReturnBridged(ReturnBridged) {}
7071

7172
/// Create an mangler for an outlined bridged property.
72-
OutlinerMangler(SILDeclRef Method, bool IsAddress)
73+
OutlinerMangler(SILDeclRef Method, bool IsAddress, bool ConsumesValue)
7374
: IsParameterBridged(nullptr), IsParameterGuaranteed(nullptr),
7475
MethodDecl(Method),
75-
Kind(IsAddress ? BridgedPropertyAddress : BridgedProperty),
76+
Kind(IsAddress ? BridgedPropertyAddress
77+
: (ConsumesValue ? BridgedProperty_Consuming
78+
: BridgedProperty)),
7679
IsReturnBridged(true) {}
7780

7881
std::string mangle();
@@ -82,6 +85,8 @@ class OutlinerMangler : public Mangle::ASTMangler {
8285
switch (Kind) {
8386
case BridgedProperty:
8487
return 'p';
88+
case BridgedProperty_Consuming:
89+
return 'o';
8590
case BridgedPropertyAddress:
8691
return 'a';
8792
case BridgedMethod:
@@ -234,6 +239,9 @@ class BridgedProperty : public OutlinePattern {
234239
ObjCMethodInst *ObjCMethod;
235240
SILInstruction *Release;
236241
ApplyInst *PropApply;
242+
SILInstruction *UnpairedRelease; // A release_value | destroy_value following
243+
// the apply which isn't paired to
244+
// load [copy] | strong_retain first value.
237245

238246
public:
239247
bool matchInstSequence(SILBasicBlock::iterator I) override;
@@ -256,7 +264,7 @@ class BridgedProperty : public OutlinePattern {
256264
std::string getOutlinedFunctionName() override;
257265

258266
private:
259-
bool matchMethodCall(SILBasicBlock::iterator);
267+
bool matchMethodCall(SILBasicBlock::iterator, LoadInst *);
260268
CanSILFunctionType getOutlinedFunctionType(SILModule &M);
261269
void clearState();
262270
};
@@ -269,12 +277,14 @@ void BridgedProperty::clearState() {
269277
ObjCMethod = nullptr;
270278
Release = nullptr;
271279
PropApply = nullptr;
280+
UnpairedRelease = nullptr;
272281
OutlinedName.clear();
273282
}
274283

275284
std::string BridgedProperty::getOutlinedFunctionName() {
276285
if (OutlinedName.empty()) {
277-
OutlinerMangler Mangler(ObjCMethod->getMember(), isa<LoadInst>(FirstInst));
286+
OutlinerMangler Mangler(ObjCMethod->getMember(), isa<LoadInst>(FirstInst),
287+
UnpairedRelease);
278288
OutlinedName = Mangler.mangle();
279289
}
280290
return OutlinedName;
@@ -297,11 +307,10 @@ CanSILFunctionType BridgedProperty::getOutlinedFunctionType(SILModule &M) {
297307
SILParameterInfo(Load->getType().getASTType(),
298308
ParameterConvention::Indirect_In_Guaranteed));
299309
else
300-
Parameters.push_back(SILParameterInfo(cast<ObjCMethodInst>(FirstInst)
301-
->getOperand()
302-
->getType()
303-
.getASTType(),
304-
ParameterConvention::Direct_Unowned));
310+
Parameters.push_back(SILParameterInfo(
311+
cast<ObjCMethodInst>(FirstInst)->getOperand()->getType().getASTType(),
312+
UnpairedRelease ? ParameterConvention::Direct_Owned
313+
: ParameterConvention::Direct_Unowned));
305314
SmallVector<SILResultInfo, 4> Results;
306315

307316
Results.push_back(SILResultInfo(
@@ -415,6 +424,8 @@ BridgedProperty::outline(SILModule &M) {
415424
FirstInst->getOperand(0)->getType());
416425
auto *Arg = OutlinedEntryBB->getArgument(0);
417426
FirstInst->setOperand(0, Arg);
427+
if (UnpairedRelease)
428+
UnpairedRelease->setOperand(0, Arg);
418429
PropApply->setArgument(0, Arg);
419430
}
420431
Builder.setInsertionPoint(OldMergeBB);
@@ -546,7 +557,8 @@ static bool matchSwitch(SwitchInfo &SI, SILInstruction *Inst,
546557
return true;
547558
}
548559

549-
bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It) {
560+
bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It,
561+
LoadInst *Load) {
550562
// Matches:
551563
// %33 = objc_method %31 : $UITextField, #UITextField.text!getter.foreign : (UITextField) -> () -> String?, $@convention(objc_method) (UITextField) -> @autoreleased Optional<NSString>
552564
// %34 = apply %33(%31) : $@convention(objc_method) (UITextField) -> @autoreleased Optional<NSString>
@@ -578,10 +590,6 @@ bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It) {
578590
ObjCMethod->getType().castTo<SILFunctionType>()->hasOpenedExistential())
579591
return false;
580592

581-
// Don't outline in the outlined function.
582-
if (ObjCMethod->getFunction()->getName().equals(getOutlinedFunctionName()))
583-
return false;
584-
585593
// %34 = apply %33(%31) : $@convention(objc_method) (UITextField) -> @autoreleased Optional<NSString>
586594
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
587595
PropApply = dyn_cast<ApplyInst>(It);
@@ -590,8 +598,60 @@ bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It) {
590598
PropApply->getArgument(0) != Instance || !PropApply->hasOneUse())
591599
return false;
592600

593-
// switch_enum %34 : $Optional<NSString>, case #Optional.some!enumelt: bb8, case #Optional.none!enumelt: bb9
601+
if (Load) {
602+
// In OSSA, there will be a destroy_value matching the earlier load [copy].
603+
// In non-ossa, there will be a release matching the earlier retain. The
604+
// only user of the retained value is the unowned objective-c method
605+
// consumer.
606+
unsigned NumUses = 0;
607+
Release = nullptr;
608+
bool hasOwnership = Load->getFunction()->hasOwnership();
609+
for (auto *Use : Load->getUses()) {
610+
++NumUses;
611+
SILInstruction *R;
612+
if (hasOwnership) {
613+
R = dyn_cast<DestroyValueInst>(Use->getUser());
614+
} else {
615+
R = dyn_cast<StrongReleaseInst>(Use->getUser());
616+
}
617+
if (R) {
618+
if (!Release) {
619+
Release = R;
620+
} else {
621+
Release = nullptr;
622+
break;
623+
}
624+
}
625+
}
626+
if (!Release)
627+
return false;
628+
if (hasOwnership) {
629+
if (NumUses != 3)
630+
return false;
631+
} else {
632+
if (NumUses != 4)
633+
return false;
634+
}
635+
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
636+
assert(Release == &*It);
637+
}
638+
594639
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
640+
if (auto *dvi = dyn_cast<DestroyValueInst>(*&It)) {
641+
if (Load)
642+
return false;
643+
if (dvi->getOperand() != Instance)
644+
return false;
645+
UnpairedRelease = dvi;
646+
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
647+
}
648+
649+
// Don't outline in the outlined function.
650+
if (ObjCMethod->getFunction()->getName().equals(getOutlinedFunctionName()))
651+
return false;
652+
653+
// switch_enum %34 : $Optional<NSString>, case #Optional.some!enumelt: bb8,
654+
// case #Optional.none!enumelt: bb9
595655
return matchSwitch(switchInfo, &*It, PropApply);
596656
}
597657

@@ -642,6 +702,7 @@ bool BridgedProperty::matchInstSequence(SILBasicBlock::iterator It) {
642702
if (Load->getFunction()->hasOwnership()) {
643703
if (Load->getOwnershipQualifier() != LoadOwnershipQualifier::Copy)
644704
return false;
705+
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
645706
} else {
646707
// strong_retain %31 : $UITextField
647708
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
@@ -652,44 +713,9 @@ bool BridgedProperty::matchInstSequence(SILBasicBlock::iterator It) {
652713
}
653714
}
654715

655-
if (!matchMethodCall(It))
716+
if (!matchMethodCall(It, Load))
656717
return false;
657718

658-
if (Load) {
659-
// In OSSA, there will be a destroy_value matching the earlier load [copy].
660-
// In non-ossa, there will be a release matching the earlier retain. The
661-
// only user of the retained value is the unowned objective-c method
662-
// consumer.
663-
unsigned NumUses = 0;
664-
Release = nullptr;
665-
bool hasOwnership = Load->getFunction()->hasOwnership();
666-
for (auto *Use : Load->getUses()) {
667-
++NumUses;
668-
SILInstruction *R;
669-
if (hasOwnership) {
670-
R = dyn_cast<DestroyValueInst>(Use->getUser());
671-
} else {
672-
R = dyn_cast<StrongReleaseInst>(Use->getUser());
673-
}
674-
if (R) {
675-
if (!Release) {
676-
Release = R;
677-
} else {
678-
Release = nullptr;
679-
break;
680-
}
681-
}
682-
}
683-
if (!Release)
684-
return false;
685-
if (hasOwnership) {
686-
if (NumUses != 3)
687-
return false;
688-
} else {
689-
if (NumUses != 4)
690-
return false;
691-
}
692-
}
693719
return true;
694720
}
695721

@@ -1274,8 +1300,6 @@ namespace {
12741300
class OutlinePatterns {
12751301
BridgedProperty BridgedPropertyPattern;
12761302
ObjCMethodCall ObjCMethodCallPattern;
1277-
llvm::DenseMap<CanType, SILDeclRef> BridgeToObjectiveCCache;
1278-
llvm::DenseMap<CanType, SILDeclRef> BridgeFromObjectiveCache;
12791303

12801304
public:
12811305
/// Try matching an outlineable pattern from the current instruction.

‎test/SILOptimizer/copy_propagation.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ sil [ossa] @takeOwnedCTwice : $@convention(thin) (@owned C, @owned C) -> ()
3636
sil [ossa] @takeGuaranteedC : $@convention(thin) (@guaranteed C) -> ()
3737
sil [ossa] @takeGuaranteedAnyObject : $@convention(thin) (@guaranteed AnyObject) -> ()
3838

39-
// -O ignores this because there's no copy_value
39+
// -O hoists the destroy
4040
// -Onone hoists the destroy and adds a poison flag.
4141
//
4242
// CHECK-LABEL: sil [ossa] @testDestroyAfterCall : {{.*}} {
4343
// CHECK: bb0:
4444
// CHECK: [[ARG:%.*]] = apply
4545
// CHECK-ONONE: destroy_value [poison] [[ARG]] : $B
46-
// CHECK: apply
4746
// CHECK-OPT: destroy_value [[ARG]] : $B
47+
// CHECK: apply
4848
// CHECK-LABEL: } // end sil function 'testDestroyAfterCall'
4949
sil [ossa] @testDestroyAfterCall : $@convention(thin) () -> () {
5050
bb0:

‎test/SILOptimizer/outliner.swift

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ public class MyGizmo {
1515
}
1616

1717
// CHECK-LABEL: sil {{.*}}@$s8outliner7MyGizmoC11usePropertyyyF :
18-
// CHECK: [[P_FUN:%.*]] = function_ref @$sSo5GizmoC14stringPropertySSSgvgToTepb_
19-
// CHECK: apply [[P_FUN]]({{.*}}) : $@convention(thin) (Gizmo) -> @owned Optional<String>
18+
// CHECK: [[A_FUN:%.*]] = function_ref @$sSo5GizmoC14stringPropertySSSgvgToTeab_
19+
// CHECK: apply [[A_FUN]]({{.*}}) : $@convention(thin) (@in_guaranteed Gizmo) -> @owned Optional<String>
2020
// CHECK-NOT: return
21-
// CHECK: apply [[P_FUN]]({{.*}}) : $@convention(thin) (Gizmo) -> @owned Optional<String>
21+
// CHECK: [[P_FUN:%.*]] = function_ref @$sSo5GizmoC14stringPropertySSSgvgToTeob_
22+
// CHECK: apply [[P_FUN]]({{.*}}) : $@convention(thin) (@owned Gizmo) -> @owned Optional<String>
2223
// CHECK: return
2324
// CHECK: } // end sil function '$s8outliner7MyGizmoC11usePropertyyyF'
2425
public func useProperty() {
@@ -65,6 +66,35 @@ public func testOutlining() {
6566
// CHECK: switch_enum [[RES]]
6667
// CHECK: } // end sil function '$s8outliner9dontCrash1ayyp_tF'
6768

69+
// CHECK-LABEL: sil shared [noinline] @$sSo5GizmoC14stringPropertySSSgvgToTeab_ : {{.*}} {
70+
// CHECK: {{bb[0-9]+}}([[ADDR:%[^,]+]] : $*Gizmo):
71+
// CHECK: [[INSTANCE:%[^,]+]] = load [[ADDR]]
72+
// CHECK: [[CONSUMING_OUTLINED_BRIDGED_PROPERTY:%[^,]+]] = function_ref @$sSo5GizmoC14stringPropertySSSgvgToTeob_
73+
// CHECK: strong_retain [[INSTANCE]]
74+
// CHECK: [[RETVAL:%[^,]+]] = apply [[CONSUMING_OUTLINED_BRIDGED_PROPERTY]]([[INSTANCE]])
75+
// CHECK: return [[RETVAL]]
76+
// CHECK: } // end sil function '$sSo5GizmoC14stringPropertySSSgvgToTeab_'
77+
78+
// CHECK-LABEL: sil shared [noinline] @$sSo5GizmoC14stringPropertySSSgvgToTeob_ : {{.*}} {
79+
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
80+
// CHECK: [[GIZME_STRINGPROPERTY_GETTER:%[^,]+]] = objc_method [[INSTANCE]] : $Gizmo, #Gizmo.stringProperty!getter.foreign
81+
// CHECK: [[MAYBE_NSSTRING:%[^,]+]] = apply [[GIZME_STRINGPROPERTY_GETTER]]([[INSTANCE]])
82+
// CHECK: strong_release [[INSTANCE]]
83+
// CHECK: switch_enum [[MAYBE_NSSTRING]] : $Optional<NSString>, case #Optional.some!enumelt: [[SOME_BLOCK:bb[0-9]+]], case #Optional.none!enumelt: [[NONE_BLOCK:bb[0-9]+]]
84+
// CHECK: [[SOME_BLOCK]]([[REGISTER_5:%[^,]+]] : $NSString):
85+
// CHECK: [[STRING_FROM_NSSTRING:%[^,]+]] = function_ref @$sSS10FoundationE36_unconditionallyBridgeFromObjectiveCySSSo8NSStringCSgFZ
86+
// CHECK: [[STRING_TYPE:%[^,]+]] = metatype $@thin String.Type
87+
// CHECK: [[STRING:%[^,]+]] = apply [[STRING_FROM_NSSTRING]]([[MAYBE_NSSTRING]], [[STRING_TYPE]])
88+
// CHECK: release_value [[MAYBE_NSSTRING]]
89+
// CHECK: [[SOME_STRING:%[^,]+]] = enum $Optional<String>, #Optional.some!enumelt, [[STRING]]
90+
// CHECK: br [[EXIT:bb[0-9]+]]([[SOME_STRING]] : $Optional<String>)
91+
// CHECK: [[NONE_BLOCK]]:
92+
// CHECK: [[NONE_STRING:%[^,]+]] = enum $Optional<String>, #Optional.none!enumelt
93+
// CHECK: br [[EXIT]]([[NONE_STRING]] : $Optional<String>)
94+
// CHECK: [[EXIT]]([[MAYBE_STRING:%[^,]+]] :
95+
// CHECK: return [[MAYBE_STRING]]
96+
// CHECK-LABEL: } // end sil function '$sSo5GizmoC14stringPropertySSSgvgToTeob_'
97+
6898
// CHECK-LABEL: sil shared [noinline] @$sSo5GizmoC14stringPropertySSSgvgToTepb_ : $@convention(thin) (Gizmo) -> @owned Optional<String>
6999
// CHECK: bb0(%0 : $Gizmo):
70100
// CHECK: %1 = objc_method %0 : $Gizmo, #Gizmo.stringProperty!getter.foreign : (Gizmo) -> () -> String?

0 commit comments

Comments
 (0)