Skip to content

[5.3] [semantic-arc-opts] Use all consuming uses instead of just destroying uses when validating if a LiveRange is alive in a scope. #32130

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 10, 2020
Merged

[5.3] [semantic-arc-opts] Use all consuming uses instead of just destroying uses when validating if a LiveRange is alive in a scope. #32130

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
91 changes: 66 additions & 25 deletions lib/SILOptimizer/Transforms/SemanticARCOpts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,36 @@ class LiveRange {
/// introducer and not to be forwarding.
OwnedValueIntroducer introducer;

/// A vector that we store all of our uses into.
///
/// Some properties of this array are:
///
/// 1. It is only mutated in the constructor of LiveRange.
///
/// 2. destroyingUses, ownershipForwardingUses, and unknownConsumingUses are
/// views into this array. We store the respective uses in the aforementioned
/// order. This is why it is important not to mutate consumingUses after we
/// construct the LiveRange since going from small -> large could invalidate
/// the uses.
SmallVector<Operand *, 6> consumingUses;

/// A list of destroy_values of the live range.
SmallVector<Operand *, 2> destroyingUses;
///
/// This is just a view into consuming uses.
ArrayRef<Operand *> destroyingUses;

/// A list of forwarding instructions that forward owned ownership, but that
/// are also able to be converted to guaranteed ownership. If we are able to
/// eliminate this LiveRange due to it being from a guaranteed value, we must
/// flip the ownership of all of these instructions to guaranteed from owned.
///
/// Corresponds to isOwnershipForwardingInst(...).
SmallVector<Operand *, 2> ownershipForwardingUses;
ArrayRef<Operand *> ownershipForwardingUses;

/// Consuming uses that we were not able to understand as a forwarding
/// instruction or a destroy_value. These must be passed a strongly control
/// equivalent +1 value.
SmallVector<Operand *, 2> unknownConsumingUses;
ArrayRef<Operand *> unknownConsumingUses;

public:
LiveRange(SILValue value);
Expand All @@ -87,6 +102,11 @@ class LiveRange {
HasConsumingUse_t
hasUnknownConsumingUse(bool assumingFixedPoint = false) const;

/// Return an array ref to /all/ consuming uses. Will include all 3 sorts of
/// consuming uses: destroying uses, forwarding consuming uses, and unknown
/// forwarding instruction.
ArrayRef<Operand *> getAllConsumingUses() const { return consumingUses; }

ArrayRef<Operand *> getDestroyingUses() const { return destroyingUses; }

private:
Expand Down Expand Up @@ -121,7 +141,7 @@ class LiveRange {
return ownershipForwardingUses;
}

void convertOwnedGeneralForwardingUsesToGuaranteed();
void convertOwnedGeneralForwardingUsesToGuaranteed() &&;

/// A consuming operation that:
///
Expand Down Expand Up @@ -192,6 +212,10 @@ LiveRange::LiveRange(SILValue value)
ownershipForwardingUses(), unknownConsumingUses() {
assert(introducer.value.getOwnershipKind() == ValueOwnershipKind::Owned);

SmallVector<Operand *, 32> tmpDestroyingUses;
SmallVector<Operand *, 32> tmpForwardingConsumingUses;
SmallVector<Operand *, 32> tmpUnknownConsumingUses;

// We know that our silvalue produces an @owned value. Look through all of our
// uses and classify them as either consuming or not.
SmallVector<Operand *, 32> worklist(introducer.value->getUses());
Expand Down Expand Up @@ -220,7 +244,7 @@ LiveRange::LiveRange(SILValue value)
// Ok, we know now that we have a consuming use. See if we have a destroy
// value, quickly up front. If we do have one, stash it and continue.
if (isa<DestroyValueInst>(user)) {
destroyingUses.push_back(op);
tmpDestroyingUses.push_back(op);
continue;
}

Expand All @@ -244,13 +268,13 @@ LiveRange::LiveRange(SILValue value)
return v.getOwnershipKind() ==
ValueOwnershipKind::Owned;
})) {
unknownConsumingUses.push_back(op);
tmpUnknownConsumingUses.push_back(op);
continue;
}

