Skip to content

Commit 3eda1ea

Browse files
committed
[arc-opts] Teach IsAddressWrittenToDefUseAnalysis how to track "well behaved" writes but don't do use it.
In a future commit, I am going to build on this to promote load [copy] -> load_borrow from inout arguments where none of the writes overlap the load [copy]'s result's lifetime. By committing this separately, I am using the current pass's logic to validate the change.
1 parent dd7f780 commit 3eda1ea

File tree

1 file changed

+37
-45
lines changed

1 file changed

+37
-45
lines changed

lib/SILOptimizer/Transforms/SemanticARCOpts.cpp

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#define DEBUG_TYPE "sil-semantic-arc-opts"
1414
#include "swift/Basic/BlotSetVector.h"
1515
#include "swift/Basic/FrozenMultiMap.h"
16+
#include "swift/Basic/MultiMapCache.h"
1617
#include "swift/Basic/STLExtras.h"
1718
#include "swift/SIL/BasicBlockUtils.h"
1819
#include "swift/SIL/DebugUtils.h"
@@ -477,35 +478,13 @@ LiveRange::hasUnknownConsumingUse(bool assumingAtFixPoint) const {
477478
// Address Written To Analysis
478479
//===----------------------------------------------------------------------===//
479480

480-
namespace {
481-
482-
/// A simple analysis that checks if a specific def (in our case an inout
483-
/// argument) is ever written to. This is conservative, local and processes
484-
/// recursively downwards from def->use.
485-
struct IsAddressWrittenToDefUseAnalysis {
486-
llvm::SmallDenseMap<SILValue, bool, 8> isWrittenToCache;
487-
488-
bool operator()(SILValue value) {
489-
auto iter = isWrittenToCache.try_emplace(value, true);
490-
491-
// If we are already in the map, just return that.
492-
if (!iter.second)
493-
return iter.first->second;
494-
495-
// Otherwise, compute our value, cache it and return.
496-
bool result = isWrittenToHelper(value);
497-
iter.first->second = result;
498-
return result;
499-
}
500-
501-
private:
502-
bool isWrittenToHelper(SILValue value);
503-
};
504-
505-
} // end anonymous namespace
506-
507-
bool IsAddressWrittenToDefUseAnalysis::isWrittenToHelper(SILValue initialValue) {
481+
/// Returns true if we were able to ascertain that either the initialValue has
482+
/// no write uses or all of the write uses were writes that we could understand.
483+
static bool
484+
constructCacheValue(SILValue initialValue,
485+
SmallVectorImpl<Operand *> &wellBehavedWriteAccumulator) {
508486
SmallVector<Operand *, 8> worklist(initialValue->getUses());
487+
509488
while (!worklist.empty()) {
510489
auto *op = worklist.pop_back_val();
511490
SILInstruction *user = op->getUser();
@@ -521,35 +500,41 @@ bool IsAddressWrittenToDefUseAnalysis::isWrittenToHelper(SILValue initialValue)
521500
if (auto *oeai = dyn_cast<OpenExistentialAddrInst>(user)) {
522501
// Mutable access!
523502
if (oeai->getAccessKind() != OpenedExistentialAccess::Immutable) {
524-
return true;
503+
wellBehavedWriteAccumulator.push_back(op);
525504
}
526505

527506
// Otherwise, look through it and continue.
528507
llvm::copy(oeai->getUses(), std::back_inserter(worklist));
529508
continue;
530509
}
531510

511+
// Add any destroy_addrs to the resultAccumulator.
512+
if (isa<DestroyAddrInst>(user)) {
513+
wellBehavedWriteAccumulator.push_back(op);
514+
continue;
515+
}
516+
532517
// load_borrow and incidental uses are fine as well.
533518
if (isa<LoadBorrowInst>(user) || isIncidentalUse(user)) {
534519
continue;
535520
}
536521

537522
// Look through immutable begin_access.
538523
if (auto *bai = dyn_cast<BeginAccessInst>(user)) {
539-
// If we do not have a read, return true.
524+
// If we do not have a read, mark this as a write.
540525
if (bai->getAccessKind() != SILAccessKind::Read) {
541-
return true;
526+
wellBehavedWriteAccumulator.push_back(op);
542527
}
543528

544529
// Otherwise, add the users to the worklist and continue.
545530
llvm::copy(bai->getUses(), std::back_inserter(worklist));
546531
continue;
547532
}
548533

549-
// As long as we do not have a load [take], we are fine.
534+
// If we have a load, we just need to mark the load [take] as a write.
550535
if (auto *li = dyn_cast<LoadInst>(user)) {
551536
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
552-
return true;
537+
wellBehavedWriteAccumulator.push_back(op);
553538
}
554539
continue;
555540
}
@@ -559,41 +544,45 @@ bool IsAddressWrittenToDefUseAnalysis::isWrittenToHelper(SILValue initialValue)
559544
// interprocedural analysis that we do not perform here.
560545
if (auto fas = FullApplySite::isa(user)) {
561546
if (fas.getArgumentConvention(*op) ==
562-
SILArgumentConvention::Indirect_In_Guaranteed)
547+
SILArgumentConvention::Indirect_In_Guaranteed) {
563548
continue;
549+
}
564550

565-
// Otherwise, be conservative and return true.
566-
return true;
551+
// Otherwise, be conservative and return that we had a write that we did
552+
// not understand.
553+
return false;
567554
}
568555

569556
// Copy addr that read are just loads.
570557
if (auto *cai = dyn_cast<CopyAddrInst>(user)) {
571558
// If our value is the destination, this is a write.
572559
if (cai->getDest() == op->get()) {
573-
return true;
560+
wellBehavedWriteAccumulator.push_back(op);
561+
continue;
574562
}
575563

576564
// Ok, so we are Src by process of elimination. Make sure we are not being
577565
// taken.
578566
if (cai->isTakeOfSrc()) {
579-
return true;
567+
wellBehavedWriteAccumulator.push_back(op);
568+
continue;
580569
}
581570

582571
// Otherwise, we are safe and can continue.
583572
continue;
584573
}
585574

586575
// If we did not recognize the user, just return conservatively that it was
587-
// written to.
576+
// written to in a way we did not understand.
588577
LLVM_DEBUG(llvm::dbgs()
589578
<< "Function: " << user->getFunction()->getName() << "\n");
590579
LLVM_DEBUG(llvm::dbgs() << "Value: " << op->get());
591580
LLVM_DEBUG(llvm::dbgs() << "Unknown instruction!: " << *user);
592-
return true;
581+
return false;
593582
}
594583

595584
// Ok, we finished our worklist and this address is not being written to.
596-
return false;
585+
return true;
597586
}
598587

