From 2573e8bbfc9d34b8f7f0d3726f53722940bc50bf Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 15 Apr 2020 20:01:13 -0700 Subject: [PATCH 1/7] [opt] Support begin/end access instructions in store/load elimination. * Update the projection utility to support projecting begin_access (used by both store and load elim). * Update RedundantLoadElimination to skip begin/end access while processing instructions. It can figure out read/write info by processing the other instructions (see comment for more details). * Update DeadStoreElimination to skip begin/end access while processing instructions for the same reason as above. * Also don't process strong_release instructions. --- include/swift/SIL/Projection.h | 4 ++++ lib/SIL/Utils/InstructionUtils.cpp | 1 + lib/SIL/Utils/Projection.cpp | 15 +++++++++++++++ .../Transforms/DeadStoreElimination.cpp | 9 +++++++++ .../Transforms/RedundantLoadElimination.cpp | 8 ++++++++ 5 files changed, 37 insertions(+) diff --git a/include/swift/SIL/Projection.h b/include/swift/SIL/Projection.h index b8d66d7983634..510a0cf78d9d0 100644 --- a/include/swift/SIL/Projection.h +++ b/include/swift/SIL/Projection.h @@ -106,6 +106,7 @@ enum class ProjectionKind : unsigned { Class = PointerIntEnumIndexKindValue<3, ProjectionKind>::value, Enum = PointerIntEnumIndexKindValue<4, ProjectionKind>::value, Box = PointerIntEnumIndexKindValue<5, ProjectionKind>::value, + Access = PointerIntEnumIndexKindValue<6, ProjectionKind>::value, LastIndexKind = Enum, }; @@ -129,6 +130,7 @@ static inline bool isCastProjectionKind(ProjectionKind Kind) { case ProjectionKind::Enum: case ProjectionKind::Box: case ProjectionKind::TailElems: + case ProjectionKind::Access: return false; } } @@ -428,6 +430,7 @@ class Projection { case ProjectionKind::TailElems: case ProjectionKind::Enum: case ProjectionKind::Box: + case ProjectionKind::Access: return false; } @@ -439,6 +442,7 @@ class Projection { case ProjectionKind::Class: case ProjectionKind::Enum: case ProjectionKind::Struct: + case ProjectionKind::Access: return true; case ProjectionKind::BitwiseCast: case ProjectionKind::Index: diff --git a/lib/SIL/Utils/InstructionUtils.cpp b/lib/SIL/Utils/InstructionUtils.cpp index b51e15605b172..a745f28b8b401 100644 --- a/lib/SIL/Utils/InstructionUtils.cpp +++ b/lib/SIL/Utils/InstructionUtils.cpp @@ -31,6 +31,7 @@ SILValue swift::stripOwnershipInsts(SILValue v) { return v; case ValueKind::CopyValueInst: case ValueKind::BeginBorrowInst: + case ValueKind::BeginAccessInst: v = cast(v)->getOperand(0); } } diff --git a/lib/SIL/Utils/Projection.cpp b/lib/SIL/Utils/Projection.cpp index 2881469102ea1..d531b13cea89d 100644 --- a/lib/SIL/Utils/Projection.cpp +++ b/lib/SIL/Utils/Projection.cpp @@ -169,6 +169,11 @@ Projection::Projection(SingleValueInstruction *I) : Value() { assert(getKind() == ProjectionKind::BitwiseCast); break; } + case SILInstructionKind::BeginAccessInst: { + Value = ValueTy(ProjectionKind::Access, uintptr_t(0)); + assert(getKind() == ProjectionKind::Access); + break; + } } } @@ -196,6 +201,7 @@ SILType Projection::getType(SILType BaseType, SILModule &M, case ProjectionKind::BitwiseCast: case ProjectionKind::TailElems: return getCastType(BaseType); + case ProjectionKind::Access: case ProjectionKind::Index: // Index types do not change the underlying type. return BaseType; @@ -237,6 +243,8 @@ Projection::createObjectProjection(SILBuilder &B, SILLocation Loc, return B.createUncheckedRefCast(Loc, Base, getCastType(BaseTy)); case ProjectionKind::BitwiseCast: return B.createUncheckedBitwiseCast(Loc, Base, getCastType(BaseTy)); + case ProjectionKind::Access: + return nullptr; } llvm_unreachable("Unhandled ProjectionKind in switch."); @@ -281,6 +289,8 @@ Projection::createAddressProjection(SILBuilder &B, SILLocation Loc, case ProjectionKind::RefCast: case ProjectionKind::BitwiseCast: return B.createUncheckedAddrCast(Loc, Base, getCastType(BaseTy)); + case ProjectionKind::Access: + return nullptr; } llvm_unreachable("Unhandled ProjectionKind in switch."); @@ -835,6 +845,10 @@ SILValue Projection::getOperandForAggregate(SILInstruction *I) const { } } break; + case ProjectionKind::Access: + if (auto access = dyn_cast(I)) + return access->getOperand(); + break; case ProjectionKind::Class: case ProjectionKind::TailElems: case ProjectionKind::Box: @@ -892,6 +906,7 @@ static bool isSupportedProjection(const Projection &p) { switch (p.getKind()) { case ProjectionKind::Struct: case ProjectionKind::Tuple: + case ProjectionKind::Access: return true; case ProjectionKind::Class: case ProjectionKind::Enum: diff --git a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp index 99c4ba46cf169..bb3e4af19034c 100644 --- a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp +++ b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp @@ -161,6 +161,8 @@ static bool isDeadStoreInertInstruction(SILInstruction *Inst) { case SILInstructionKind::DeallocRefInst: case SILInstructionKind::CondFailInst: case SILInstructionKind::FixLifetimeInst: + case SILInstructionKind::EndAccessInst: + case SILInstructionKind::StrongReleaseInst: return true; default: return false; @@ -1140,6 +1142,13 @@ void DSEContext::processInstruction(SILInstruction *I, DSEKind Kind) { processStoreInst(I, Kind); } else if (isa(I)) { processDebugValueAddrInst(I, Kind); + } else if (isa(I)) { + // Do the same thing here as in RLE. Begin access writes/reads memory only + // if one of its users writes/reads memory. We will look at all of its users + // and we can project to the source memory location so, if there are any + // actual writes/reads of memory we will catch them there so, we can ignore + // begin_access here. + return; } else if (I->mayReadFromMemory()) { processUnknownReadInst(I, Kind); } diff --git a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp index e446ac4a53ec2..aceb82986756d 100644 --- a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp +++ b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp @@ -160,6 +160,7 @@ static bool isRLEInertInstruction(SILInstruction *Inst) { case SILInstructionKind::IsEscapingClosureInst: case SILInstructionKind::IsUniqueInst: case SILInstructionKind::FixLifetimeInst: + case SILInstructionKind::EndAccessInst: return true; default: return false; @@ -1089,6 +1090,13 @@ void BlockState::processInstructionWithKind(RLEContext &Ctx, return; } + // Begin access writes to memory only if one of its users writes to memory. + // We will look at all of its users and we can project to the source memory + // location so, if there are any actual writes to memory we will catch them + // there so, we can ignore begin_access here. + if (isa(Inst)) + return; + // If this instruction has side effects, but is inert from a load store // perspective, skip it. if (isRLEInertInstruction(Inst)) From 71a9c68f5a586a11d164bebc4d7050a02feb9f99 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Thu, 16 Apr 2020 22:44:00 -0700 Subject: [PATCH 2/7] Properly handle strong release in DSE --- lib/SILOptimizer/PassManager/PassPipeline.cpp | 2 ++ lib/SILOptimizer/Transforms/DeadCodeElimination.cpp | 6 ++++++ lib/SILOptimizer/Transforms/DeadStoreElimination.cpp | 11 ++++++++++- lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp | 8 +++----- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index f2d717f4f4f72..2e1f4e29839c2 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp @@ -528,6 +528,8 @@ static void addLowLevelPassPipeline(SILPassPipelinePlan &P) { P.addDeadObjectElimination(); P.addObjectOutliner(); P.addDeadStoreElimination(); + P.addDCE(); + P.addDeadObjectElimination(); // We've done a lot of optimizations on this function, attempt to FSO. P.addFunctionSignatureOpts(); diff --git a/lib/SILOptimizer/Transforms/DeadCodeElimination.cpp b/lib/SILOptimizer/Transforms/DeadCodeElimination.cpp index 263af770e1443..9c3276bca929e 100644 --- a/lib/SILOptimizer/Transforms/DeadCodeElimination.cpp +++ b/lib/SILOptimizer/Transforms/DeadCodeElimination.cpp @@ -40,6 +40,12 @@ namespace { // FIXME: Reconcile the similarities between this and // isInstructionTriviallyDead. static bool seemsUseful(SILInstruction *I) { + if (auto access = dyn_cast(I)) + return access->getSingleUse() == nullptr && !access->use_empty(); + + if (isa(I)) + return I->getOperand(0)->getSingleUse() == nullptr; + if (I->mayHaveSideEffects()) return true; diff --git a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp index bb3e4af19034c..e660da231114e 100644 --- a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp +++ b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp @@ -162,7 +162,6 @@ static bool isDeadStoreInertInstruction(SILInstruction *Inst) { case SILInstructionKind::CondFailInst: case SILInstructionKind::FixLifetimeInst: case SILInstructionKind::EndAccessInst: - case SILInstructionKind::StrongReleaseInst: return true; default: return false; @@ -1149,6 +1148,16 @@ void DSEContext::processInstruction(SILInstruction *I, DSEKind Kind) { // actual writes/reads of memory we will catch them there so, we can ignore // begin_access here. return; + } else if (auto *release = dyn_cast(I)) { + // For strong releases, we have to prove that the strong release won't + // envoke the destructor. For now, we just try to find a set_deallocating + // instruction that indicates the life-ending strong_release has been + // devirtualized. TODO: there should be a better way to do this. + for (auto *use : release->getOperand()->getUses()) { + if (isa(use->getUser())) + return; + } + processUnknownReadInst(I, Kind); } else if (I->mayReadFromMemory()) { processUnknownReadInst(I, Kind); } diff --git a/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp b/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp index 3642e337053fc..0f96c6764bfd7 100644 --- a/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp +++ b/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp @@ -70,13 +70,11 @@ void ReleaseDevirtualizer::run() { SILFunction *F = getFunction(); RCIA = PM->getAnalysis()->get(F); + // The last release_value or strong_release instruction before the + // deallocation. + SILInstruction *LastRelease = nullptr; bool Changed = false; for (SILBasicBlock &BB : *F) { - - // The last release_value or strong_release instruction before the - // deallocation. - SILInstruction *LastRelease = nullptr; - for (SILInstruction &I : BB) { if (LastRelease) { if (auto *DRI = dyn_cast(&I)) { From 0a8a5ea5f45f8598458b3a5c330e764ca11fb556 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 17 Apr 2020 21:16:42 -0700 Subject: [PATCH 3/7] Fix DSE and RLE unkown instruction handling. DSE and RLE passes used to, in most cases, bail on all tracked locations when they ran into an instruction that modified memory that they couldn't identify. In most cases this memory address can be tracked to a specific tracked location. If so, only that location has to be invalidated. --- .../swift/SILOptimizer/Utils/InstOptUtils.h | 7 ++ .../Transforms/DeadStoreElimination.cpp | 74 ++++++++++++++++-- .../Transforms/RedundantLoadElimination.cpp | 77 ++++++++++++++++--- lib/SILOptimizer/Utils/InstOptUtils.cpp | 22 ++++++ 4 files changed, 162 insertions(+), 18 deletions(-) diff --git a/include/swift/SILOptimizer/Utils/InstOptUtils.h b/include/swift/SILOptimizer/Utils/InstOptUtils.h index 16a4c3382e954..9b3cc50cc7c83 100644 --- a/include/swift/SILOptimizer/Utils/InstOptUtils.h +++ b/include/swift/SILOptimizer/Utils/InstOptUtils.h @@ -574,6 +574,13 @@ findLocalApplySites(FunctionRefBaseInst *fri); /// Gets the base implementation of a method. AbstractFunctionDecl *getBaseMethod(AbstractFunctionDecl *FD); +struct LookUpSearchResult { + SILValue objectEntry; + SILValue matched; +}; + +LookUpSearchResult lookUpForMatchingAddr(SILValue addr, SILValue match); + bool tryOptimizeApplyOfPartialApply( PartialApplyInst *pai, SILBuilderContext &builderCtxt, InstModCallbacks callbacks = InstModCallbacks()); diff --git a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp index e660da231114e..79c330e5d2091 100644 --- a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp +++ b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp @@ -162,6 +162,7 @@ static bool isDeadStoreInertInstruction(SILInstruction *Inst) { case SILInstructionKind::CondFailInst: case SILInstructionKind::FixLifetimeInst: case SILInstructionKind::EndAccessInst: + case SILInstructionKind::SetDeallocatingInst: return true; default: return false; @@ -1085,25 +1086,77 @@ void DSEContext::processDebugValueAddrInst(SILInstruction *I, DSEKind Kind) { void DSEContext::processUnknownReadInstForGenKillSet(SILInstruction *I) { BlockState *S = getBlockState(I); + SmallVector toStopTracking; + bool unknownWrite = false; for (unsigned i = 0; i < S->LocationNum; ++i) { if (!S->BBMaxStoreSet.test(i)) continue; - if (!AA->mayReadFromMemory(I, LocationVault[i].getBase())) - continue; - // Update the genset and kill set. - S->startTrackingLocation(S->BBKillSet, i); - S->stopTrackingLocation(S->BBGenSet, i); + auto val = LocationVault[i].getBase(); + for (auto &op : I->getAllOperands()) { + if (!op.get()->getType().isAddress()) + continue; + auto projection = lookUpForMatchingAddr(op.get(), val); + if (projection.matched) { + if (!AA->mayWriteToMemory(I, val)) + continue; + toStopTracking.push_back(i); + } else { // if (!projection.objectEntry) + unknownWrite = true; + } + } + } + + if (unknownWrite) { + for (unsigned i = 0; i < S->LocationNum; ++i) { + if (!S->BBMaxStoreSet.test(i)) + continue; + S->startTrackingLocation(S->BBKillSet, i); + S->stopTrackingLocation(S->BBGenSet, i); + } + } else { + for (auto i : toStopTracking) { + if (!S->BBMaxStoreSet.test(i)) + continue; + S->startTrackingLocation(S->BBKillSet, i); + S->stopTrackingLocation(S->BBGenSet, i); + } } } void DSEContext::processUnknownReadInstForDSE(SILInstruction *I) { BlockState *S = getBlockState(I); + SmallVector toStopTracking; + bool unknownWrite = false; for (unsigned i = 0; i < S->LocationNum; ++i) { if (!S->isTrackingLocation(S->BBWriteSetMid, i)) continue; - if (!AA->mayReadFromMemory(I, LocationVault[i].getBase())) - continue; - S->stopTrackingLocation(S->BBWriteSetMid, i); + auto val = LocationVault[i].getBase(); + for (auto &op : I->getAllOperands()) { + if (!op.get()->getType().isAddress()) + continue; + auto projection = lookUpForMatchingAddr(op.get(), val); + if (projection.matched) { + if (!AA->mayWriteToMemory(I, val)) + continue; + toStopTracking.push_back(i); + } else { // if (!projection.objectEntry) + unknownWrite = true; + } + } + } + + if (unknownWrite) { + for (unsigned i = 0; i < S->LocationNum; ++i) { + if (!S->isTrackingLocation(S->BBWriteSetMid, i)) + continue; + S->stopTrackingLocation(S->BBWriteSetMid, i); + } + } else { + for (auto i : toStopTracking) { + if (!S->isTrackingLocation(S->BBWriteSetMid, i)) + continue; + S->stopTrackingLocation(S->BBWriteSetMid, i); + } } } @@ -1149,6 +1202,11 @@ void DSEContext::processInstruction(SILInstruction *I, DSEKind Kind) { // begin_access here. return; } else if (auto *release = dyn_cast(I)) { + // If the strong release operand type can't have a custom destructor it's + // OK. + if (release->getOperand()->getType().getClassOrBoundGenericClass() == + nullptr) + return; // For strong releases, we have to prove that the strong release won't // envoke the destructor. For now, we just try to find a set_deallocating // instruction that indicates the life-ending strong_release has been diff --git a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp index aceb82986756d..23cf875308337 100644 --- a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp +++ b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp @@ -161,6 +161,8 @@ static bool isRLEInertInstruction(SILInstruction *Inst) { case SILInstructionKind::IsUniqueInst: case SILInstructionKind::FixLifetimeInst: case SILInstructionKind::EndAccessInst: + case SILInstructionKind::SetDeallocatingInst: + case SILInstructionKind::DeallocRefInst: return true; default: return false; @@ -968,6 +970,8 @@ void BlockState::processLoadInst(RLEContext &Ctx, LoadInst *LI, RLEKind Kind) { void BlockState::processUnknownWriteInstForGenKillSet(RLEContext &Ctx, SILInstruction *I) { auto *AA = Ctx.getAA(); + SmallVector toStopTracking; + bool unknownWrite = false; for (unsigned i = 0; i < LocationNum; ++i) { if (!isTrackingLocation(ForwardSetMax, i)) continue; @@ -976,17 +980,42 @@ void BlockState::processUnknownWriteInstForGenKillSet(RLEContext &Ctx, // TODO: checking may alias with Base is overly conservative, // we should check may alias with base plus projection path. LSLocation &R = Ctx.getLocation(i); - if (!AA->mayWriteToMemory(I, R.getBase())) - continue; - // MayAlias. - stopTrackingLocation(BBGenSet, i); - startTrackingLocation(BBKillSet, i); + for (auto &op : I->getAllOperands()) { + if (!op.get()->getType().isAddress()) + continue; + auto projection = lookUpForMatchingAddr(op.get(), R.getBase()); + if (projection.matched) { + if (!AA->mayWriteToMemory(I, R.getBase())) + continue; + toStopTracking.push_back(i); + } else { // if (!projection.objectEntry) + unknownWrite = true; + } + } + } + + if (unknownWrite) { + for (unsigned i = 0; i < LocationNum; ++i) { + if (!isTrackingLocation(ForwardSetMax, i)) + continue; + stopTrackingLocation(BBGenSet, i); + startTrackingLocation(BBKillSet, i); + } + } else { + for (auto i : toStopTracking) { + if (!isTrackingLocation(ForwardSetMax, i)) + continue; + stopTrackingLocation(BBGenSet, i); + startTrackingLocation(BBKillSet, i); + } } } void BlockState::processUnknownWriteInstForRLE(RLEContext &Ctx, SILInstruction *I) { auto *AA = Ctx.getAA(); + SmallVector toStopTracking; + bool unknownWrite = false; for (unsigned i = 0; i < LocationNum; ++i) { if (!isTrackingLocation(ForwardSetIn, i)) continue; @@ -995,11 +1024,34 @@ void BlockState::processUnknownWriteInstForRLE(RLEContext &Ctx, // TODO: checking may alias with Base is overly conservative, // we should check may alias with base plus projection path. LSLocation &R = Ctx.getLocation(i); - if (!AA->mayWriteToMemory(I, R.getBase())) - continue; - // MayAlias. - stopTrackingLocation(ForwardSetIn, i); - stopTrackingValue(ForwardValIn, i); + for (auto &op : I->getAllOperands()) { + if (!op.get()->getType().isAddress()) + continue; + auto projection = lookUpForMatchingAddr(op.get(), R.getBase()); + if (projection.matched) { + if (!AA->mayWriteToMemory(I, R.getBase())) + continue; + toStopTracking.push_back(i); + } else { // if (!projection.objectEntry) + unknownWrite = true; + } + } + } + + if (unknownWrite) { + for (unsigned i = 0; i < LocationNum; ++i) { + if (!isTrackingLocation(ForwardSetIn, i)) + continue; + stopTrackingLocation(ForwardSetIn, i); + stopTrackingValue(ForwardValIn, i); + } + } else { + for (auto i : toStopTracking) { + if (!isTrackingLocation(ForwardSetIn, i)) + continue; + stopTrackingLocation(ForwardSetIn, i); + stopTrackingValue(ForwardValIn, i); + } } } @@ -1097,6 +1149,11 @@ void BlockState::processInstructionWithKind(RLEContext &Ctx, if (isa(Inst)) return; + // If the load is valid, then this strong release won't invoke the destructor. + // So we can ignore it. + if (isa(Inst)) + return; + // If this instruction has side effects, but is inert from a load store // perspective, skip it. if (isRLEInertInstruction(Inst)) diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp index a64eb247ff81a..79f69430abaaf 100644 --- a/lib/SILOptimizer/Utils/InstOptUtils.cpp +++ b/lib/SILOptimizer/Utils/InstOptUtils.cpp @@ -2008,3 +2008,25 @@ AbstractFunctionDecl *swift::getBaseMethod(AbstractFunctionDecl *FD) { } return FD; } + +LookUpSearchResult swift::lookUpForMatchingAddr(SILValue addr, SILValue match) { + if (!addr.getDefiningInstruction()) + return {nullptr, nullptr}; + + for (;;) { + if (addr == match) + return {nullptr, addr}; + switch (addr->getKind()) { + case ValueKind::RefElementAddrInst: + case ValueKind::StructElementAddrInst: + case ValueKind::TupleElementAddrInst: + addr = addr.getDefiningInstruction()->getOperand(0); + break; + case ValueKind::AllocRefInst: + case ValueKind::AllocStackInst: + return {addr, nullptr}; + default: + return {nullptr, nullptr}; + } + } +} From 65e2a78b9cd5e2b72c72b3dc800f378ba85a2c2d Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sat, 18 Apr 2020 13:46:32 -0700 Subject: [PATCH 4/7] Fix stack promotion --- lib/SILOptimizer/Transforms/StackPromotion.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/SILOptimizer/Transforms/StackPromotion.cpp b/lib/SILOptimizer/Transforms/StackPromotion.cpp index a7579edc39094..e86ca2b5ce500 100644 --- a/lib/SILOptimizer/Transforms/StackPromotion.cpp +++ b/lib/SILOptimizer/Transforms/StackPromotion.cpp @@ -140,6 +140,12 @@ bool StackPromotion::tryPromoteAlloc(AllocRefInst *ARI, EscapeAnalysis *EA, LLVM_DEBUG(llvm::dbgs() << " uses don't post-dom allocation -> don't promote"); return false; } + + if (Frontier.empty()) { + // TODO: assert(false); + return false; + } + NumStackPromoted++; // We set the [stack] attribute in the alloc_ref. From 44ade89dcbe971565e0488c5850bccc592ade096 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sat, 18 Apr 2020 21:15:05 -0700 Subject: [PATCH 5/7] Use isNoAlias when checking if we have to invalidate instructions --- .../single-source/ObjectAllocation.swift | 84 +++---------------- .../Transforms/DeadStoreElimination.cpp | 74 ++++------------ .../Transforms/RedundantLoadElimination.cpp | 78 ++++------------- 3 files changed, 44 insertions(+), 192 deletions(-) diff --git a/benchmark/single-source/ObjectAllocation.swift b/benchmark/single-source/ObjectAllocation.swift index 1320ac5771472..2eb019e53fd60 100644 --- a/benchmark/single-source/ObjectAllocation.swift +++ b/benchmark/single-source/ObjectAllocation.swift @@ -22,6 +22,11 @@ public var ObjectAllocation = BenchmarkInfo( tags: [.runtime, .cpubench] ) +@inline(never) +func dummy(_ x: T) -> T{ + return x +} + final class XX { var xx: Int @@ -40,37 +45,11 @@ final class TreeNode { } } -final class LinkedNode { - var next: LinkedNode? - var xx: Int - - init(_ x: Int, _ n: LinkedNode?) { - xx = x - next = n - } -} - -@inline(never) -func getInt(_ x: XX) -> Int { - return x.xx -} - -@inline(never) -func testSingleObject() -> Int { - var s = 0 - for i in 0..<1000 { - let x = XX(i) - s += getInt(x) - } - return s -} - @inline(never) func addInts(_ t: TreeNode) -> Int { return t.left.xx + t.right.xx } -@inline(never) func testTree() -> Int { var s = 0 for i in 0..<300 { @@ -80,60 +59,17 @@ func testTree() -> Int { return s } -@inline(never) -func addAllInts(_ n: LinkedNode) -> Int { - var s = 0 - var iter: LinkedNode? = n - while let iter2 = iter { - s += iter2.xx - iter = iter2.next - } - return s -} - -@inline(never) -func testList() -> Int { - var s = 0 - for i in 0..<250 { - let l = LinkedNode(i, LinkedNode(27, LinkedNode(42, nil))) - s += addAllInts(l) - } - return s -} - -@inline(never) -func identity(_ x: Int) -> Int { - return x -} - -@inline(never) -func testArray() -> Int { - var s = 0 - for _ in 0..<1000 { - for i in [0, 1, 2] { - s += identity(i) - } - } - return s -} - @inline(never) public func run_ObjectAllocation(_ N: Int) { - var SingleObjectResult = 0 var TreeResult = 0 - var ListResult = 0 - var ArrayResult = 0 + for _ in 0.. toStopTracking; - bool unknownWrite = false; for (unsigned i = 0; i < S->LocationNum; ++i) { if (!S->BBMaxStoreSet.test(i)) continue; auto val = LocationVault[i].getBase(); - for (auto &op : I->getAllOperands()) { - if (!op.get()->getType().isAddress()) - continue; - auto projection = lookUpForMatchingAddr(op.get(), val); - if (projection.matched) { - if (!AA->mayWriteToMemory(I, val)) - continue; - toStopTracking.push_back(i); - } else { // if (!projection.objectEntry) - unknownWrite = true; - } - } - } - - if (unknownWrite) { - for (unsigned i = 0; i < S->LocationNum; ++i) { - if (!S->BBMaxStoreSet.test(i)) - continue; - S->startTrackingLocation(S->BBKillSet, i); - S->stopTrackingLocation(S->BBGenSet, i); - } - } else { - for (auto i : toStopTracking) { - if (!S->BBMaxStoreSet.test(i)) - continue; - S->startTrackingLocation(S->BBKillSet, i); - S->stopTrackingLocation(S->BBGenSet, i); - } + if (!AA->mayReadFromMemory(I, val)) + continue; + if (llvm::all_of(I->getAllOperands(), [&AA = AA, &val](Operand &op) { + return AA->isNoAlias(op.get(), val); + })) + continue; + // Update the genset and kill set. + S->startTrackingLocation(S->BBKillSet, i); + S->stopTrackingLocation(S->BBGenSet, i); } } void DSEContext::processUnknownReadInstForDSE(SILInstruction *I) { BlockState *S = getBlockState(I); - SmallVector toStopTracking; - bool unknownWrite = false; for (unsigned i = 0; i < S->LocationNum; ++i) { if (!S->isTrackingLocation(S->BBWriteSetMid, i)) continue; auto val = LocationVault[i].getBase(); - for (auto &op : I->getAllOperands()) { - if (!op.get()->getType().isAddress()) - continue; - auto projection = lookUpForMatchingAddr(op.get(), val); - if (projection.matched) { - if (!AA->mayWriteToMemory(I, val)) - continue; - toStopTracking.push_back(i); - } else { // if (!projection.objectEntry) - unknownWrite = true; - } - } - } - - if (unknownWrite) { - for (unsigned i = 0; i < S->LocationNum; ++i) { - if (!S->isTrackingLocation(S->BBWriteSetMid, i)) - continue; - S->stopTrackingLocation(S->BBWriteSetMid, i); - } - } else { - for (auto i : toStopTracking) { - if (!S->isTrackingLocation(S->BBWriteSetMid, i)) - continue; - S->stopTrackingLocation(S->BBWriteSetMid, i); - } + if (!AA->mayReadFromMemory(I, val)) + continue; + if (llvm::all_of(I->getAllOperands(), [&AA = AA, &val](Operand &op) { + return AA->isNoAlias(op.get(), val); + })) + continue; + S->stopTrackingLocation(S->BBWriteSetMid, i); } } diff --git a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp index 23cf875308337..fa71ea6838c8f 100644 --- a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp +++ b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp @@ -970,8 +970,6 @@ void BlockState::processLoadInst(RLEContext &Ctx, LoadInst *LI, RLEKind Kind) { void BlockState::processUnknownWriteInstForGenKillSet(RLEContext &Ctx, SILInstruction *I) { auto *AA = Ctx.getAA(); - SmallVector toStopTracking; - bool unknownWrite = false; for (unsigned i = 0; i < LocationNum; ++i) { if (!isTrackingLocation(ForwardSetMax, i)) continue; @@ -980,42 +978,21 @@ void BlockState::processUnknownWriteInstForGenKillSet(RLEContext &Ctx, // TODO: checking may alias with Base is overly conservative, // we should check may alias with base plus projection path. LSLocation &R = Ctx.getLocation(i); - for (auto &op : I->getAllOperands()) { - if (!op.get()->getType().isAddress()) - continue; - auto projection = lookUpForMatchingAddr(op.get(), R.getBase()); - if (projection.matched) { - if (!AA->mayWriteToMemory(I, R.getBase())) - continue; - toStopTracking.push_back(i); - } else { // if (!projection.objectEntry) - unknownWrite = true; - } - } - } - - if (unknownWrite) { - for (unsigned i = 0; i < LocationNum; ++i) { - if (!isTrackingLocation(ForwardSetMax, i)) - continue; - stopTrackingLocation(BBGenSet, i); - startTrackingLocation(BBKillSet, i); - } - } else { - for (auto i : toStopTracking) { - if (!isTrackingLocation(ForwardSetMax, i)) - continue; - stopTrackingLocation(BBGenSet, i); - startTrackingLocation(BBKillSet, i); - } + if (!AA->mayWriteToMemory(I, R.getBase())) + continue; + if (llvm::all_of(I->getAllOperands(), [&AA, &R](Operand &op) { + return AA->isNoAlias(op.get(), R.getBase()); + })) + continue; + // MayAlias. + stopTrackingLocation(BBGenSet, i); + startTrackingLocation(BBKillSet, i); } } void BlockState::processUnknownWriteInstForRLE(RLEContext &Ctx, SILInstruction *I) { auto *AA = Ctx.getAA(); - SmallVector toStopTracking; - bool unknownWrite = false; for (unsigned i = 0; i < LocationNum; ++i) { if (!isTrackingLocation(ForwardSetIn, i)) continue; @@ -1024,34 +1001,15 @@ void BlockState::processUnknownWriteInstForRLE(RLEContext &Ctx, // TODO: checking may alias with Base is overly conservative, // we should check may alias with base plus projection path. LSLocation &R = Ctx.getLocation(i); - for (auto &op : I->getAllOperands()) { - if (!op.get()->getType().isAddress()) - continue; - auto projection = lookUpForMatchingAddr(op.get(), R.getBase()); - if (projection.matched) { - if (!AA->mayWriteToMemory(I, R.getBase())) - continue; - toStopTracking.push_back(i); - } else { // if (!projection.objectEntry) - unknownWrite = true; - } - } - } - - if (unknownWrite) { - for (unsigned i = 0; i < LocationNum; ++i) { - if (!isTrackingLocation(ForwardSetIn, i)) - continue; - stopTrackingLocation(ForwardSetIn, i); - stopTrackingValue(ForwardValIn, i); - } - } else { - for (auto i : toStopTracking) { - if (!isTrackingLocation(ForwardSetIn, i)) - continue; - stopTrackingLocation(ForwardSetIn, i); - stopTrackingValue(ForwardValIn, i); - } + if (!AA->mayWriteToMemory(I, R.getBase())) + continue; + if (llvm::all_of(I->getAllOperands(), [&AA, &R](Operand &op) { + return AA->isNoAlias(op.get(), R.getBase()); + })) + continue; + // MayAlias. + stopTrackingLocation(ForwardSetIn, i); + stopTrackingValue(ForwardValIn, i); } } From 659f49379138b0b23863f35e7dd34862c908f701 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 10:21:10 -0700 Subject: [PATCH 6/7] Store sources are unkown reads if the source is a reference type --- lib/SILOptimizer/Transforms/DeadStoreElimination.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp index 3ae3e287a82c1..b298ffe17b711 100644 --- a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp +++ b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp @@ -890,8 +890,13 @@ bool DSEContext::processWriteForDSE(BlockState *S, unsigned bit) { continue; // If 2 locations may alias, we can still keep both stores. LSLocation &L = LocationVault[i]; - if (!L.isMustAliasLSLocation(R, AA)) + if (!L.isMustAliasLSLocation(R, AA)) { + // If this is a reference type, then the *live* store counts as an unkown + // read. + if (L.getBase()->getType().isAnyClassReferenceType()) + S->stopTrackingLocation(S->BBWriteSetMid, i); continue; + } // There is a must alias store. No need to check further. StoreDead = true; break; From e1febb4776ea6f148fc41aab7169e33f752d0f4a Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 10:35:07 -0700 Subject: [PATCH 7/7] Reset no-longer-needed changes --- .../single-source/ObjectAllocation.swift | 84 ++++++++++++++++--- .../swift/SILOptimizer/Utils/InstOptUtils.h | 7 -- lib/SILOptimizer/Utils/InstOptUtils.cpp | 22 ----- 3 files changed, 74 insertions(+), 39 deletions(-) diff --git a/benchmark/single-source/ObjectAllocation.swift b/benchmark/single-source/ObjectAllocation.swift index 2eb019e53fd60..1320ac5771472 100644 --- a/benchmark/single-source/ObjectAllocation.swift +++ b/benchmark/single-source/ObjectAllocation.swift @@ -22,11 +22,6 @@ public var ObjectAllocation = BenchmarkInfo( tags: [.runtime, .cpubench] ) -@inline(never) -func dummy(_ x: T) -> T{ - return x -} - final class XX { var xx: Int @@ -45,11 +40,37 @@ final class TreeNode { } } +final class LinkedNode { + var next: LinkedNode? + var xx: Int + + init(_ x: Int, _ n: LinkedNode?) { + xx = x + next = n + } +} + +@inline(never) +func getInt(_ x: XX) -> Int { + return x.xx +} + +@inline(never) +func testSingleObject() -> Int { + var s = 0 + for i in 0..<1000 { + let x = XX(i) + s += getInt(x) + } + return s +} + @inline(never) func addInts(_ t: TreeNode) -> Int { return t.left.xx + t.right.xx } +@inline(never) func testTree() -> Int { var s = 0 for i in 0..<300 { @@ -59,17 +80,60 @@ func testTree() -> Int { return s } +@inline(never) +func addAllInts(_ n: LinkedNode) -> Int { + var s = 0 + var iter: LinkedNode? = n + while let iter2 = iter { + s += iter2.xx + iter = iter2.next + } + return s +} + +@inline(never) +func testList() -> Int { + var s = 0 + for i in 0..<250 { + let l = LinkedNode(i, LinkedNode(27, LinkedNode(42, nil))) + s += addAllInts(l) + } + return s +} + +@inline(never) +func identity(_ x: Int) -> Int { + return x +} + +@inline(never) +func testArray() -> Int { + var s = 0 + for _ in 0..<1000 { + for i in [0, 1, 2] { + s += identity(i) + } + } + return s +} + @inline(never) public func run_ObjectAllocation(_ N: Int) { + var SingleObjectResult = 0 var TreeResult = 0 - + var ListResult = 0 + var ArrayResult = 0 for _ in 0..getKind()) { - case ValueKind::RefElementAddrInst: - case ValueKind::StructElementAddrInst: - case ValueKind::TupleElementAddrInst: - addr = addr.getDefiningInstruction()->getOperand(0); - break; - case ValueKind::AllocRefInst: - case ValueKind::AllocStackInst: - return {addr, nullptr}; - default: - return {nullptr, nullptr}; - } - } -}