From a6bed21d9e99b1692160d42dcbf11d5324a36329 Mon Sep 17 00:00:00 2001 From: Ravi Kandhadai Date: Mon, 3 Feb 2020 18:49:20 -0800 Subject: [PATCH] [SIL Optimization] Make ArraySemantics.cpp aware of "array.uninitialized_intrinsic" semantics attribute that is used by the top-level array initializer (in ArrayShared.swift), which is the entry point used by the compiler to initialize array from array literals. This initializer is early-inlined so that other optimizations can work on its body. Fix DeadObjectElimination and ArrayCOWOpts optimization passes to work with this semantics attribute in addition to "array.uninitialized", which they already use. Refactor mapInitializationStores function from ArrayElementValuePropagation.cpp to ArraySemantic.cpp so that the array-initialization pattern matching functionality implemented by the function can be reused by other optimizations. --- .../SILOptimizer/Analysis/ArraySemantic.h | 18 +- lib/SILOptimizer/Analysis/ArraySemantic.cpp | 169 +++++++++++++----- .../LoopTransforms/COWArrayOpt.cpp | 4 +- .../ArrayElementValuePropagation.cpp | 74 ++------ .../Transforms/DeadObjectElimination.cpp | 7 +- test/SILOptimizer/dead_array_elim.sil | 2 +- test/SILOptimizer/dead_array_elim.swift | 40 +++++ 7 files changed, 198 insertions(+), 116 deletions(-) create mode 100644 test/SILOptimizer/dead_array_elim.swift diff --git a/include/swift/SILOptimizer/Analysis/ArraySemantic.h b/include/swift/SILOptimizer/Analysis/ArraySemantic.h index fcca204fd1362..0dbe35c940843 100644 --- a/include/swift/SILOptimizer/Analysis/ArraySemantic.h +++ b/include/swift/SILOptimizer/Analysis/ArraySemantic.h @@ -41,7 +41,8 @@ enum class ArrayCallKind { // a function, and it has a self parameter, make sure that it is defined // before this comment. kArrayInit, - kArrayUninitialized + kArrayUninitialized, + kArrayUninitializedIntrinsic }; /// Wrapper around array semantic calls. @@ -183,6 +184,21 @@ class ArraySemanticsCall { /// Can this function be inlined by the early inliner. bool canInlineEarly() const; + /// If this is a call to ArrayUninitialized (or + /// ArrayUninitializedInstrinsic), identify the instructions that store + /// elements into the array indices. For every index, add the store + /// instruction that stores to that index to \p ElementStoreMap. + /// + /// \returns true iff this is an "array.uninitialized" semantic call, and the + /// stores into the array indices are identified and the \p ElementStoreMap is + /// populated. + /// + /// Note that this function does not support array initializations that use + /// copy_addr, which implies that arrays with address-only types would not + /// be recognized by this function as yet. + bool mapInitializationStores( + llvm::DenseMap &ElementStoreMap); + protected: /// Validate the signature of this call. bool isValidSignature(); diff --git a/lib/SILOptimizer/Analysis/ArraySemantic.cpp b/lib/SILOptimizer/Analysis/ArraySemantic.cpp index f58bbb4885b22..7ccb82ab44774 100644 --- a/lib/SILOptimizer/Analysis/ArraySemantic.cpp +++ b/lib/SILOptimizer/Analysis/ArraySemantic.cpp @@ -170,6 +170,7 @@ ArrayCallKind swift::ArraySemanticsCall::getKind() const { ArrayCallKind::kArrayPropsIsNativeTypeChecked) .StartsWith("array.init", ArrayCallKind::kArrayInit) .Case("array.uninitialized", ArrayCallKind::kArrayUninitialized) + .Case("array.uninitialized_intrinsic", ArrayCallKind::kArrayUninitializedIntrinsic) .Case("array.check_subscript", ArrayCallKind::kCheckSubscript) .Case("array.check_index", ArrayCallKind::kCheckIndex) .Case("array.get_count", ArrayCallKind::kGetCount) @@ -591,11 +592,15 @@ bool swift::ArraySemanticsCall::canInlineEarly() const { case ArrayCallKind::kAppendContentsOf: case ArrayCallKind::kReserveCapacityForAppend: case ArrayCallKind::kAppendElement: + case ArrayCallKind::kArrayUninitializedIntrinsic: // append(Element) calls other semantics functions. Therefore it's // important that it's inlined by the early inliner (which is before all // the array optimizations). Also, this semantics is only used to lookup // Array.append(Element), so inlining it does not prevent any other // optimization. + // + // Early inlining array.uninitialized_intrinsic semantic call helps in + // stack promotion. return true; } } @@ -622,61 +627,57 @@ SILValue swift::ArraySemanticsCall::getInitializationCount() const { return SILValue(); } -SILValue swift::ArraySemanticsCall::getArrayValue() const { - if (getKind() == ArrayCallKind::kArrayUninitialized) { - TupleExtractInst *ArrayDef = nullptr; - for (auto *Op : SemanticsCall->getUses()) { - auto *TupleElt = dyn_cast(Op->getUser()); - if (!TupleElt) - return SILValue(); - switch (TupleElt->getFieldNo()) { - default: - return SILValue(); - case 0: { - // Should only have one tuple extract after CSE. - if (ArrayDef) - return SILValue(); - ArrayDef = TupleElt; - break; - } - case 1: /*Ignore the storage address */ break; - } +/// Given an array semantic call \c arrayCall, if it is an "array.uninitialized" +/// initializer, which returns a two-element tuple, return the element of the +/// tuple at \c tupleElementIndex. Return a null SILValue if the +/// array call is not an "array.uninitialized" initializer or if the extraction +/// of the result tuple fails. +static SILValue getArrayUninitializedInitResult(ArraySemanticsCall arrayCall, + unsigned tupleElementIndex) { + assert(tupleElementIndex <= 1 && "tupleElementIndex must be 0 or 1"); + ArrayCallKind arrayCallKind = arrayCall.getKind(); + if (arrayCallKind != ArrayCallKind::kArrayUninitialized && + arrayCallKind != ArrayCallKind::kArrayUninitializedIntrinsic) + return SILValue(); + + // In OSSA, the call result will be extracted through a destructure_tuple + // instruction. + ApplyInst *callInst = arrayCall; + if (callInst->getFunction()->hasOwnership()) { + Operand *singleUse = callInst->getSingleUse(); + if (!singleUse) + return SILValue(); + if (DestructureTupleInst *destructTuple = + dyn_cast(singleUse->getUser())) { + return destructTuple->getResult(tupleElementIndex); } - return SILValue(ArrayDef); + return SILValue(); + } + + // In non-OSSA, look for a tuple_extract instruction of the call result with + // the requested tupleElementIndex. + TupleExtractInst *tupleExtractInst = nullptr; + for (auto *op : callInst->getUses()) { + auto *tupleElt = dyn_cast(op->getUser()); + if (!tupleElt) + return SILValue(); + if (tupleElt->getFieldNo() != tupleElementIndex) + continue; + tupleExtractInst = tupleElt; + break; } + return SILValue(tupleExtractInst); +} - if (getKind() == ArrayCallKind::kArrayInit) +SILValue swift::ArraySemanticsCall::getArrayValue() const { + ArrayCallKind arrayCallKind = getKind(); + if (arrayCallKind == ArrayCallKind::kArrayInit) return SILValue(SemanticsCall); - - return SILValue(); + return getArrayUninitializedInitResult(*this, 0); } SILValue swift::ArraySemanticsCall::getArrayElementStoragePointer() const { - if (getKind() == ArrayCallKind::kArrayUninitialized) { - TupleExtractInst *ArrayElementStorage = nullptr; - for (auto *Op : SemanticsCall->getUses()) { - auto *TupleElt = dyn_cast(Op->getUser()); - if (!TupleElt) - return SILValue(); - switch (TupleElt->getFieldNo()) { - default: - return SILValue(); - case 0: { - // Ignore the array value. - break; - } - case 1: - // Should only have one tuple extract after CSE. - if (ArrayElementStorage) - return SILValue(); - ArrayElementStorage = TupleElt; - break; - } - } - return SILValue(ArrayElementStorage); - } - - return SILValue(); + return getArrayUninitializedInitResult(*this, 1); } bool swift::ArraySemanticsCall::replaceByValue(SILValue V) { @@ -786,3 +787,75 @@ bool swift::ArraySemanticsCall::replaceByAppendingValues( return true; } + +bool swift::ArraySemanticsCall::mapInitializationStores( + llvm::DenseMap &ElementValueMap) { + if (getKind() != ArrayCallKind::kArrayUninitialized && + getKind() != ArrayCallKind::kArrayUninitializedIntrinsic) + return false; + SILValue ElementBuffer = getArrayElementStoragePointer(); + if (!ElementBuffer) + return false; + + // Match initialization stores into ElementBuffer. E.g. + // %83 = struct_extract %element_buffer : $UnsafeMutablePointer + // %84 = pointer_to_address %83 : $Builtin.RawPointer to strict $*Int + // store %85 to %84 : $*Int + // %87 = integer_literal $Builtin.Word, 1 + // %88 = index_addr %84 : $*Int, %87 : $Builtin.Word + // store %some_value to %88 : $*Int + + // If this an ArrayUinitializedIntrinsic then the ElementBuffer is a + // builtin.RawPointer. Otherwise, it is an UnsafeMutablePointer, which would + // be struct-extracted to obtain a builtin.RawPointer. + SILValue UnsafeMutablePointerExtract = + (getKind() == ArrayCallKind::kArrayUninitialized) + ? dyn_cast_or_null( + getSingleNonDebugUser(ElementBuffer)) + : ElementBuffer; + if (!UnsafeMutablePointerExtract) + return false; + + auto *PointerToAddress = dyn_cast_or_null( + getSingleNonDebugUser(UnsafeMutablePointerExtract)); + if (!PointerToAddress) + return false; + + // Match the stores. We can have either a store directly to the address or + // to an index_addr projection. + for (auto *Op : PointerToAddress->getUses()) { + auto *Inst = Op->getUser(); + + // Store to the base. + auto *SI = dyn_cast(Inst); + if (SI && SI->getDest() == PointerToAddress) { + // We have already seen an entry for this index bail. + if (ElementValueMap.count(0)) + return false; + ElementValueMap[0] = SI; + continue; + } else if (SI) + return false; + + // Store to an index_addr projection. + auto *IndexAddr = dyn_cast(Inst); + if (!IndexAddr) + return false; + SI = dyn_cast_or_null(getSingleNonDebugUser(IndexAddr)); + if (!SI || SI->getDest() != IndexAddr) + return false; + auto *Index = dyn_cast(IndexAddr->getIndex()); + if (!Index) + return false; + auto IndexVal = Index->getValue(); + // Let's not blow up our map. + if (IndexVal.getActiveBits() > 16) + return false; + // Already saw an entry. + if (ElementValueMap.count(IndexVal.getZExtValue())) + return false; + + ElementValueMap[IndexVal.getZExtValue()] = SI; + } + return !ElementValueMap.empty(); +} diff --git a/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp b/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp index b4265a2690aa3..117f1dd38ee0a 100644 --- a/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp +++ b/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp @@ -314,6 +314,7 @@ static bool isNonMutatingArraySemanticCall(SILInstruction *Inst) { case ArrayCallKind::kWithUnsafeMutableBufferPointer: case ArrayCallKind::kArrayInit: case ArrayCallKind::kArrayUninitialized: + case ArrayCallKind::kArrayUninitializedIntrinsic: case ArrayCallKind::kAppendContentsOf: case ArrayCallKind::kAppendElement: return false; @@ -662,7 +663,8 @@ bool COWArrayOpt::hasLoopOnlyDestructorSafeArrayOperations() { auto Kind = Sem.getKind(); // Safe because they create new arrays. if (Kind == ArrayCallKind::kArrayInit || - Kind == ArrayCallKind::kArrayUninitialized) + Kind == ArrayCallKind::kArrayUninitialized || + Kind == ArrayCallKind::kArrayUninitializedIntrinsic) continue; // All array types must be the same. This is a stronger guaranteed than // we actually need. The requirement is that we can't create another diff --git a/lib/SILOptimizer/Transforms/ArrayElementValuePropagation.cpp b/lib/SILOptimizer/Transforms/ArrayElementValuePropagation.cpp index 4a64ff9fe1962..8a24b9d41cf66 100644 --- a/lib/SILOptimizer/Transforms/ArrayElementValuePropagation.cpp +++ b/lib/SILOptimizer/Transforms/ArrayElementValuePropagation.cpp @@ -67,7 +67,7 @@ class ArrayAllocation { /// A map of Array indices to element values llvm::DenseMap ElementValueMap; - bool mapInitializationStores(SILValue ElementBuffer); + bool mapInitializationStores(ArraySemanticsCall arrayUninitCall); bool recursivelyCollectUses(ValueBase *Def); bool replacementsAreValid(); @@ -93,64 +93,16 @@ class ArrayAllocation { }; /// Map the indices of array element initialization stores to their values. -bool ArrayAllocation::mapInitializationStores(SILValue ElementBuffer) { - assert(ElementBuffer && - "Must have identified an array element storage pointer"); - - // Match initialization stores. - // %83 = struct_extract %element_buffer : $UnsafeMutablePointer - // %84 = pointer_to_address %83 : $Builtin.RawPointer to strict $*Int - // store %85 to %84 : $*Int - // %87 = integer_literal $Builtin.Word, 1 - // %88 = index_addr %84 : $*Int, %87 : $Builtin.Word - // store %some_value to %88 : $*Int - - auto *UnsafeMutablePointerExtract = - dyn_cast_or_null(getSingleNonDebugUser(ElementBuffer)); - if (!UnsafeMutablePointerExtract) +bool ArrayAllocation::mapInitializationStores( + ArraySemanticsCall arrayUninitCall) { + llvm::DenseMap elementStoreMap; + if (!arrayUninitCall.mapInitializationStores(elementStoreMap)) return false; - auto *PointerToAddress = dyn_cast_or_null( - getSingleNonDebugUser(UnsafeMutablePointerExtract)); - if (!PointerToAddress) - return false; - - // Match the stores. We can have either a store directly to the address or - // to an index_addr projection. - for (auto *Op : PointerToAddress->getUses()) { - auto *Inst = Op->getUser(); - - // Store to the base. - auto *SI = dyn_cast(Inst); - if (SI && SI->getDest() == PointerToAddress) { - // We have already seen an entry for this index bail. - if (ElementValueMap.count(0)) - return false; - ElementValueMap[0] = SI->getSrc(); - continue; - } else if (SI) - return false; - - // Store an index_addr projection. - auto *IndexAddr = dyn_cast(Inst); - if (!IndexAddr) - return false; - SI = dyn_cast_or_null(getSingleNonDebugUser(IndexAddr)); - if (!SI || SI->getDest() != IndexAddr) - return false; - auto *Index = dyn_cast(IndexAddr->getIndex()); - if (!Index) - return false; - auto IndexVal = Index->getValue(); - // Let's not blow up our map. - if (IndexVal.getActiveBits() > 16) - return false; - // Already saw an entry. - if (ElementValueMap.count(IndexVal.getZExtValue())) - return false; - - ElementValueMap[IndexVal.getZExtValue()] = SI->getSrc(); - } - return !ElementValueMap.empty(); + // Extract the SIL values of the array elements from the stores. + ElementValueMap.grow(elementStoreMap.size()); + for (auto keyValue : elementStoreMap) + ElementValueMap[keyValue.getFirst()] = keyValue.getSecond()->getSrc(); + return true; } bool ArrayAllocation::replacementsAreValid() { @@ -217,12 +169,8 @@ bool ArrayAllocation::analyze(ApplyInst *Alloc) { if (!ArrayValue) return false; - SILValue ElementBuffer = Uninitialized.getArrayElementStoragePointer(); - if (!ElementBuffer) - return false; - // Figure out all stores to the array. - if (!mapInitializationStores(ElementBuffer)) + if (!mapInitializationStores(Uninitialized)) return false; // Check if the array value was stored or has escaped. diff --git a/lib/SILOptimizer/Transforms/DeadObjectElimination.cpp b/lib/SILOptimizer/Transforms/DeadObjectElimination.cpp index 6056b41222477..df9ffcc8e3004 100644 --- a/lib/SILOptimizer/Transforms/DeadObjectElimination.cpp +++ b/lib/SILOptimizer/Transforms/DeadObjectElimination.cpp @@ -641,7 +641,8 @@ static bool removeAndReleaseArray(SingleValueInstruction *NewArrayValue, /// side effect? static bool isAllocatingApply(SILInstruction *Inst) { ArraySemanticsCall ArrayAlloc(Inst); - return ArrayAlloc.getKind() == ArrayCallKind::kArrayUninitialized; + return ArrayAlloc.getKind() == ArrayCallKind::kArrayUninitialized || + ArrayAlloc.getKind() == ArrayCallKind::kArrayUninitializedIntrinsic; } namespace { @@ -832,7 +833,9 @@ static bool getDeadInstsAfterInitializerRemoved( bool DeadObjectElimination::processAllocApply(ApplyInst *AI, DeadEndBlocks &DEBlocks) { // Currently only handle array.uninitialized - if (ArraySemanticsCall(AI).getKind() != ArrayCallKind::kArrayUninitialized) + if (ArraySemanticsCall(AI).getKind() != ArrayCallKind::kArrayUninitialized && + ArraySemanticsCall(AI).getKind() != + ArrayCallKind::kArrayUninitializedIntrinsic) return false; llvm::SmallVector instsDeadAfterInitializerRemoved; diff --git a/test/SILOptimizer/dead_array_elim.sil b/test/SILOptimizer/dead_array_elim.sil index 54b053e25d2e9..620dbb5cec86e 100644 --- a/test/SILOptimizer/dead_array_elim.sil +++ b/test/SILOptimizer/dead_array_elim.sil @@ -21,7 +21,7 @@ class TrivialDestructor { // Remove a dead array. // rdar://20980377 Add dead array elimination to DeadObjectElimination // Swift._allocateUninitializedArray (Builtin.Word) -> (Swift.Array, Builtin.RawPointer) -sil [_semantics "array.uninitialized"] @allocArray : $@convention(thin) <τ_0_0> (Builtin.Word) -> @owned (Array<τ_0_0>, Builtin.RawPointer) +sil [_semantics "array.uninitialized_intrinsic"] @allocArray : $@convention(thin) <τ_0_0> (Builtin.Word) -> @owned (Array<τ_0_0>, Builtin.RawPointer) sil [_semantics "array.uninitialized"] @adoptStorageSpecialiedForInt : $@convention(method) (@guaranteed _ContiguousArrayStorage, Builtin.Word, @thin Array.Type) -> (@owned Array, UnsafeMutablePointer) diff --git a/test/SILOptimizer/dead_array_elim.swift b/test/SILOptimizer/dead_array_elim.swift new file mode 100644 index 0000000000000..fdc389e8441c2 --- /dev/null +++ b/test/SILOptimizer/dead_array_elim.swift @@ -0,0 +1,40 @@ +// RUN: %target-swift-frontend -O -emit-sil -primary-file %s | %FileCheck %s + +// These tests check whether DeadObjectElimination pass runs as a part of the +// optimization pipeline and eliminates dead array literals in Swift code. +// Note that DeadObjectElimination pass relies on @_semantics annotations on +// the array initializer that is used by the compiler to create array literals. +// This test would fail if in case the initializer used by the compiler to +// initialize array literals doesn't match the one expected by the pass. + +// CHECK-LABEL: sil hidden @$s15dead_array_elim24testDeadArrayEliminationyyF +func testDeadArrayElimination() { + _ = [1, 2, 3] + // CHECK: bb0: + // CHECK-NEXT: %{{.*}} = tuple () + // CHECK-NEXT: return %{{.*}} : $() +} + +// CHECK-LABEL: sil hidden @$s15dead_array_elim29testEmptyDeadArrayEliminationyyF +func testEmptyDeadArrayElimination() { + _ = [] + // CHECK: bb0: + // CHECK-NEXT: %{{.*}} = tuple () + // CHECK-NEXT: return %{{.*}} : $() +} + +// The use case tested by the following test, where a _fixLifetime call is +// invoked on an array, appears when new os log APIs are used. +// CHECK-LABEL: sil hidden @$s15dead_array_elim35testDeadArrayElimWithFixLifetimeUseyyF +func testDeadArrayElimWithFixLifetimeUse() { + let a: [Int] = [] + _fixLifetime(a) + // CHECK: bb0: + // CHECK-NEXT: %{{.*}} = tuple () + // CHECK-NEXT: return %{{.*}} : $() +} + +// FIXME: DeadObjectElimination doesn't optimize this yet. +func testDeadArrayElimWithAddressOnlyValues(x: T, y: T) { + _ = [x, y] +}