599588
//===----------------------------------------------------------------------===//
@@ -623,7 +612,7 @@ struct SemanticARCOptVisitor
623612
SILFunction &F;
624613
Optional<DeadEndBlocks> TheDeadEndBlocks;
625614
ValueLifetimeAnalysis::Frontier lifetimeFrontier;
626-
IsAddressWrittenToDefUseAnalysis isAddressWrittenToDefUseAnalysis;
615+
SmallMultiMapCache<SILValue, Operand *> addressToExhaustiveWriteListCache;
627616

628617
/// Are we assuming that we reached a fix point and are re-processing to
629618
/// prepare to use the phiToIncomingValueMultiMap.
@@ -658,7 +647,8 @@ struct SemanticARCOptVisitor
658647
using FrozenMultiMapRange =
659648
decltype(joinedOwnedIntroducerToConsumedOperands)::PairToSecondEltRange;
660649

661-
explicit SemanticARCOptVisitor(SILFunction &F) : F(F) {}
650+
explicit SemanticARCOptVisitor(SILFunction &F)
651+
: F(F), addressToExhaustiveWriteListCache(constructCacheValue) {}
662652

663653
DeadEndBlocks &getDeadEndBlocks() {
664654
if (!TheDeadEndBlocks)
@@ -1663,7 +1653,8 @@ class StorageGuaranteesLoadVisitor
16631653

16641654
// If we have a modify, check if our value is /ever/ written to. If it is
16651655
// never actually written to, then we convert to a load_borrow.
1666-
return answer(ARCOpt.isAddressWrittenToDefUseAnalysis(access));
1656+
auto result = ARCOpt.addressToExhaustiveWriteListCache.get(access);
1657+
return answer(!result.hasValue() || result.getValue().size());
16671658
}
16681659

16691660
void visitArgumentAccess(SILFunctionArgument *arg) {
@@ -1676,7 +1667,8 @@ class StorageGuaranteesLoadVisitor
16761667
// If we have an inout parameter that isn't ever actually written to, return
16771668
// false.
16781669
if (arg->getKnownParameterInfo().isIndirectMutating()) {
1679-
return answer(ARCOpt.isAddressWrittenToDefUseAnalysis(arg));
1670+
auto result = ARCOpt.addressToExhaustiveWriteListCache.get(arg);
1671+
return answer(!result.hasValue() || result.getValue().size());
16801672
}
16811673

16821674
// TODO: This should be extended:

0 commit comments

Comments
 (0)