Skip to content

[cast-opt] Improve handling of arguments when optimizing Swift -> Obj… #25693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2019
Merged

[cast-opt] Improve handling of arguments when optimizing Swift -> Obj… #25693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/swift/SIL/SILBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2092,6 +2092,16 @@ class SILBuilder {
lowering.emitDestroyValue(*this, Loc, v);
}

/// Convenience function for destroying objects and addresses.
///
/// Objects are destroyed using emitDestroyValueOperation and addresses by
/// emitting destroy_addr.
void emitDestroyOperation(SILLocation loc, SILValue v) {
if (v->getType().isObject())
return emitDestroyValueOperation(loc, v);
createDestroyAddr(loc, v);
}

SILValue emitTupleExtract(SILLocation Loc, SILValue Operand, unsigned FieldNo,
SILType ResultTy) {
// Fold tuple_extract(tuple(x,y,z),2)
Expand Down
97 changes: 35 additions & 62 deletions lib/SILOptimizer/Utils/CastOptimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,27 +346,27 @@ CastOptimizer::optimizeBridgedObjCToSwiftCast(SILDynamicCastInst dynamicCast) {
// If we have guaranteed normal arguments, insert the destroy.
//
// TODO: Is it safe to just eliminate the initial retain?
Builder.emitDestroyValueOperation(Loc, srcArg);
Builder.emitDestroyOperation(Loc, srcArg);

// If we have an unconditional_checked_cast_addr, return early. We do not need
// to handle any conditional code.
if (isa<UnconditionalCheckedCastAddrInst>(Inst)) {
// Destroy the source value as unconditional_checked_cast_addr would.
Builder.emitDestroyValueOperation(Loc, srcOp);
Builder.emitDestroyOperation(Loc, srcOp);
eraseInstAction(Inst);
return (newI) ? newI : AI;
}

auto *CCABI = cast<CheckedCastAddrBranchInst>(Inst);
switch (CCABI->getConsumptionKind()) {
case CastConsumptionKind::TakeAlways:
Builder.emitDestroyValueOperation(Loc, srcOp);
Builder.emitDestroyOperation(Loc, srcOp);
break;
case CastConsumptionKind::TakeOnSuccess: {
{
// Insert a release in the success BB.
SILBuilderWithScope successBuilder(SuccessBB->begin());
successBuilder.emitDestroyValueOperation(Loc, srcOp);
successBuilder.emitDestroyOperation(Loc, srcOp);
}
{
// And a store in the failure BB.
Expand Down Expand Up @@ -590,7 +590,7 @@ static SILValue computeFinalCastedValue(SILBuilderWithScope &builder,
return failureBB->createPhiArgument(newAI->getType(),
ValueOwnershipKind::Owned);
}());
innerBuilder.emitDestroyValueOperation(loc, valueToDestroy);
innerBuilder.emitDestroyOperation(loc, valueToDestroy);
}

auto *condBrSuccessBB =
Expand Down Expand Up @@ -651,23 +651,25 @@ CastOptimizer::optimizeBridgedSwiftToObjCCast(SILDynamicCastInst dynamicCast) {
SILType SubstFnTy = bridgedFunc->getLoweredType().substGenericArgs(M, subMap);
SILFunctionConventions substConv(SubstFnTy.castTo<SILFunctionType>(), M);

// check that we can go through with the optimization
// Check that this is a case that the authors of this code thought it could
// handle.
if (!canOptimizeCast(BridgedTargetTy, M, substConv)) {
return nullptr;
}

SILBuilderWithScope Builder(Inst, builderContext);
auto FnRef = Builder.createFunctionRefFor(Loc, bridgedFunc);
auto ParamTypes = SubstFnTy.castTo<SILFunctionType>()->getParameters();
SILValue oldSrc;
if (Src->getType().isAddress() && !substConv.isSILIndirect(ParamTypes[0])) {
// Create load
oldSrc = Src;
Src =
Builder.emitLoadValueOperation(Loc, Src, LoadOwnershipQualifier::Take);
}

// Compensate different owning conventions of the replaced cast instruction
// and the inserted conversion function.
bool needRetainBeforeCall = false;
bool needReleaseAfterCall = false;
bool needReleaseInSuccess = false;
switch (ParamTypes[0].getConvention()) {
Expand All @@ -681,21 +683,10 @@ CastOptimizer::optimizeBridgedSwiftToObjCCast(SILDynamicCastInst dynamicCast) {
needReleaseInSuccess = true;
break;
case CastConsumptionKind::BorrowAlways:
llvm_unreachable("Should never hit this");
case CastConsumptionKind::CopyOnSuccess:
// Conservatively insert a retain/release pair around the conversion
// function because the conversion function could decrement the
// (global) reference count of the source object.
//
// %src = load %global_var
// apply %conversion_func(@guaranteed %src)
//
// sil conversion_func {
// %old_value = load %global_var
// store %something_else, %global_var
// strong_release %old_value
// }
needRetainBeforeCall = true;
needReleaseAfterCall = true;
// We assume that our caller is correct and will treat our argument as
// being immutable, so we do not need to do anything here.
break;
}
break;
Expand Down Expand Up @@ -733,53 +724,35 @@ CastOptimizer::optimizeBridgedSwiftToObjCCast(SILDynamicCastInst dynamicCast) {
return nullptr;
}

bool needStackAllocatedTemporary = false;
if (needRetainBeforeCall) {
if (AddressOnlyType) {
needStackAllocatedTemporary = true;
auto NewSrc = Builder.createAllocStack(Loc, Src->getType());
Builder.createCopyAddr(Loc, Src, NewSrc, IsNotTake, IsInitialization);
Src = NewSrc;
} else {
Builder.createRetainValue(Loc, Src, Builder.getDefaultAtomicity());
}
}

// Generate a code to invoke the bridging function.
auto *NewAI = Builder.createApply(Loc, FnRef, subMap, Src);

auto releaseSrc = [&](SILBuilder &Builder) {
if (AddressOnlyType) {
Builder.createDestroyAddr(Loc, Src);
} else {
Builder.emitDestroyValueOperation(Loc, Src);
}
};

Optional<SILBuilder> SuccBuilder;
if (needReleaseInSuccess || needStackAllocatedTemporary)
SuccBuilder.emplace(SuccessBB->begin());

// First if we are going to destroy the value unconditionally, just insert the
// destroy right after the call. This handles some of the conditional cases
// and /all/ of the consuming unconditional cases.
if (needReleaseAfterCall) {
releaseSrc(Builder);
} else if (needReleaseInSuccess) {
if (SuccessBB) {
releaseSrc(*SuccBuilder);
} else {
// For an unconditional cast, success is the only defined path
releaseSrc(Builder);
}
}

// Pop the temporary stack slot for a copied temporary.
if (needStackAllocatedTemporary) {
assert((bool)SuccessBB == (bool)FailureBB);
Builder.emitDestroyOperation(Loc, Src);
} else {
if (SuccessBB) {
SuccBuilder->createDeallocStack(Loc, Src);
SILBuilderWithScope FailBuilder(&*FailureBB->begin(), Builder);
FailBuilder.createDeallocStack(Loc, Src);
SILBuilderWithScope succBuilder(&*SuccessBB->begin(), Builder);
if (needReleaseInSuccess) {
succBuilder.emitDestroyOperation(Loc, Src);
} else {
if (oldSrc) {
succBuilder.emitStoreValueOperation(Loc, Src, oldSrc,
StoreOwnershipQualifier::Init);
}
}
SILBuilderWithScope failBuilder(&*FailureBB->begin(), Builder);
if (oldSrc) {
failBuilder.emitStoreValueOperation(Loc, Src, oldSrc,
StoreOwnershipQualifier::Init);
}
} else {
Builder.createDeallocStack(Loc, Src);
if (oldSrc) {
Builder.emitStoreValueOperation(Loc, Src, oldSrc,
StoreOwnershipQualifier::Init);
}
}
}

Expand Down
111 changes: 0 additions & 111 deletions test/SILOptimizer/const_fold_objc_bridge.sil
View file
Open in desktop

This file was deleted.

Loading