Skip to content

Commit 55b97f2

Browse files
committed
[Outliner] Bridged property expects hoisted destroy.
Thanks to CopyPropagation canonicalizing all values, not just those for which there is a copy_value instruction, the lifetime of the value that is loaded in the BridgedProperty pattern is shortened and the destroy_value appears right after its use rather than after the full CFG for the bridged property. Updated the BridgedProperty pattern to match that newly hoisted instruction.
1 parent db500e1 commit 55b97f2

File tree

1 file changed

+42
-38
lines changed

1 file changed

+42
-38
lines changed

lib/SILOptimizer/Transforms/Outliner.cpp

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ class BridgedProperty : public OutlinePattern {
256256
std::string getOutlinedFunctionName() override;
257257

258258
private:
259-
bool matchMethodCall(SILBasicBlock::iterator);
259+
bool matchMethodCall(SILBasicBlock::iterator, LoadInst *);
260260
CanSILFunctionType getOutlinedFunctionType(SILModule &M);
261261
void clearState();
262262
};
@@ -546,7 +546,8 @@ static bool matchSwitch(SwitchInfo &SI, SILInstruction *Inst,
546546
return true;
547547
}
548548

549-
bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It) {
549+
bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It,
550+
LoadInst *Load) {
550551
// Matches:
551552
// %33 = objc_method %31 : $UITextField, #UITextField.text!getter.foreign : (UITextField) -> () -> String?, $@convention(objc_method) (UITextField) -> @autoreleased Optional<NSString>
552553
// %34 = apply %33(%31) : $@convention(objc_method) (UITextField) -> @autoreleased Optional<NSString>
@@ -590,6 +591,44 @@ bool BridgedProperty::matchMethodCall(SILBasicBlock::iterator It) {
590591
PropApply->getArgument(0) != Instance || !PropApply->hasOneUse())
591592
return false;
592593

594+
if (Load) {
595+
// In OSSA, there will be a destroy_value matching the earlier load [copy].
596+
// In non-ossa, there will be a release matching the earlier retain. The
597+
// only user of the retained value is the unowned objective-c method
598+
// consumer.
599+
unsigned NumUses = 0;
600+
Release = nullptr;
601+
bool hasOwnership = Load->getFunction()->hasOwnership();
602+
for (auto *Use : Load->getUses()) {
603+
++NumUses;
604+
SILInstruction *R;
605+
if (hasOwnership) {
606+
R = dyn_cast<DestroyValueInst>(Use->getUser());
607+
} else {
608+
R = dyn_cast<StrongReleaseInst>(Use->getUser());
609+
}
610+
if (R) {
611+
if (!Release) {
612+
Release = R;
613+
} else {
614+
Release = nullptr;
615+
break;
616+
}
617+
}
618+
}
619+
if (!Release)
620+
return false;
621+
if (hasOwnership) {
622+
if (NumUses != 3)
623+
return false;
624+
} else {
625+
if (NumUses != 4)
626+
return false;
627+
}
628+
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
629+
assert(Release == &*It);
630+
}
631+
593632
// switch_enum %34 : $Optional<NSString>, case #Optional.some!enumelt: bb8, case #Optional.none!enumelt: bb9
594633
ADVANCE_ITERATOR_OR_RETURN_FALSE(It);
595634
return matchSwitch(switchInfo, &*It, PropApply);
@@ -653,44 +692,9 @@ bool BridgedProperty::matchInstSequence(SILBasicBlock::iterator It) {
653692
}
654693
}
655694

656-
if (!matchMethodCall(It))
695+
if (!matchMethodCall(It, Load))
657696
return false;
658697

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

0 commit comments

Comments
 (0)