Skip to content

Commit

Permalink
Merge pull request swiftlang#78616 from meg-gupta/outlinefix6.1
Browse files Browse the repository at this point in the history
[6.1] Outliner: Do not outline if the BridgedArg value is not available at the BridgedCall
  • Loading branch information
meg-gupta authored Jan 23, 2025
2 parents cae0dfe + 1c12c7b commit 7a0a6e7
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 15 deletions.
5 changes: 5 additions & 0 deletions include/swift/SILOptimizer/Utils/OwnershipOptUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,11 @@ class OwnershipRAUWHelper {
/// specified lexical value.
bool areUsesWithinLexicalValueLifetime(SILValue, ArrayRef<Operand *>);

/// Whether the provided uses lie within the current liveness of the
/// specified value.
bool areUsesWithinValueLifetime(SILValue value, ArrayRef<Operand *> uses,
DeadEndBlocks *deBlocks);

/// A utility composed ontop of OwnershipFixupContext that knows how to replace
/// a single use of a value with another value with a different ownership. We
/// allow for the values to have different types.
Expand Down
19 changes: 12 additions & 7 deletions lib/SILOptimizer/Transforms/Outliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,8 @@ class BridgedArgument {
: BridgeFun(nullptr), BridgeCall(nullptr), OptionalResult(nullptr),
ReleaseAfterBridge(nullptr), ReleaseArgAfterCall(nullptr), Idx(0) {}

static BridgedArgument match(unsigned ArgIdx, SILValue Arg, ApplyInst *AI);
static BridgedArgument match(unsigned ArgIdx, SILValue Arg, ApplyInst *AI,
DeadEndBlocks *deBlocks);

operator bool() const { return BridgeFun != nullptr; }
SILValue bridgedValue() { return BridgedValue; }
Expand Down Expand Up @@ -826,7 +827,7 @@ static SILInstruction *findReleaseOf(SILValue releasedValue,
}

BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
ApplyInst *AI) {
ApplyInst *AI, DeadEndBlocks *deBlocks) {
// Match
// %15 = function_ref @$SSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF
// %16 = apply %15(%14) : $@convention(method) (@guaranteed String) -> @owned NSString
Expand Down Expand Up @@ -879,6 +880,7 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,

if (SILBasicBlock::iterator(BridgeCall) == BridgeCall->getParent()->begin())
return BridgedArgument();

auto *FunRef =
dyn_cast<FunctionRefInst>(std::prev(SILBasicBlock::iterator(BridgeCall)));
if (!FunRef || !FunRef->hasOneUse() || BridgeCall->getCallee() != FunRef)
Expand Down Expand Up @@ -912,6 +914,12 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
BridgeFun->getName() != bridgeWitness.mangle())
return BridgedArgument();

if (hasOwnership && !BridgedValueRelease) {
SmallVector<Operand *> newUses{&AI->getOperandRef(ArgIdx)};
if (!areUsesWithinValueLifetime(BridgedValue, newUses, deBlocks)) {
return BridgedArgument();
}
}
return BridgedArgument(ArgIdx, FunRef, BridgeCall, Enum, BridgedValueRelease,
ReleaseAfter);
}
Expand Down Expand Up @@ -1087,10 +1095,6 @@ ObjCMethodCall::outline(SILModule &M) {
if (BridgedArgIdx < BridgedArguments.size() &&
BridgedArguments[BridgedArgIdx].Idx == OrigSigIdx) {
auto bridgedArgValue = BridgedArguments[BridgedArgIdx].bridgedValue();
if (bridgedArgValue->getOwnershipKind() == OwnershipKind::Guaranteed) {
bridgedArgValue = makeGuaranteedValueAvailable(
bridgedArgValue, BridgedCall, *deBlocks);
}
Args.push_back(bridgedArgValue);
++BridgedArgIdx;
} else {
Expand Down Expand Up @@ -1215,7 +1219,8 @@ bool ObjCMethodCall::matchInstSequence(SILBasicBlock::iterator I) {
if (Ty.isAnyObject())
continue;

auto BridgedArg = BridgedArgument::match(CurIdx, Param.get(), BridgedCall);
auto BridgedArg =
BridgedArgument::match(CurIdx, Param.get(), BridgedCall, deBlocks);
if (!BridgedArg)
continue;

Expand Down
24 changes: 24 additions & 0 deletions lib/SILOptimizer/Utils/OwnershipOptUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,30 @@ bool swift::areUsesWithinLexicalValueLifetime(SILValue value,
return false;
}

bool swift::areUsesWithinValueLifetime(SILValue value, ArrayRef<Operand *> uses,
DeadEndBlocks *deBlocks) {
assert(value->getFunction()->hasOwnership());

if (value->getOwnershipKind() == OwnershipKind::None) {
return true;
}
if (value->getOwnershipKind() != OwnershipKind::Guaranteed &&
value->getOwnershipKind() != OwnershipKind::Owned) {
return false;
}
if (value->getOwnershipKind() == OwnershipKind::Guaranteed) {
value = findOwnershipReferenceAggregate(value);
BorrowedValue borrowedValue(value);
if (!borrowedValue.isLocalScope()) {
return true;
}
}
SSAPrunedLiveness liveness(value->getFunction());
liveness.initializeDef(value);
liveness.computeSimple();
return liveness.areUsesWithinBoundary(uses, deBlocks);
}

//===----------------------------------------------------------------------===//
// BorrowedLifetimeExtender
//===----------------------------------------------------------------------===//
Expand Down
2 changes: 2 additions & 0 deletions test/SILOptimizer/outliner.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// RUN: %target-swift-frontend -Osize -import-objc-header %S/Inputs/Outliner.h %s -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation | %FileCheck %s
// RUN: %target-swift-frontend -Osize -g -import-objc-header %S/Inputs/Outliner.h %s -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation | %FileCheck %s

// RUN: %target-swift-frontend -Osize -import-objc-header %S/Inputs/Outliner.h %s -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation -enable-ossa-modules | %FileCheck %s
// RUN: %target-swift-frontend -Osize -g -import-objc-header %S/Inputs/Outliner.h %s -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation -enable-ossa-modules | %FileCheck %s
// REQUIRES: objc_interop
// REQUIRES: optimized_stdlib
// REQUIRES: swift_in_compiler
Expand Down
36 changes: 28 additions & 8 deletions test/SILOptimizer/outliner_ossa.sil
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ bb7(%64 : @owned $Optional<Data>):
return %102 : $()
}

// Not optimized
// CHECK-LABEL: sil [Osize] [ossa] @test2 :
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
// CHECK: apply [[FUNC]]
// CHECK: objc_method
// CHECK-LABEL: } // end sil function 'test2'
sil [Osize] [ossa] @test2 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
bb0(%0: @owned $MyObject):
Expand All @@ -96,9 +96,9 @@ bb0(%0: @owned $MyObject):
return %51 : $Optional<NSData>
}

// Not optimized
// CHECK-LABEL: sil [Osize] [ossa] @test3 :
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
// CHECK: apply [[FUNC]]
// CHECK: objc_method
// CHECK-LABEL: } // end sil function 'test3'
sil [Osize] [ossa] @test3 : $@convention(thin) (@owned MyObject, @in Data) -> @owned Optional<NSData> {
bb0(%0: @owned $MyObject, %1 : $*Data):
Expand All @@ -116,9 +116,9 @@ bb0(%0: @owned $MyObject, %1 : $*Data):
return %51 : $Optional<NSData>
}

// Not optimized
// CHECK-LABEL: sil [Osize] [ossa] @test4 :
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC8take_two4arg14arg210Foundation4DataVSgAJ_AJtFZToTembgbgnn_ :
// CHECK: apply [[FUNC]]
// CHECK: objc_method
// CHECK-LABEL: } // end sil function 'test4'
sil [Osize] [ossa] @test4 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
bb0(%0: @owned $MyObject):
Expand Down Expand Up @@ -177,9 +177,9 @@ bb3:
return %51 : $Optional<NSData>
}

