diff --git a/lib/SILOptimizer/Transforms/SemanticARCOpts.cpp b/lib/SILOptimizer/Transforms/SemanticARCOpts.cpp index c40ec0b6a30fb..1b4c5204a70be 100644 --- a/lib/SILOptimizer/Transforms/SemanticARCOpts.cpp +++ b/lib/SILOptimizer/Transforms/SemanticARCOpts.cpp @@ -52,8 +52,23 @@ 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 consumingUses; + /// A list of destroy_values of the live range. - SmallVector destroyingUses; + /// + /// This is just a view into consuming uses. + ArrayRef 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 @@ -61,12 +76,12 @@ class LiveRange { /// flip the ownership of all of these instructions to guaranteed from owned. /// /// Corresponds to isOwnershipForwardingInst(...). - SmallVector ownershipForwardingUses; + ArrayRef 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 unknownConsumingUses; + ArrayRef unknownConsumingUses; public: LiveRange(SILValue value); @@ -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 getAllConsumingUses() const { return consumingUses; } + ArrayRef getDestroyingUses() const { return destroyingUses; } private: @@ -121,7 +141,7 @@ class LiveRange { return ownershipForwardingUses; } - void convertOwnedGeneralForwardingUsesToGuaranteed(); + void convertOwnedGeneralForwardingUsesToGuaranteed() &&; /// A consuming operation that: /// @@ -192,6 +212,10 @@ LiveRange::LiveRange(SILValue value) ownershipForwardingUses(), unknownConsumingUses() { assert(introducer.value.getOwnershipKind() == ValueOwnershipKind::Owned); + SmallVector tmpDestroyingUses; + SmallVector tmpForwardingConsumingUses; + SmallVector 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 worklist(introducer.value->getUses()); @@ -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(user)) { - destroyingUses.push_back(op); + tmpDestroyingUses.push_back(op); continue; } @@ -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. @@ -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( @@ -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. @@ -402,7 +441,8 @@ void LiveRange::convertToGuaranteedAndRAUW(SILValue newGuaranteedValue, InstModCallbacks callbacks) && { auto *value = cast(introducer.value); while (!destroyingUses.empty()) { - auto *d = destroyingUses.pop_back_val(); + auto *d = destroyingUses.back(); + destroyingUses = destroyingUses.drop_back(); callbacks.deleteInst(d->getUser()); ++NumEliminatedInsts; } @@ -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, @@ -429,7 +469,8 @@ 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; } @@ -437,7 +478,7 @@ void LiveRange::convertArgToGuaranteed(DeadEndBlocks &deadEndBlocks, // 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 @@ -1297,8 +1338,9 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst SmallVector scratchSpace; SmallPtrSet 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; } @@ -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; } } @@ -1641,7 +1682,7 @@ class StorageGuaranteesLoadVisitor SmallPtrSet 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()); } @@ -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); } @@ -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); } diff --git a/test/SILOptimizer/semantic-arc-opts.sil b/test/SILOptimizer/semantic-arc-opts.sil index 93f565b65ef20..30d2ed520f47f 100644 --- a/test/SILOptimizer/semantic-arc-opts.sil +++ b/test/SILOptimizer/semantic-arc-opts.sil @@ -59,6 +59,7 @@ class ClassLet { @_hasStorage let aLet: Klass @_hasStorage var aVar: Klass @_hasStorage let aLetTuple: (Klass, Klass) + @_hasStorage let anOptionalLet: FakeOptional @_hasStorage let anotherLet: ClassLet } @@ -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 + %9 = load [copy] %8 : $*FakeOptional + end_access %8 : $*FakeOptional + end_borrow %6 : $ClassLet + destroy_value %5 : $ClassLet + switch_enum %9 : $FakeOptional, case #FakeOptional.none!enumelt: bb1, case #FakeOptional.some!enumelt: bb2 + +bb1: + %107 = tuple () + return %107 : $() + +bb2(%39 : @owned $Klass): + unreachable +}