Skip to content

Commit 090d57a

Browse files
committed
Look past class casts when finding the reference root.
For class storage AccessedStorage is now close to what some passes use for RC identity, but it still does not look past wrapping references in an Optional.
1 parent c0d664d commit 090d57a

File tree

2 files changed

+45
-19
lines changed

2 files changed

+45
-19
lines changed

include/swift/SIL/MemAccessUtils.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,10 @@ SILBasicBlock::iterator removeBeginAccess(BeginAccessInst *beginAccess);
12011201

12021202
namespace swift {
12031203

1204+
/// Return true if \p value is a cast that preserves the identity of the
1205+
/// reference at operand zero.
1206+
bool isRCIdentityPreservingCast(SingleValueInstruction *svi);
1207+
12041208
/// If \p svi is an access projection, return an address-type operand for the
12051209
/// incoming address.
12061210
///

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -365,15 +365,27 @@ bool swift::isLetAddress(SILValue address) {
365365
// MARK: FindReferenceRoot
366366
//===----------------------------------------------------------------------===//
367367

368+
bool swift::isRCIdentityPreservingCast(SingleValueInstruction *svi) {
369+
switch (svi->getKind()) {
370+
default:
371+
return false;
372+
// Ignore ownership casts
373+
case SILInstructionKind::CopyValueInst:
374+
case SILInstructionKind::BeginBorrowInst:
375+
// Ignore class type casts
376+
case SILInstructionKind::UpcastInst:
377+
case SILInstructionKind::UncheckedRefCastInst:
378+
case SILInstructionKind::UnconditionalCheckedCastInst:
379+
case SILInstructionKind::UnconditionalCheckedCastValueInst:
380+
case SILInstructionKind::RefToBridgeObjectInst:
381+
case SILInstructionKind::BridgeObjectToRefInst:
382+
return true;
383+
}
384+
}
385+
368386
namespace {
369387

370388
// Essentially RC identity where the starting point is already a reference.
371-
//
372-
// FIXME: We cannot currently see through type casts for true RC identity,
373-
// because property indices are not unique within a class hierarchy. Either fix
374-
// RefElementAddr::getFieldNo so it returns a unique index, or figure out a
375-
// different way to encode the property's VarDecl. (Note that the lack of a
376-
// unique property index could be the source of bugs elsewhere).
377389
class FindReferenceRoot {
378390
SmallPtrSet<SILPhiArgument *, 4> visitedPhis;
379391

@@ -386,18 +398,13 @@ class FindReferenceRoot {
386398

387399
protected:
388400
// Return an invalid value for a phi with no resolved inputs.
389-
//
390-
// FIXME: We should be able to see through RC identity like this:
391-
// nextRoot = stripRCIdentityCasts(nextRoot);
392401
SILValue recursiveFindRoot(SILValue ref) {
393-
while (true) {
394-
SILValue nextRoot = ref;
395-
nextRoot = stripOwnershipInsts(nextRoot);
396-
if (nextRoot == ref)
402+
while (auto *svi = dyn_cast<SingleValueInstruction>(ref)) {
403+
if (!isRCIdentityPreservingCast(svi)) {
397404
break;
398-
ref = nextRoot;
399-
}
400-
405+
}
406+
ref = svi->getOperand(0);
407+
};
401408
auto *phi = dyn_cast<SILPhiArgument>(ref);
402409
if (!phi || !phi->isPhiArgument()) {
403410
return ref;
@@ -558,8 +565,16 @@ const ValueDecl *AccessedStorage::getDecl(SILValue base) const {
558565
return global->getDecl();
559566

560567
case Class: {
561-
auto *decl = getObject()->getType().getNominalOrBoundGenericNominal();
562-
return decl ? getIndexedField(decl, getPropertyIndex()) : nullptr;
568+
// The property index is relative to the VarDecl in ref_element_addr, and
569+
// can only be reliably determined when the base is avaiable. Otherwise, we
570+
// can only make a best effort to extract it from the object type, which
571+
// might not even be a class in the case of bridge objects.
572+
if (ClassDecl *classDecl =
573+
base ? cast<RefElementAddrInst>(base)->getClassDecl()
574+
: getObject()->getType().getClassOrBoundGenericClass()) {
575+
return getIndexedField(classDecl, getPropertyIndex());
576+
}
577+
return nullptr;
563578
}
564579
case Tail:
565580
return nullptr;
@@ -1297,7 +1312,14 @@ void AccessPathDefUseTraversal::followProjection(SingleValueInstruction *svi,
12971312
AccessPathDefUseTraversal::UseKind
12981313
AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi,
12991314
DFSEntry dfs) {
1300-
if (isAccessedStorageCast(svi)) {
1315+
if (dfs.isRef()) {
1316+
if (isRCIdentityPreservingCast(svi)) {
1317+
pushUsers(svi, dfs);
1318+
return IgnoredUse;
1319+
}
1320+
// 'svi' will be processed below as either RefElementAddrInst,
1321+
// RefTailAddrInst, or some unknown LeafUse.
1322+
} else if (isAccessedStorageCast(svi)) {
13011323
pushUsers(svi, dfs);
13021324
return IgnoredUse;
13031325
}

0 commit comments

Comments
 (0)