// Not optimized
// CHECK-LABEL: sil [Osize] [ossa] @test6 :
// CHECK: [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
// CHECK: apply [[FUNC]]
// CHECK: objc_method
// CHECK-LABEL: } // end sil function 'test6'
sil [Osize] [ossa] @test6 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
bb0(%0: @owned $MyObject):
Expand Down Expand Up @@ -305,3 +305,23 @@ sil [Osize] [ossa] @destroy_after_borrow : $@convention(thin) () -> @owned Sub {
apply undef(%162) : $@convention(thin) (Int) -> ()
return %107 : $Sub
}

// CHECK-LABEL: sil [Osize] [ossa] @test_consume :
// CHECK: objc_method
// CHECK-LABEL: } // end sil function 'test_consume'
sil [Osize] [ossa] @test_consume : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
bb0(%0: @owned $MyObject):
%35 = metatype $@objc_metatype MyObject.Type
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
%43 = apply %41() : $@convention(thin) () -> @owned Data
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
%45 = apply %44(%43) : $@convention(method) (@guaranteed Data) -> @owned NSData
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
destroy_value %0 : $MyObject
%48 = apply undef(%43) : $@convention(method) (@owned Data) -> ()
%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>
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
destroy_value %46 : $Optional<NSData>
return %51 : $Optional<NSData>
}

0 comments on commit 7a0a6e7

Please sign in to comment.