From 200b5e6d958133797529c09c77b84bd733c7f07e Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 12:29:03 -0700 Subject: [PATCH 1/6] [LVA] Support begin_access in projections. Adds support for projecting through begin_access instructions. --- include/swift/SIL/Projection.h | 4 ++++ lib/SIL/Utils/InstructionUtils.cpp | 1 + lib/SIL/Utils/Projection.cpp | 15 +++++++++++++++ 3 files changed, 20 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: From d6bdde0dc318bc4fc0dce98b66162b5cc05c19ab Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 13:11:27 -0700 Subject: [PATCH 2/6] [opt] Add dead object elimination pass after dead store elimination. After low-level SSA passes, often the only use of an object will be dead stores. At some point after dead store elimination, dead object elimination is useful. The dead code elimination pass is fast and removes some remaining uses of the object after DSE (such as dead access instructions). --- lib/SILOptimizer/PassManager/PassPipeline.cpp | 2 ++ 1 file changed, 2 insertions(+) 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(); From 167d992f6985ce5317f2dc903a680ea1b29a67ea Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 12:36:35 -0700 Subject: [PATCH 3/6] [LVA] Support access instructions in DCE. Simple check for the following pattern: x = begin_access end_access x --- .../Transforms/DeadCodeElimination.cpp | 6 ++++++ test/SILOptimizer/dead_code_elimination.sil | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) 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/test/SILOptimizer/dead_code_elimination.sil b/test/SILOptimizer/dead_code_elimination.sil index 07db0a890b3cf..3e93dbc4e2986 100644 --- a/test/SILOptimizer/dead_code_elimination.sil +++ b/test/SILOptimizer/dead_code_elimination.sil @@ -259,3 +259,21 @@ bb2: bb3: br bb1 } + +// Check that DCE eliminates dead access instructions. +// CHECK-LABEL: sil @dead_access +// CHECK: bb0 +// CHECK-NEXT: tuple +// CHECK-NEXT: return +// CHECK-LABEL: end sil function 'dead_access' +sil @dead_access : $@convention(thin) (@in Container) -> () { +bb0(%0 : $*Container): + %1 = begin_access [modify] [dynamic] %0 : $*Container + end_access %1 : $*Container + + %3 = begin_access [read] [static] %0 : $*Container + end_access %3 : $*Container + + %999 = tuple () + return %999 : $() +} From 8d946d8bc5cf041639d6fa2a20a9afdfe08410f2 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 12:50:13 -0700 Subject: [PATCH 4/6] [LVA] Update ignore instructions RLE and DSE. RLE can ignore end_access, set_deallocating, dealloc_ref, begin_access, and strong_release instructions. DSE can ignore end_access, set_deallocating, begin_access, and sometimes strong_Release. --- .../Transforms/DeadStoreElimination.cpp | 24 +++++++++++++++++++ .../Transforms/RedundantLoadElimination.cpp | 15 ++++++++++++ test/SILOptimizer/dead_store_elim.sil | 22 +++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp index 99c4ba46cf169..c51230a319168 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::SetDeallocatingInst: return true; default: return false; @@ -1140,6 +1142,28 @@ 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 (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 + // 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/RedundantLoadElimination.cpp b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp index e446ac4a53ec2..a558c38b58c66 100644 --- a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp +++ b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp @@ -160,6 +160,9 @@ static bool isRLEInertInstruction(SILInstruction *Inst) { case SILInstructionKind::IsEscapingClosureInst: case SILInstructionKind::IsUniqueInst: case SILInstructionKind::FixLifetimeInst: + case SILInstructionKind::EndAccessInst: + case SILInstructionKind::SetDeallocatingInst: + case SILInstructionKind::DeallocRefInst: return true; default: return false; @@ -1089,6 +1092,18 @@ 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 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/test/SILOptimizer/dead_store_elim.sil b/test/SILOptimizer/dead_store_elim.sil index 796667e1df31f..dc438e723a5f7 100644 --- a/test/SILOptimizer/dead_store_elim.sil +++ b/test/SILOptimizer/dead_store_elim.sil @@ -1501,3 +1501,25 @@ bb0(%0 : $*foo): %20 = tuple() return %20 : $() } + +// Check that begin_access, end_access, strong_release, set_deallocating, and dealloc_ref don't prevent optimization. +// CHECK-LABEL: ignore_read_write +// CHECK: bb0 +// CHECK-NOT: store +// CHECK-LABEL: end sil function 'ignore_read_write' +sil @ignore_read_write : $@convention(thin) () -> Int { +bb0: + %1 = alloc_ref [stack] $foo + strong_retain %1 : $foo + %2 = integer_literal $Builtin.Int64, 0 + %3 = struct $Int (%2 : $Builtin.Int64) + %4 = ref_element_addr %1 : $foo, #foo.a + %5 = begin_access [modify] [dynamic] [no_nested_conflict] %4 : $*Int + store %3 to %5 : $*Int + end_access %5 : $*Int + strong_release %1 : $foo + set_deallocating %1 : $foo + dealloc_ref %1 : $foo + dealloc_ref [stack] %1 : $foo + return %3 : $Int +} From 22c1f89ca9a04e39cca8e170fa20a0caa5112ca8 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 19 Apr 2020 13:07:05 -0700 Subject: [PATCH 5/6] [opt] Move "LastRelease" outside BB iteration. This means that "LastRelease" can be tracked through multiple basic blocks. --- .../Transforms/ReleaseDevirtualizer.cpp | 8 +++---- test/SILOptimizer/devirt_release.sil | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) 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)) { diff --git a/test/SILOptimizer/devirt_release.sil b/test/SILOptimizer/devirt_release.sil index f7eef9d24fd61..731f4b50dcf34 100644 --- a/test/SILOptimizer/devirt_release.sil +++ b/test/SILOptimizer/devirt_release.sil @@ -93,6 +93,27 @@ bb0: return %r : $() } +// CHECK-LABEL: sil @devirtualize_release_multiblock +// CHECK-NOT: strong_release +// CHECK: [[A:%[0-9]+]] = alloc_ref +// CHECK-NEXT: set_deallocating [[A]] +// CHECK: [[D:%[0-9]+]] = function_ref @$s4test1BCfD +// CHECK-NEXT: apply [[D]]([[A]]) +// CHECK-NEXT: br +// CHECK: bb1: +// CHECK-NEXT: dealloc_ref [stack] [[A]] +// CHECK: return +sil @devirtualize_release_multiblock : $@convention(thin) () -> () { +bb0: + %1 = alloc_ref [stack] $B + strong_release %1 : $B + br bb1 + +bb1: + dealloc_ref [stack] %1 : $B + %r = tuple () + return %r : $() +} sil @unknown_func : $@convention(thin) () -> () From ad4a8935019d363b5c53fd5c00c3297b2878849a Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 22 Apr 2020 10:51:14 -0700 Subject: [PATCH 6/6] Remove last release devirtualizer change --- lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp b/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp index 0f96c6764bfd7..3642e337053fc 100644 --- a/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp +++ b/lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp @@ -70,11 +70,13 @@ 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)) {