Skip to content

[semantic-arc] Ignore type dependent uses when using a worklist to check for writes. #38301

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
Show file tree
Hide file tree
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
52 changes: 52 additions & 0 deletions include/swift/SIL/SILValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class DeadEndBlocks;
class ValueBaseUseIterator;
class ConsumingUseIterator;
class NonConsumingUseIterator;
class NonTypeDependentUseIterator;
class SILValue;

/// An enumeration which contains values for all the concrete ValueBase
Expand Down Expand Up @@ -387,6 +388,9 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
using consuming_use_range = iterator_range<consuming_use_iterator>;
using non_consuming_use_iterator = NonConsumingUseIterator;
using non_consuming_use_range = iterator_range<non_consuming_use_iterator>;
using non_typedependent_use_iterator = NonTypeDependentUseIterator;
using non_typedependent_use_range =
iterator_range<non_typedependent_use_iterator>;

inline use_iterator use_begin() const;
inline use_iterator use_end() const;
Expand All @@ -397,6 +401,9 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
inline non_consuming_use_iterator non_consuming_use_begin() const;
inline non_consuming_use_iterator non_consuming_use_end() const;

inline non_typedependent_use_iterator non_typedependent_use_begin() const;
inline non_typedependent_use_iterator non_typedependent_use_end() const;

/// Returns a range of all uses, which is useful for iterating over all uses.
/// To ignore debug-info instructions use swift::getNonDebugUses instead
/// (see comment in DebugUtils.h).
Expand All @@ -421,6 +428,10 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
/// Returns a range of all non consuming uses
inline non_consuming_use_range getNonConsumingUses() const;

/// Returns a range of uses that are not classified as a type dependent
/// operand of the user.
inline non_typedependent_use_range getNonTypeDependentUses() const;

template <class T>
inline T *getSingleUserOfType() const;

Expand Down Expand Up @@ -1071,6 +1082,7 @@ class Operand {
friend class ValueBaseUseIterator;
friend class ConsumingUseIterator;
friend class NonConsumingUseIterator;
friend class NonTypeDependentUseIterator;
template <unsigned N> friend class FixedOperandList;
friend class TrailingOperandsList;
};
Expand Down Expand Up @@ -1197,6 +1209,41 @@ ValueBase::non_consuming_use_end() const {
return ValueBase::non_consuming_use_iterator(nullptr);
}

class NonTypeDependentUseIterator : public ValueBaseUseIterator {
public:
explicit NonTypeDependentUseIterator(Operand *cur)
: ValueBaseUseIterator(cur) {}
NonTypeDependentUseIterator &operator++() {
assert(Cur && "incrementing past end()!");
assert(!Cur->isTypeDependent());
while ((Cur = Cur->NextUse)) {
if (!Cur->isTypeDependent())
break;
}
return *this;
}

NonTypeDependentUseIterator operator++(int unused) {
NonTypeDependentUseIterator copy = *this;
++*this;
return copy;
}
};

inline ValueBase::non_typedependent_use_iterator
ValueBase::non_typedependent_use_begin() const {
auto cur = FirstUse;
while (cur && cur->isTypeDependent()) {
cur = cur->NextUse;
}
return ValueBase::non_typedependent_use_iterator(cur);
}

inline ValueBase::non_typedependent_use_iterator
ValueBase::non_typedependent_use_end() const {
return ValueBase::non_typedependent_use_iterator(nullptr);
}