// Ok, this is a forwarding instruction whose ownership we can flip from
// owned -> guaranteed.
ownershipForwardingUses.push_back(op);
tmpForwardingConsumingUses.push_back(op);

// If we have a non-terminator, just visit its users recursively to see if
// the the users force the live range to be alive.
Expand Down Expand Up @@ -284,6 +308,20 @@ LiveRange::LiveRange(SILValue value)
}
}
}

// The order in which we append these to consumingUses matters since we assume
// their order as an invariant. This is done to ensure that we can pass off
// all of our uses or individual sub-arrays of our users without needing to
// move around memory.
llvm::copy(tmpDestroyingUses, std::back_inserter(consumingUses));
llvm::copy(tmpForwardingConsumingUses, std::back_inserter(consumingUses));
llvm::copy(tmpUnknownConsumingUses, std::back_inserter(consumingUses));

auto cUseArrayRef = llvm::makeArrayRef(consumingUses);
destroyingUses = cUseArrayRef.take_front(tmpDestroyingUses.size());
ownershipForwardingUses = cUseArrayRef.slice(
tmpDestroyingUses.size(), tmpForwardingConsumingUses.size());
unknownConsumingUses = cUseArrayRef.take_back(tmpUnknownConsumingUses.size());
}

void LiveRange::insertEndBorrowsAtDestroys(
Expand Down Expand Up @@ -339,9 +377,10 @@ void LiveRange::insertEndBorrowsAtDestroys(
}
}

void LiveRange::convertOwnedGeneralForwardingUsesToGuaranteed() {
void LiveRange::convertOwnedGeneralForwardingUsesToGuaranteed() && {
while (!ownershipForwardingUses.empty()) {
auto *i = ownershipForwardingUses.pop_back_val()->getUser();
auto *i = ownershipForwardingUses.back()->getUser();
ownershipForwardingUses = ownershipForwardingUses.drop_back();

// If this is a term inst, just convert all of its incoming values that are
// owned to be guaranteed.
Expand Down Expand Up @@ -402,7 +441,8 @@ void LiveRange::convertToGuaranteedAndRAUW(SILValue newGuaranteedValue,
InstModCallbacks callbacks) && {
auto *value = cast<SingleValueInstruction>(introducer.value);
while (!destroyingUses.empty()) {
auto *d = destroyingUses.pop_back_val();
auto *d = destroyingUses.back();
destroyingUses = destroyingUses.drop_back();
callbacks.deleteInst(d->getUser());
++NumEliminatedInsts;
}
Expand All @@ -412,7 +452,7 @@ void LiveRange::convertToGuaranteedAndRAUW(SILValue newGuaranteedValue,
// Then change all of our guaranteed forwarding insts to have guaranteed
// ownership kind instead of what ever they previously had (ignoring trivial
// results);
convertOwnedGeneralForwardingUsesToGuaranteed();
std::move(*this).convertOwnedGeneralForwardingUsesToGuaranteed();
}

void LiveRange::convertArgToGuaranteed(DeadEndBlocks &deadEndBlocks,
Expand All @@ -429,15 +469,16 @@ void LiveRange::convertArgToGuaranteed(DeadEndBlocks &deadEndBlocks,

// Then eliminate all of the destroys...
while (!destroyingUses.empty()) {
auto *d = destroyingUses.pop_back_val();
auto *d = destroyingUses.back();
destroyingUses = destroyingUses.drop_back();
callbacks.deleteInst(d->getUser());
++NumEliminatedInsts;
}

// and change all of our guaranteed forwarding insts to have guaranteed
// ownership kind instead of what ever they previously had (ignoring trivial
// results);
convertOwnedGeneralForwardingUsesToGuaranteed();
std::move(*this).convertOwnedGeneralForwardingUsesToGuaranteed();
}

LiveRange::HasConsumingUse_t
Expand Down Expand Up @@ -1297,8 +1338,9 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
SmallVector<Operand *, 8> scratchSpace;
SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
return !borrowScope.areUsesWithinScope(
destroys, scratchSpace, visitedBlocks, getDeadEndBlocks());
return !borrowScope.areUsesWithinScope(lr.getAllConsumingUses(),
scratchSpace, visitedBlocks,
getDeadEndBlocks());
})) {
return false;
}
Expand Down Expand Up @@ -1330,12 +1372,11 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
return false;
}

if (llvm::any_of(borrowScopeIntroducers,
[&](BorrowedValue borrowScope) {
return !borrowScope.areUsesWithinScope(
phiArgLR.getDestroyingUses(), scratchSpace,
visitedBlocks, getDeadEndBlocks());
})) {
if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
return !borrowScope.areUsesWithinScope(
phiArgLR.getAllConsumingUses(), scratchSpace, visitedBlocks,
getDeadEndBlocks());
})) {
return false;
}
}
Expand Down Expand Up @@ -1641,7 +1682,7 @@ class StorageGuaranteesLoadVisitor
SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());
if (!checker.validateLifetime(access, endScopeUses,
liveRange.getDestroyingUses())) {
liveRange.getAllConsumingUses())) {
// If we fail the linear lifetime check, then just recur:
return next(access->getOperand());
}
Expand Down Expand Up @@ -1738,8 +1779,8 @@ class StorageGuaranteesLoadVisitor
LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());

