Skip to content

RCIdentityAnalysis: some refactoring to improve clarity. #34274

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.

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
Oct 13, 2020
Merged
Changes from all commits
Commits
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
65 changes: 37 additions & 28 deletions lib/SILOptimizer/Analysis/RCIdentityAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,21 @@ static bool isNoPayloadEnum(SILValue V) {
return !EI->hasOperand();
}

/// Returns true if V is a valid operand of a cast which preserves RC identity.
///
/// V is a valid operand if it has a non-trivial type.
/// RCIdentityAnalysis must not look through casts which cast from a trivial
/// type, like a metatype, to something which is retainable, like an AnyObject.
/// On some platforms such casts dynamically allocate a ref-counted box for the
/// metatype. Naturally that is the place where a new rc-identity begins.
/// Therefore such a cast is introducing a new rc identical object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice comment!

///
/// If we would look through such a cast, ARC optimizations would get confused
/// and might eliminate a retain of such an object completely.
static bool isValidRCIdentityCastOperand(SILValue V, SILFunction *F) {
return !V->getType().isTrivial(*F);
}

/// RC identity is more than a guarantee that references refer to the same
/// object. It also means that reference counting operations on those references
/// have the same semantics. If the types on either side of a cast do not have
Expand All @@ -42,35 +57,26 @@ static bool isNoPayloadEnum(SILValue V) {
/// necessarily preserve RC identity because it may cast from a
/// reference-counted type to a non-reference counted type, or from a larger to
/// a smaller struct with fewer references.
static bool isRCIdentityPreservingCast(ValueKind Kind) {
switch (Kind) {
static SILValue getRCIdentityPreservingCastOperand(SILValue V) {
switch (V->getKind()) {
case ValueKind::UpcastInst:
case ValueKind::UncheckedRefCastInst:
case ValueKind::UnconditionalCheckedCastInst:
case ValueKind::InitExistentialRefInst:
case ValueKind::OpenExistentialRefInst:
case ValueKind::RefToBridgeObjectInst:
case ValueKind::BridgeObjectToRefInst:
case ValueKind::ConvertFunctionInst:
return true;
case ValueKind::ConvertFunctionInst: {
auto *inst = cast<SingleValueInstruction>(V);
SILValue castOp = inst->getOperand(0);
if (isValidRCIdentityCastOperand(castOp, inst->getFunction()))
return castOp;
break;
}
default:
return false;
break;
}
}

/// Returns a null SILValue if V has a trivial type, otherwise returns V.
///
/// RCIdentityAnalysis must not look through casts which cast from a trivial
/// type, like a metatype, to something which is retainable, like an AnyObject.
/// On some platforms such casts dynamically allocate a ref-counted box for the
/// metatype.
/// Now, if the RCRoot of such an AnyObject would be a trivial type, ARC
/// optimizations get confused and might eliminate a retain of such an object
/// completely.
static SILValue noTrivialType(SILValue V, SILFunction *F) {
if (V->getType().isTrivial(*F))
return SILValue();
return V;
return SILValue();
}

//===----------------------------------------------------------------------===//
Expand All @@ -79,10 +85,8 @@ static SILValue noTrivialType(SILValue V, SILFunction *F) {

static SILValue stripRCIdentityPreservingInsts(SILValue V) {
// First strip off RC identity preserving casts.
if (isRCIdentityPreservingCast(V->getKind())) {
auto *inst = cast<SingleValueInstruction>(V);
return noTrivialType(inst->getOperand(0), inst->getFunction());
}
if (SILValue castOp = getRCIdentityPreservingCastOperand(V))
return castOp;

// Then if we have a struct_extract that is extracting a non-trivial member
// from a struct with no other non-trivial members, a ref count operation on
Expand Down Expand Up @@ -133,9 +137,14 @@ static SILValue stripRCIdentityPreservingInsts(SILValue V) {
// since we will only visit it twice if we go around a back edge due to a
// different SILArgument that is actually being used for its phi node like
// purposes.
if (auto *A = dyn_cast<SILPhiArgument>(V))
if (SILValue Result = A->getSingleTerminatorOperand())
return noTrivialType(Result, A->getFunction());
if (auto *A = dyn_cast<SILPhiArgument>(V)) {
if (SILValue Result = A->getSingleTerminatorOperand()) {
// In case the terminator is a conditional cast, Result is the source of
// the cast.
if (isValidRCIdentityCastOperand(Result, A->getFunction()))
return Result;
}
}

return SILValue();
}
Expand Down Expand Up @@ -339,7 +348,7 @@ SILValue RCIdentityFunctionInfo::stripRCIdentityPreservingArgs(SILValue V,

unsigned IVListSize = IncomingValues.size();
if (IVListSize == 1 &&
!noTrivialType(IncomingValues[0].second, A->getFunction()))
!isValidRCIdentityCastOperand(IncomingValues[0].second, A->getFunction()))
return SILValue();

assert(IVListSize != 1 && "Should have been handled in "
Expand Down