inline bool ValueBase::hasOneUse() const {
auto I = use_begin(), E = use_end();
if (I == E) return false;
Expand Down Expand Up @@ -1242,6 +1289,11 @@ ValueBase::getNonConsumingUses() const {
return {non_consuming_use_begin(), non_consuming_use_end()};
}

inline ValueBase::non_typedependent_use_range
ValueBase::getNonTypeDependentUses() const {
return {non_typedependent_use_begin(), non_typedependent_use_end()};
}

inline bool ValueBase::hasTwoUses() const {
auto iter = use_begin(), end = use_end();
for (unsigned i = 0; i < 2; ++i) {
Expand Down
14 changes: 10 additions & 4 deletions lib/SILOptimizer/SemanticARC/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,20 @@ void Context::verify() const {
bool Context::constructCacheValue(
SILValue initialValue,
SmallVectorImpl<Operand *> &wellBehavedWriteAccumulator) {
SmallVector<Operand *, 8> worklist(initialValue->getUses());
SmallVector<Operand *, 8> worklist(initialValue->getNonTypeDependentUses());

while (!worklist.empty()) {
auto *op = worklist.pop_back_val();
assert(!op->isTypeDependent() &&
"Uses that are type dependent should have been filtered before "
"being inserted into the worklist");
SILInstruction *user = op->getUser();

if (Projection::isAddressProjection(user) ||
isa<ProjectBlockStorageInst>(user)) {
for (SILValue r : user->getResults()) {
llvm::copy(r->getUses(), std::back_inserter(worklist));
llvm::copy(r->getNonTypeDependentUses(),
std::back_inserter(worklist));
}
continue;
}
Expand All @@ -59,7 +63,8 @@ bool Context::constructCacheValue(
}

// Otherwise, look through it and continue.
llvm::copy(oeai->getUses(), std::back_inserter(worklist));
llvm::copy(oeai->getNonTypeDependentUses(),
std::back_inserter(worklist));
continue;
}

Expand Down Expand Up @@ -93,7 +98,8 @@ bool Context::constructCacheValue(
}

// And then add the users to the worklist and continue.
llvm::copy(bai->getUses(), std::back_inserter(worklist));
llvm::copy(bai->getNonTypeDependentUses(),
std::back_inserter(worklist));
continue;
}

Expand Down
49 changes: 49 additions & 0 deletions test/SILOptimizer/semantic-arc-opts-loadcopy-to-loadborrow.sil
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Builtin
//////////////////

typealias AnyObject = Builtin.AnyObject
protocol Error {}

enum MyNever {}
enum FakeOptional<T> {
Expand Down Expand Up @@ -1450,3 +1451,51 @@ bb1:
bb2:
unreachable
}

// Make sure that we ignore type dependent operands when walking through the use
// list to determine if we are writing to a piece of memory.
protocol HasClassProperty {
var c: Klass { get set }
}

struct HasClassPropertyWrapper {
var h: HasClassProperty
var otherProperty: Builtin.NativeObject
}

sil @use_hasclassproperty : $@convention(method) <τ_0_0 where τ_0_0 : HasClassProperty> (@inout τ_0_0) -> (@owned Builtin.NativeObject, @error Error)

// Make sure we don't crash on this code. We used to crash by not ignoring the
// type dependent operand usage of %2.
//
// TODO: We should be able to convert %5 to a load_borrow since we know that %1
// and %4 do not alias. When that is done, update the test case appropriately.
//
// CHECK-LABEL: sil [ossa] @use_class_property_wrapper : $@convention(thin) (@inout HasClassPropertyWrapper) -> () {
// CHECK: load [copy]
// CHECK: } // end sil function 'use_class_property_wrapper'
sil [ossa] @use_class_property_wrapper : $@convention(thin) (@inout HasClassPropertyWrapper) -> () {
bb0(%0 : $*HasClassPropertyWrapper):
%1 = struct_element_addr %0 : $*HasClassPropertyWrapper, #HasClassPropertyWrapper.h
%2 = open_existential_addr mutable_access %1 : $*HasClassProperty to $*@opened("85AB1D00-DF62-11EB-A413-ACDE48001122") HasClassProperty
%3 = function_ref @use_hasclassproperty : $@convention(method) <τ_0_0 where τ_0_0 : HasClassProperty> (@inout τ_0_0) -> (@owned Builtin.NativeObject, @error Error)
%4 = struct_element_addr %0 : $*HasClassPropertyWrapper, #HasClassPropertyWrapper.otherProperty
%5 = load [copy] %4 : $*Builtin.NativeObject
%f2 = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %f2(%5) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
try_apply %3<@opened("85AB1D00-DF62-11EB-A413-ACDE48001122") HasClassProperty>(%2) : $@convention(method) <τ_0_0 where τ_0_0 : HasClassProperty> (@inout τ_0_0) -> (@owned Builtin.NativeObject, @error Error), normal bb1, error bb2

bb1(%str : @owned $Builtin.NativeObject):
destroy_value %str : $Builtin.NativeObject
destroy_value %5 : $Builtin.NativeObject
br bb3

bb2(%error : @owned $Error):
destroy_value %error : $Error
destroy_value %5 : $Builtin.NativeObject
br bb3

bb3:
%9999 = tuple()
return %9999 : $()
}