// Returns true on success. So we invert.
bool foundError = !checker.validateLifetime(baseObject, endScopeInsts,
liveRange.getDestroyingUses());
bool foundError = !checker.validateLifetime(
baseObject, endScopeInsts, liveRange.getAllConsumingUses());
return answer(foundError);
}

Expand Down Expand Up @@ -1777,7 +1818,7 @@ class StorageGuaranteesLoadVisitor
// Returns true on success. So we invert.
bool foundError = !checker.validateLifetime(
stack, destroyAddrOperands /*consuming users*/,
liveRange.getDestroyingUses() /*non consuming users*/);
liveRange.getAllConsumingUses() /*non consuming users*/);
return answer(foundError);
}

Expand Down
29 changes: 29 additions & 0 deletions test/SILOptimizer/semantic-arc-opts.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class ClassLet {
@_hasStorage let aLet: Klass
@_hasStorage var aVar: Klass
@_hasStorage let aLetTuple: (Klass, Klass)
@_hasStorage let anOptionalLet: FakeOptional<Klass>

@_hasStorage let anotherLet: ClassLet
}
Expand Down Expand Up @@ -2204,3 +2205,31 @@ bb6:
%9999 = tuple()
return %9999 : $()
}

// Make sure that we do not promote the load [copy] to a load_borrow since it
// has a use outside of the access scope.
//
// CHECK-LABEL: sil [ossa] @deadEndBlockDoNotPromote : $@convention(method) (@guaranteed ClassLet) -> () {
// CHECK: load_borrow
// CHECK: load [copy]
// CHECK: } // end sil function 'deadEndBlockDoNotPromote'
sil [ossa] @deadEndBlockDoNotPromote : $@convention(method) (@guaranteed ClassLet) -> () {
bb0(%0 : @guaranteed $ClassLet):
%4 = ref_element_addr %0 : $ClassLet, #ClassLet.anotherLet
%5 = load [copy] %4 : $*ClassLet
%6 = begin_borrow %5 : $ClassLet
%7 = ref_element_addr %6 : $ClassLet, #ClassLet.anOptionalLet
%8 = begin_access [read] [dynamic] %7 : $*FakeOptional<Klass>
%9 = load [copy] %8 : $*FakeOptional<Klass>
end_access %8 : $*FakeOptional<Klass>
end_borrow %6 : $ClassLet
destroy_value %5 : $ClassLet
switch_enum %9 : $FakeOptional<Klass>, case #FakeOptional.none!enumelt: bb1, case #FakeOptional.some!enumelt: bb2

bb1:
%107 = tuple ()
return %107 : $()

bb2(%39 : @owned $Klass):
unreachable
}