From 3be2be585ce3e996ab6c075dc95096acbcea5012 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Fri, 20 Nov 2015 21:18:38 -0800 Subject: [PATCH 01/10] [MemoryBehavior] Cleanup and document logic for access to readonly memory location. --- lib/SILAnalysis/MemoryBehavior.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/SILAnalysis/MemoryBehavior.cpp b/lib/SILAnalysis/MemoryBehavior.cpp index 644ffb8245f0d..aaaff36c3a167 100644 --- a/lib/SILAnalysis/MemoryBehavior.cpp +++ b/lib/SILAnalysis/MemoryBehavior.cpp @@ -66,18 +66,20 @@ class MemoryBehaviorVisitor // If we do not have any more information, just use the general memory // behavior implementation. auto Behavior = Inst->getMemoryBehavior(); - if (!isLetPointer(V)) + bool ReadOnlyAccess = isLetPointer(V); + + // If this is a regular read-write access then return the computed memory + // behavior. + if (!ReadOnlyAccess) return Behavior; + // If this is a read-only access to 'let variable' then we can strip away + // the write access. switch (Behavior) { - case MemBehavior::MayHaveSideEffects: - return MemBehavior::MayRead; - case MemBehavior::MayReadWrite: - return MemBehavior::MayRead; - case MemBehavior::MayWrite: - return MemBehavior::None; - default: - return Behavior; + case MemBehavior::MayHaveSideEffects: return MemBehavior::MayRead; + case MemBehavior::MayReadWrite: return MemBehavior::MayRead; + case MemBehavior::MayWrite: return MemBehavior::None; + default: return Behavior; } } From f6d6f31c942590e5143e9f1693a115ea320bac00 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Fri, 20 Nov 2015 21:27:39 -0800 Subject: [PATCH 02/10] [AliasAnalysis] Document the AliasResult enum. NFC. --- include/swift/SILAnalysis/AliasAnalysis.h | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/include/swift/SILAnalysis/AliasAnalysis.h b/include/swift/SILAnalysis/AliasAnalysis.h index f3fe72472c5df..2f000b4d5a2c7 100644 --- a/include/swift/SILAnalysis/AliasAnalysis.h +++ b/include/swift/SILAnalysis/AliasAnalysis.h @@ -28,10 +28,24 @@ class SideEffectAnalysis; /// needed since we do not have an "analysis" infrastructure. class AliasAnalysis : public SILAnalysis { public: - /// The result of an alias query. This is based off of LLVM's alias - /// analysis so see LLVM's documentation for more information. + + /// This enum describes the different kinds of aliasing relations between + /// pointers. + /// + /// NoAlias: There is never dependence between memory referenced by the two + /// pointers. Example: Two pointers pointing to non-overlapping + /// memory ranges. + /// + /// MayAlias: Two pointers might refer to the same memory location. + /// + /// + /// PartialAlias: The two memory locations are known to be overlapping + /// but do not start at the same address. + /// + /// + /// MustAlias: The two memory locations always start at exactly the same + /// location. The pointers are equal. /// - /// FIXME: PartialAlias? enum class AliasResult : unsigned { NoAlias=0, ///< The two values have no dependencies on each /// other. From c1dce5c75cd15f14811af72d741a253584bf341f Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Fri, 20 Nov 2015 21:51:08 -0800 Subject: [PATCH 03/10] [AliasAnalysis] Document and roll the alias() family of methods. NFC. Document alias() and roll the mayAlias() family of methods into a single function. --- include/swift/SILAnalysis/AliasAnalysis.h | 37 +++++++++-------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/include/swift/SILAnalysis/AliasAnalysis.h b/include/swift/SILAnalysis/AliasAnalysis.h index 2f000b4d5a2c7..1e87b45eddeb1 100644 --- a/include/swift/SILAnalysis/AliasAnalysis.h +++ b/include/swift/SILAnalysis/AliasAnalysis.h @@ -84,33 +84,26 @@ class AliasAnalysis : public SILAnalysis { SideEffectAnalysis *getSideEffectAnalysis() const { return SEA; } - /// Perform an alias query to see if V1, V2 refer to the same values. - AliasResult alias(SILValue V1, SILValue V2, SILType TBAAType1 = SILType(), + /// Compute the alias properties of the pointers \p V1 and \p V2. + /// The optional arguments \pTBAAType1 and \p TBAAType2 specify the exact + /// types that the pointers access. + AliasResult alias(SILValue V1, SILValue V2, + SILType TBAAType1 = SILType(), SILType TBAAType2 = SILType()); - /// Convenience method that returns true if V1 and V2 must alias. - bool isMustAlias(SILValue V1, SILValue V2, SILType TBAAType1 = SILType(), - SILType TBAAType2 = SILType()) { - return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::MustAlias; +#define ALIAS_PROPERTY_CONVENIENCE_METHOD(KIND) \ + bool is##KIND(SILValue V1, SILValue V2, \ + SILType TBAAType1 = SILType(), \ + SILType TBAAType2 = SILType()) { \ + return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::KIND; \ } - /// Convenience method that returns true if V1 and V2 partially alias. - bool isPartialAlias(SILValue V1, SILValue V2, SILType TBAAType1 = SILType(), - SILType TBAAType2 = SILType()) { - return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::PartialAlias; - } + ALIAS_PROPERTY_CONVENIENCE_METHOD(MustAlias) + ALIAS_PROPERTY_CONVENIENCE_METHOD(PartialAlias) + ALIAS_PROPERTY_CONVENIENCE_METHOD(NoAlias) + ALIAS_PROPERTY_CONVENIENCE_METHOD(MayAlias) - /// Convenience method that returns true if V1, V2 can not alias. - bool isNoAlias(SILValue V1, SILValue V2, SILType TBAAType1 = SILType(), - SILType TBAAType2 = SILType()) { - return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::NoAlias; - } - - /// Convenience method that returns true if V1, V2 may alias. - bool isMayAlias(SILValue V1, SILValue V2, SILType TBAAType1 = SILType(), - SILType TBAAType2 = SILType()) { - return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::MayAlias; - } +#undef ALIAS_PROPERTY_CONVENIENCE_METHOD /// Use the alias analysis to determine the memory behavior of Inst with /// respect to V. From 4b2da0a4a032c7e4fc5a4cc84d12e9ad9c2759f4 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Fri, 20 Nov 2015 21:55:41 -0800 Subject: [PATCH 04/10] [AliasAnalysis] Remove the default argument from getMemoryBehavior. ObserveRetains is not an obvious default and it does not make sense to have it as a default argument. --- include/swift/SILAnalysis/AliasAnalysis.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/swift/SILAnalysis/AliasAnalysis.h b/include/swift/SILAnalysis/AliasAnalysis.h index 1e87b45eddeb1..bc70eb5a1792b 100644 --- a/include/swift/SILAnalysis/AliasAnalysis.h +++ b/include/swift/SILAnalysis/AliasAnalysis.h @@ -111,8 +111,7 @@ class AliasAnalysis : public SILAnalysis { /// TODO: When ref count behavior is separated from generic memory behavior, /// the IgnoreRefCountIncrements flag will be unnecessary. MemoryBehavior getMemoryBehavior(SILInstruction *Inst, SILValue V, - RetainObserveKind = - RetainObserveKind::ObserveRetains); + RetainObserveKind); /// Returns true if Inst may read from memory in a manner that affects V. bool mayReadFromMemory(SILInstruction *Inst, SILValue V) { @@ -141,7 +140,8 @@ class AliasAnalysis : public SILAnalysis { /// Returns true if Inst may have side effects in a manner that affects V. bool mayHaveSideEffects(SILInstruction *Inst, SILValue V) { - MemoryBehavior B = getMemoryBehavior(Inst, V); + MemoryBehavior B = getMemoryBehavior(Inst, V, + RetainObserveKind::ObserveRetains); return B == MemoryBehavior::MayWrite || B == MemoryBehavior::MayReadWrite || B == MemoryBehavior::MayHaveSideEffects; @@ -151,7 +151,8 @@ class AliasAnalysis : public SILAnalysis { /// V. This is independent of whether or not Inst may write to V and is meant /// to encode notions such as ref count modifications. bool mayHavePureSideEffects(SILInstruction *Inst, SILValue V) { - return getMemoryBehavior(Inst, V) == MemoryBehavior::MayHaveSideEffects; + return getMemoryBehavior(Inst, V, RetainObserveKind::ObserveRetains) == + MemoryBehavior::MayHaveSideEffects; } virtual void invalidate(SILAnalysis::InvalidationKind K) { AliasCache.clear(); } From 08d75ca865180395d150e82c8f7e336592c39127 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Sat, 21 Nov 2015 13:35:47 -0800 Subject: [PATCH 05/10] Abbreviate AliasResult. --- lib/SILAnalysis/AliasAnalysis.cpp | 61 ++++++++++++++----------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/lib/SILAnalysis/AliasAnalysis.cpp b/lib/SILAnalysis/AliasAnalysis.cpp index 143b540cb862e..775b82d2af50f 100644 --- a/lib/SILAnalysis/AliasAnalysis.cpp +++ b/lib/SILAnalysis/AliasAnalysis.cpp @@ -85,17 +85,14 @@ static llvm::cl::opt // Utilities //===----------------------------------------------------------------------===// -llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &OS, - AliasAnalysis::AliasResult R) { +using AliasResult = AliasAnalysis::AliasResult; + +llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &OS, AliasResult R) { switch (R) { - case AliasAnalysis::AliasResult::NoAlias: - return OS << "NoAlias"; - case AliasAnalysis::AliasResult::MayAlias: - return OS << "MayAlias"; - case AliasAnalysis::AliasResult::PartialAlias: - return OS << "PartialAlias"; - case AliasAnalysis::AliasResult::MustAlias: - return OS << "MustAlias"; + case AliasResult::NoAlias: return OS << "NoAlias"; + case AliasResult::MayAlias: return OS << "MayAlias"; + case AliasResult::PartialAlias: return OS << "PartialAlias"; + case AliasResult::MustAlias: return OS << "MustAlias"; } } @@ -265,9 +262,8 @@ static bool aliasUnequalObjects(SILValue O1, SILValue O2) { /// Uses a bunch of ad-hoc rules to disambiguate a GEP instruction against /// another pointer. We know that V1 is a GEP, but we don't know anything about /// V2. O1, O2 are getUnderlyingObject of V1, V2 respectively. -AliasAnalysis::AliasResult -AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2, - SILValue O1, SILValue O2) { +AliasResult AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2, + SILValue O1, SILValue O2) { // If V2 is also a gep instruction with a must-alias or not-aliasing base // pointer, figure out if the indices of the GEPs tell us anything about the @@ -278,9 +274,9 @@ AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2, // must alias relation on O1. This ensures that given an alloc_stack and a // gep from that alloc_stack, we say that they partially alias. if (isSameValueOrGlobal(O1, V2.stripCasts())) - return AliasAnalysis::AliasResult::PartialAlias; + return AliasResult::PartialAlias; - return AliasAnalysis::AliasResult::MayAlias; + return AliasResult::MayAlias; } assert(!Projection::isAddrProjection(O1) && @@ -289,12 +285,12 @@ AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2, "underlying object may not be a projection"); // Do the base pointers alias? - AliasAnalysis::AliasResult BaseAlias = aliasInner(O1, O2); + AliasResult BaseAlias = aliasInner(O1, O2); // If the underlying objects are not aliased, the projected values are also // not aliased. - if (BaseAlias == AliasAnalysis::AliasResult::NoAlias) - return AliasAnalysis::AliasResult::NoAlias; + if (BaseAlias == AliasResult::NoAlias) + return AliasResult::NoAlias; // Let's do alias checking based on projections. auto V1Path = ProjectionPath::getAddrProjectionPath(O1, V1, true); @@ -311,15 +307,15 @@ AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2, // horizon) or enable Projection to represent a cast as a special sort of // projection. if (!V1Path || !V2Path) - return AliasAnalysis::AliasResult::MayAlias; + return AliasResult::MayAlias; auto R = V1Path->computeSubSeqRelation(*V2Path); // If all of the projections are equal (and they have the same base pointer), // the two GEPs must be the same. - if (BaseAlias == AliasAnalysis::AliasResult::MustAlias && + if (BaseAlias == AliasResult::MustAlias && R == SubSeqRelation_t::Equal) - return AliasAnalysis::AliasResult::MustAlias; + return AliasResult::MustAlias; // The two GEPs do not alias if they are accessing different fields, even if // we don't know the base pointers. Different fields should not overlap. @@ -327,16 +323,16 @@ AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2, // TODO: Replace this with a check on the computed subseq relation. See the // TODO in computeSubSeqRelation. if (V1Path->hasNonEmptySymmetricDifference(V2Path.getValue())) - return AliasAnalysis::AliasResult::NoAlias; + return AliasResult::NoAlias; // If one of the GEPs is a super path of the other then they partially // alias. W - if (BaseAlias == AliasAnalysis::AliasResult::MustAlias && + if (BaseAlias == AliasResult::MustAlias && isStrictSubSeqRelation(R)) - return AliasAnalysis::AliasResult::PartialAlias; + return AliasResult::PartialAlias; // We failed to prove anything. Be conservative and return MayAlias. - return AliasAnalysis::AliasResult::MayAlias; + return AliasResult::MayAlias; } @@ -606,9 +602,9 @@ static bool typesMayAlias(SILType T1, SILType T2, SILType TBAA1Ty, /// The main AA entry point. Performs various analyses on V1, V2 in an attempt /// to disambiguate the two values. -AliasAnalysis::AliasResult AliasAnalysis::alias(SILValue V1, SILValue V2, - SILType TBAAType1, - SILType TBAAType2) { +AliasResult AliasAnalysis::alias(SILValue V1, SILValue V2, + SILType TBAAType1, + SILType TBAAType2) { auto Result = aliasInner(V1, V2, TBAAType1, TBAAType2); AliasCache.clear(); return Result; @@ -616,9 +612,9 @@ AliasAnalysis::AliasResult AliasAnalysis::alias(SILValue V1, SILValue V2, /// The main AA entry point. Performs various analyses on V1, V2 in an attempt /// to disambiguate the two values. -AliasAnalysis::AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2, - SILType TBAAType1, - SILType TBAAType2) { +AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2, + SILType TBAAType1, + SILType TBAAType2) { #ifndef NDEBUG // If alias analysis is disabled, always return may alias. if (!shouldRunAA()) @@ -733,8 +729,7 @@ bool swift::isLetPointer(SILValue V) { return false; } -AliasAnalysis::AliasResult AliasAnalysis::cacheValue(AliasCacheKey Key, - AliasResult Result) { +AliasResult AliasAnalysis::cacheValue(AliasCacheKey Key, AliasResult Result) { if (!CacheAAResults) return Result; DEBUG(llvm::dbgs() << "Caching Alias Result: " << Result << "\n" << Key.first From 2276a939176130641a3da277c1d94005a40f3104 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Sat, 21 Nov 2015 14:51:02 -0800 Subject: [PATCH 06/10] [AliasAnalysis] Clean some of the docs. Move some comments from the definition to the decleration and tidy some of the comments. --- include/swift/SILAnalysis/AliasAnalysis.h | 13 ++++++++----- lib/SILAnalysis/AliasAnalysis.cpp | 4 +--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/swift/SILAnalysis/AliasAnalysis.h b/include/swift/SILAnalysis/AliasAnalysis.h index bc70eb5a1792b..4bfda505b70c9 100644 --- a/include/swift/SILAnalysis/AliasAnalysis.h +++ b/include/swift/SILAnalysis/AliasAnalysis.h @@ -113,7 +113,8 @@ class AliasAnalysis : public SILAnalysis { MemoryBehavior getMemoryBehavior(SILInstruction *Inst, SILValue V, RetainObserveKind); - /// Returns true if Inst may read from memory in a manner that affects V. + /// Returns true if \p Inst may read from memory in a manner that + /// affects V. bool mayReadFromMemory(SILInstruction *Inst, SILValue V) { MemoryBehavior B = getMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); @@ -122,7 +123,8 @@ class AliasAnalysis : public SILAnalysis { B == MemoryBehavior::MayHaveSideEffects; } - /// Returns true if Inst may write to memory in a manner that affects V. + /// Returns true if \p Inst may write to memory in a manner that + /// affects V. bool mayWriteToMemory(SILInstruction *Inst, SILValue V) { MemoryBehavior B = getMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); @@ -131,8 +133,8 @@ class AliasAnalysis : public SILAnalysis { B == MemoryBehavior::MayHaveSideEffects; } - /// Returns true if Inst may read or write to memory in a manner that affects - /// V. + /// Returns true if \p Inst may read or write to memory in a manner that + /// affects V. bool mayReadOrWriteMemory(SILInstruction *Inst, SILValue V) { auto B = getMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); return B != MemoryBehavior::None; @@ -169,7 +171,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, /// Otherwise, return an empty type. SILType computeTBAAType(SILValue V); -/// Check if V points to a let-variable. +/// Check if \p V points to a let-member. +/// Nobody can write into let members. bool isLetPointer(SILValue V); } // end namespace swift diff --git a/lib/SILAnalysis/AliasAnalysis.cpp b/lib/SILAnalysis/AliasAnalysis.cpp index 775b82d2af50f..70b80e1393a65 100644 --- a/lib/SILAnalysis/AliasAnalysis.cpp +++ b/lib/SILAnalysis/AliasAnalysis.cpp @@ -699,10 +699,8 @@ AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2, return AliasResult::MayAlias; } -/// Check if this is the address of a "let" member. -/// Nobody can write into let members. bool swift::isLetPointer(SILValue V) { - // Traverse the "access" path for V and check that starts with "let" + // Traverse the "access" path for V and check that it starts with "let" // and everything along this path is a value-type (i.e. struct or tuple). // Is this an address of a "let" class member? From 96ab8d88a0290973df84e6e4ee652ffddddd11cc Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Sat, 21 Nov 2015 15:01:57 -0800 Subject: [PATCH 07/10] [AliasAnalysis] Rename getMemoryBehavior. getMemoryBehavior is also used by SILInstruction. This commit renames getMemoryBehavior to computeMemoryBehavior and cleans some of the code that touches it. --- lib/SILAnalysis/MemoryBehavior.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/SILAnalysis/MemoryBehavior.cpp b/lib/SILAnalysis/MemoryBehavior.cpp index aaaff36c3a167..d9aa9ef631cf0 100644 --- a/lib/SILAnalysis/MemoryBehavior.cpp +++ b/lib/SILAnalysis/MemoryBehavior.cpp @@ -305,8 +305,8 @@ MemBehavior MemoryBehaviorVisitor::visitReleaseValueInst(ReleaseValueInst *SI) { // Top Level Entrypoint //===----------------------------------------------------------------------===// -SILInstruction::MemoryBehavior -AliasAnalysis::getMemoryBehavior(SILInstruction *Inst, SILValue V, +MemBehavior +AliasAnalysis::computeMemoryBehavior(SILInstruction *Inst, SILValue V, RetainObserveKind IgnoreRefCountIncrements) { DEBUG(llvm::dbgs() << "GET MEMORY BEHAVIOR FOR:\n " << *Inst << " " << *V.getDef()); From c7a4637bd5c7932211df7b9e66e0706a21cce1a5 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Sat, 21 Nov 2015 15:03:55 -0800 Subject: [PATCH 08/10] [AliasAnalysis] Rename getMemoryBehavior. getMemoryBehavior is also used by SILInstruction. This commit renames getMemoryBehavior to computeMemoryBehavior and cleans some of the code that touches it. // I forgot to add this file in my previous commit. --- include/swift/SILAnalysis/AliasAnalysis.h | 33 +++++++++++------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/include/swift/SILAnalysis/AliasAnalysis.h b/include/swift/SILAnalysis/AliasAnalysis.h index 4bfda505b70c9..b2f582747313d 100644 --- a/include/swift/SILAnalysis/AliasAnalysis.h +++ b/include/swift/SILAnalysis/AliasAnalysis.h @@ -110,51 +110,48 @@ class AliasAnalysis : public SILAnalysis { /// /// TODO: When ref count behavior is separated from generic memory behavior, /// the IgnoreRefCountIncrements flag will be unnecessary. - MemoryBehavior getMemoryBehavior(SILInstruction *Inst, SILValue V, - RetainObserveKind); + MemoryBehavior computeMemoryBehavior(SILInstruction *Inst, SILValue V, + RetainObserveKind); /// Returns true if \p Inst may read from memory in a manner that /// affects V. bool mayReadFromMemory(SILInstruction *Inst, SILValue V) { - MemoryBehavior B = getMemoryBehavior(Inst, V, - RetainObserveKind::IgnoreRetains); + auto B = computeMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); return B == MemoryBehavior::MayRead || - B == MemoryBehavior::MayReadWrite || - B == MemoryBehavior::MayHaveSideEffects; + B == MemoryBehavior::MayReadWrite || + B == MemoryBehavior::MayHaveSideEffects; } /// Returns true if \p Inst may write to memory in a manner that /// affects V. bool mayWriteToMemory(SILInstruction *Inst, SILValue V) { - MemoryBehavior B = getMemoryBehavior(Inst, V, - RetainObserveKind::IgnoreRetains); + auto B = computeMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); return B == MemoryBehavior::MayWrite || - B == MemoryBehavior::MayReadWrite || - B == MemoryBehavior::MayHaveSideEffects; + B == MemoryBehavior::MayReadWrite || + B == MemoryBehavior::MayHaveSideEffects; } /// Returns true if \p Inst may read or write to memory in a manner that /// affects V. bool mayReadOrWriteMemory(SILInstruction *Inst, SILValue V) { - auto B = getMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); - return B != MemoryBehavior::None; + auto B = computeMemoryBehavior(Inst, V, RetainObserveKind::IgnoreRetains); + return MemoryBehavior::None != B; } /// Returns true if Inst may have side effects in a manner that affects V. bool mayHaveSideEffects(SILInstruction *Inst, SILValue V) { - MemoryBehavior B = getMemoryBehavior(Inst, V, - RetainObserveKind::ObserveRetains); + auto B = computeMemoryBehavior(Inst, V, RetainObserveKind::ObserveRetains); return B == MemoryBehavior::MayWrite || - B == MemoryBehavior::MayReadWrite || - B == MemoryBehavior::MayHaveSideEffects; + B == MemoryBehavior::MayReadWrite || + B == MemoryBehavior::MayHaveSideEffects; } /// Returns true if Inst may have side effects in a manner that affects /// V. This is independent of whether or not Inst may write to V and is meant /// to encode notions such as ref count modifications. bool mayHavePureSideEffects(SILInstruction *Inst, SILValue V) { - return getMemoryBehavior(Inst, V, RetainObserveKind::ObserveRetains) == - MemoryBehavior::MayHaveSideEffects; + auto B = computeMemoryBehavior(Inst, V, RetainObserveKind::ObserveRetains); + return MemoryBehavior::MayHaveSideEffects == B; } virtual void invalidate(SILAnalysis::InvalidationKind K) { AliasCache.clear(); } From 00e1685b1503315e7896aea848e43db9264d3b2f Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Sat, 21 Nov 2015 23:41:50 -0800 Subject: [PATCH 09/10] Rename 'Threshold' to a more specific variable name. --- lib/SILAnalysis/ValueTracking.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/SILAnalysis/ValueTracking.cpp b/lib/SILAnalysis/ValueTracking.cpp index 965e8f587ad5c..118fea23e991e 100644 --- a/lib/SILAnalysis/ValueTracking.cpp +++ b/lib/SILAnalysis/ValueTracking.cpp @@ -185,7 +185,7 @@ static bool isTransitiveEscapeInst(SILInstruction *Inst) { } /// Maximum amount of ValueCapture queries. -static unsigned const Threshold = 32; +static unsigned const ValueCaptureSearchThreshold = 32; namespace { @@ -205,8 +205,8 @@ enum CaptureException : unsigned { /// case to be conservative, we must assume it is captured. /// FIXME: Maybe put this on SILValue? static bool valueMayBeCaptured(SILValue V, CaptureException Exception) { - llvm::SmallVector Worklist; - llvm::SmallPtrSet Visited; + llvm::SmallVector Worklist; + llvm::SmallPtrSet Visited; unsigned Count = 0; DEBUG(llvm::dbgs() << " Checking for capture.\n"); @@ -216,7 +216,7 @@ static bool valueMayBeCaptured(SILValue V, CaptureException Exception) { for (auto *UI : V.getUses()) { // If we have more uses than the threshold, be conservative and bail so we // don't use too much compile time. - if (Count++ >= Threshold) + if (Count++ >= ValueCaptureSearchThreshold) return true; Visited.insert(UI); Worklist.push_back(UI); @@ -238,7 +238,7 @@ static bool valueMayBeCaptured(SILValue V, CaptureException Exception) { for (auto *UI : Inst->getUses()) { // If we have more uses than the threshold, be conservative and bail // so we don't use too much compile time. - if (Count++ >= Threshold) + if (Count++ >= ValueCaptureSearchThreshold) return true; if (Visited.insert(UI).second) { From 971aa239e4a0586e218c3b6a1ef8e7ef5281a672 Mon Sep 17 00:00:00 2001 From: Nadav Rotem Date: Sat, 21 Nov 2015 23:44:29 -0800 Subject: [PATCH 10/10] [AliasAnalysis] Remove the AliasAnalysis Cache. The AliasAnalysis is unsafe. When we delete instructions we don't have a good mechanism for invalidating the cache and we don't want instructions to explicitly invalidate AliasAnalysis after every deletion of an instruction. Morever, the alias analysis cache _always_ misses. Removing the cache did not change the build time of the standard library at all. --- include/swift/SILAnalysis/AliasAnalysis.h | 6 +--- lib/SILAnalysis/AliasAnalysis.cpp | 39 ++--------------------- 2 files changed, 4 insertions(+), 41 deletions(-) diff --git a/include/swift/SILAnalysis/AliasAnalysis.h b/include/swift/SILAnalysis/AliasAnalysis.h index b2f582747313d..495ef80019cd6 100644 --- a/include/swift/SILAnalysis/AliasAnalysis.h +++ b/include/swift/SILAnalysis/AliasAnalysis.h @@ -56,15 +56,11 @@ class AliasAnalysis : public SILAnalysis { }; private: - using AliasCacheKey = std::pair; - llvm::DenseMap AliasCache; SILModule *Mod; SideEffectAnalysis *SEA; using MemoryBehavior = SILInstruction::MemoryBehavior; - AliasResult cacheValue(AliasCacheKey Key, AliasResult Result); - AliasResult aliasAddressProjection(SILValue V1, SILValue V2, SILValue O1, SILValue O2); @@ -154,7 +150,7 @@ class AliasAnalysis : public SILAnalysis { return MemoryBehavior::MayHaveSideEffects == B; } - virtual void invalidate(SILAnalysis::InvalidationKind K) { AliasCache.clear(); } + virtual void invalidate(SILAnalysis::InvalidationKind K) { } virtual void invalidate(SILFunction *, SILAnalysis::InvalidationKind K) { invalidate(K); diff --git a/lib/SILAnalysis/AliasAnalysis.cpp b/lib/SILAnalysis/AliasAnalysis.cpp index 70b80e1393a65..365536869af18 100644 --- a/lib/SILAnalysis/AliasAnalysis.cpp +++ b/lib/SILAnalysis/AliasAnalysis.cpp @@ -76,11 +76,6 @@ static inline bool shouldRunBasicAA() { #endif -static llvm::cl::opt - CacheAAResults("cache-aa-results", - llvm::cl::desc("Should AA results be cached"), - llvm::cl::init(true)); - //===----------------------------------------------------------------------===// // Utilities //===----------------------------------------------------------------------===// @@ -605,9 +600,7 @@ static bool typesMayAlias(SILType T1, SILType T2, SILType TBAA1Ty, AliasResult AliasAnalysis::alias(SILValue V1, SILValue V2, SILType TBAAType1, SILType TBAAType2) { - auto Result = aliasInner(V1, V2, TBAAType1, TBAAType2); - AliasCache.clear(); - return Result; + return aliasInner(V1, V2, TBAAType1, TBAAType2); } /// The main AA entry point. Performs various analyses on V1, V2 in an attempt @@ -644,24 +637,6 @@ AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2, DEBUG(llvm::dbgs() << " After Cast Stripping V1:" << *V1.getDef()); DEBUG(llvm::dbgs() << " After Cast Stripping V2:" << *V2.getDef()); - // Create a key to lookup if we have already computed an alias result for V1, - // V2. Canonicalize our cache keys so that the pointer with the lower address - // is always the first element of the pair. This ensures we do not pollute our - // cache with two entries with the same key, albeit with the key's fields - // swapped. - auto Key = V1 < V2? std::make_pair(V1, V2) : std::make_pair(V2, V1); - - // If we find our key in the cache, just return the alias result. - if (CacheAAResults) { - auto Pair = AliasCache.find(Key); - if (Pair != AliasCache.end()) { - DEBUG(llvm::dbgs() << " Found value in the cache: " << Pair->second - << "\n"); - - return Pair->second; - } - } - // Ok, we need to actually compute an Alias Analysis result for V1, V2. Begin // by finding the "base" of V1, V2 by stripping off all casts and GEPs. SILValue O1 = getUnderlyingObject(V1); @@ -672,7 +647,7 @@ AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2, // If O1 and O2 do not equal, see if we can prove that they can not be the // same object. If we can, return No Alias. if (O1 != O2 && aliasUnequalObjects(O1, O2)) - return cacheValue(Key, AliasResult::NoAlias); + return AliasResult::NoAlias; // Ok, either O1, O2 are the same or we could not prove anything based off of // their inequality. Now we climb up use-def chains and attempt to do tricks @@ -690,10 +665,9 @@ AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2, if (Projection::isAddrProjection(V1)) { AliasResult Result = aliasAddressProjection(V1, V2, O1, O2); if (Result != AliasResult::MayAlias) - return cacheValue(Key, Result); + return Result; } - // We could not prove anything. Be conservative and return that V1, V2 may // alias. return AliasResult::MayAlias; @@ -727,13 +701,6 @@ bool swift::isLetPointer(SILValue V) { return false; } -AliasResult AliasAnalysis::cacheValue(AliasCacheKey Key, AliasResult Result) { - if (!CacheAAResults) - return Result; - DEBUG(llvm::dbgs() << "Caching Alias Result: " << Result << "\n" << Key.first - << Key.second << "\n"); - return AliasCache[Key] = Result; -} void AliasAnalysis::initialize(SILPassManager *PM) { SEA = PM->getAnalysis();