From 68db2e7c6ce6f2bf9cdce1add556ef69971bc6a1 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Wed, 14 Oct 2020 18:54:27 +0200 Subject: [PATCH] TempRValueOpt: don't use collectLoads in tryOptimizeStoreIntoTemp This refactoring removes a lot of special-casing in collectLoads and also makes tryOptimizeStoreIntoTemp simpler. It's a NFC. --- .../Transforms/TempRValueElimination.cpp | 101 +++++++----------- 1 file changed, 36 insertions(+), 65 deletions(-) diff --git a/lib/SILOptimizer/Transforms/TempRValueElimination.cpp b/lib/SILOptimizer/Transforms/TempRValueElimination.cpp index 7c80311fd4fe1..fc9b472dc5a46 100644 --- a/lib/SILOptimizer/Transforms/TempRValueElimination.cpp +++ b/lib/SILOptimizer/Transforms/TempRValueElimination.cpp @@ -61,6 +61,17 @@ namespace { /// it finds cases in which it is easy to determine that the source is /// unmodified during the copy destination's lifetime. Thus, the destination can /// be viewed as a short-lived "rvalue". +/// +/// As a second optimization, also stores into temporaries are handled. This is +/// a simple form of redundant-load-elimination (RLE). +/// +/// %temp = alloc_stack $T +/// store %src to [initialization] %temp : $*T +/// // no writes to %temp +/// %v = load [take] %temp : $*T +/// dealloc_stack %temp : $*T +/// +/// TODO: Check if we still need to handle stores when RLE supports OSSA. class TempRValueOptPass : public SILFunctionTransform { AliasAnalysis *aa = nullptr; @@ -94,14 +105,6 @@ class TempRValueOptPass : public SILFunctionTransform { bool TempRValueOptPass::collectLoadsFromProjection( SingleValueInstruction *projection, SILValue srcAddr, SmallPtrSetImpl &loadInsts) { - if (!srcAddr) { - LLVM_DEBUG( - llvm::dbgs() - << " Temp has addr_projection use?! Can not yet promote to value" - << *projection); - return false; - } - // Transitively look through projections on stack addresses. for (auto *projUseOper : projection->getUses()) { auto *user = projUseOper->getUser(); @@ -127,28 +130,15 @@ bool TempRValueOptPass::canApplyBeTreatedAsLoad( return false; } - if (srcAddr) { - // If the function may write to the source of the copy_addr, the apply - // cannot be treated as a load: all (potential) writes of the source must - // appear _after_ all loads of the temporary. But in case of a function call - // we don't know in which order the writes and loads are executed inside the - // called function. The source may be written before the temporary is - // loaded, which would make the optization invalid. - if (aa->mayWriteToMemory(apply.getInstruction(), srcAddr)) - return false; - } else { - // If we do not have an src address, but are indirect, bail. We would need - // to perform function signature specialization to change the functions - // signature to pass something direct. - if (convention.isIndirectConvention()) { - LLVM_DEBUG( - llvm::dbgs() - << " Temp used to materialize value for indirect convention?! Can " - "not remove temporary without func sig opts" - << *apply.getInstruction()); - return false; - } - } + // If the function may write to the source of the copy_addr, the apply + // cannot be treated as a load: all (potential) writes of the source must + // appear _after_ all loads of the temporary. But in case of a function call + // we don't know in which order the writes and loads are executed inside the + // called function. The source may be written before the temporary is + // loaded, which would make the optization invalid. + if (aa->mayWriteToMemory(apply.getInstruction(), srcAddr)) + return false; + return true; } @@ -237,14 +227,6 @@ bool TempRValueOptPass::collectLoads( return true; } case SILInstructionKind::OpenExistentialAddrInst: { - // If we do not have an srcAddr, bail. We do not support promoting this yet. - if (!srcAddr) { - LLVM_DEBUG(llvm::dbgs() << " Temp has open_existential_addr use?! Can " - "not yet promote to value" - << *user); - return false; - } - // We only support open existential addr if the access is immutable. auto *oeai = cast(user); if (oeai->getAccessKind() != OpenedExistentialAccess::Immutable) { @@ -294,11 +276,6 @@ bool TempRValueOptPass::collectLoads( return true; case SILInstructionKind::LoadBorrowInst: - // If we do not have a source addr, we must be trying to eliminate a - // store. Until we check that the source object is not destroyed within the - // given range, we need bail. - if (!srcAddr) - return false; loadInsts.insert(user); return true; case SILInstructionKind::FixLifetimeInst: @@ -598,29 +575,21 @@ TempRValueOptPass::tryOptimizeStoreIntoTemp(StoreInst *si) { if (user == si) continue; - // Destroys and deallocations are allowed to be in a different block. - if (isa(user) || isa(user)) - continue; - - // Same for load [take] on the top level temp object. SILGen always takes - // whole values from temporaries. If we have load [take] on projections from - // our base, we fail since those would be re-initializations. - if (auto *li = dyn_cast(user)) { - if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take) { - continue; - } - } - - // We pass in SILValue() since we do not have a source address. - if (!collectLoads(useOper, user, tempObj, SILValue(), loadInsts)) - return {std::next(si->getIterator()), false}; - // Bail if there is any kind of user which is not handled in the code below. switch (user->getKind()) { - case SILInstructionKind::CopyAddrInst: + case SILInstructionKind::DestroyAddrInst: + case SILInstructionKind::DeallocStackInst: + case SILInstructionKind::LoadInst: case SILInstructionKind::FixLifetimeInst: + break; + case SILInstructionKind::CopyAddrInst: + if (cast(user)->getDest() == tempObj) + return {std::next(si->getIterator()), false}; + break; case SILInstructionKind::MarkDependenceInst: - continue; + if (cast(user)->getValue() == tempObj) + return {std::next(si->getIterator()), false}; + break; default: return {std::next(si->getIterator()), false}; } @@ -690,11 +659,13 @@ TempRValueOptPass::tryOptimizeStoreIntoTemp(StoreInst *si) { break; } case SILInstructionKind::MarkDependenceInst: { - SILBuilderWithScope builder(user); auto mdi = cast(user); - auto newInst = builder.createMarkDependence(user->getLoc(), mdi->getValue(), si->getSrc()); + assert(mdi->getBase() == tempObj); + SILBuilderWithScope builder(user); + auto newInst = builder.createMarkDependence(user->getLoc(), + mdi->getValue(), si->getSrc()); mdi->replaceAllUsesWith(newInst); - toDelete.push_back(user); + toDelete.push_back(mdi); break; } // ASSUMPTION: no operations that may be handled by this default clause can