diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h --- a/llvm/include/llvm/Transforms/IPO/Attributor.h +++ b/llvm/include/llvm/Transforms/IPO/Attributor.h @@ -2537,6 +2537,24 @@ /// Return the locations encoded by \p MLK as a readable string. static std::string getMemoryLocationsAsStr(MemoryLocationsKind MLK); + /// Simple enum to distinguish read/write/read-write accesses. + enum AccessKind { + NONE = 0, + READ = 1 << 0, + WRITE = 1 << 1, + READ_WRITE = READ | WRITE, + }; + + /// Check \p Pred on all accesses to the memory kinds specified by \p MLK. + /// + /// This method will evaluate \p Pred on all accesses (access instruction + + /// underlying accessed memory pointer) and it will return true if \p Pred + /// holds every time. + virtual bool checkForAllAccessesToMemoryKind( + const function_ref &Pred, + MemoryLocationsKind MLK) const = 0; + /// Create an abstract attribute view for the position \p IRP. static AAMemoryLocation &createForPosition(const IRPosition &IRP, Attributor &A); diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -5322,17 +5322,83 @@ return IRAttribute::manifest(A); } + /// See AAMemoryLocation::checkForAllAccessesToMemoryKind(...). + bool checkForAllAccessesToMemoryKind( + const function_ref &Pred, + MemoryLocationsKind RequestedMLK) const override { + if (!isValidState()) + return false; + + MemoryLocationsKind AssumedMLK = getAssumedNotAccessedLocation(); + if (AssumedMLK == NO_LOCATIONS) + return true; + + for (MemoryLocationsKind CurMLK = 1; CurMLK < NO_LOCATIONS; CurMLK *= 2) { + if (CurMLK & RequestedMLK) + continue; + + const auto &Accesses = AccessKindAccessesMap.lookup(CurMLK); + for (const AccessInfo &AI : Accesses) + if (!Pred(*AI.I, AI.Ptr, AI.Kind, CurMLK)) + return false; + } + + return true; + } + protected: + /// Helper struct to tie together an instruction that has a read or write + /// effect with the pointer it accesses (if any). + struct AccessInfo { + + /// The instruction that caused the access. + const Instruction *I; + + /// The base pointer that is accessed, or null if unknown. + const Value *Ptr; + + /// The kind of access (read/write/read+write). + AccessKind Kind; + + bool operator==(const AccessInfo &RHS) const { + return I == RHS.I & Ptr == RHS.Ptr & Kind == RHS.Kind; + } + bool operator()(const AccessInfo &LHS, const AccessInfo &RHS) const { + if (LHS.I != RHS.I) + return LHS.I < RHS.I; + if (LHS.Ptr != RHS.Ptr) + return LHS.Ptr < RHS.Ptr; + if (LHS.Kind != RHS.Kind) + return LHS.Kind < RHS.Kind; + return false; + } + }; + + /// Mapping from *single* memory location kinds, e.g., LOCAL_MEM with the + /// value of NO_LOCAL_MEM, to the accesses encountered for this memory kind. + using AccessKindAccessesMapTy = + DenseMap>; + AccessKindAccessesMapTy AccessKindAccessesMap; + /// Return the kind(s) of location that may be accessed by \p V. AAMemoryLocation::MemoryLocationsKind categorizeAccessedLocations(Attributor &A, Instruction &I, bool &Changed); - /// Update the state \p State given that \p I is an access to a \p MLK memory - /// location with the access pointer \p Ptr. - static void updateState(AAMemoryLocation::StateType &State, - MemoryLocationsKind MLK, const Instruction &I, - const Value *Ptr, bool &Changed) { + /// Update the state \p State and the AccessKindAccessesMap given that \p I is + /// an access to a \p MLK memory location with the access pointer \p Ptr. + static void updateStateAndAccessesMap(AAMemoryLocation::StateType &State, + AccessKindAccessesMapTy &AccessMap, + MemoryLocationsKind MLK, + const Instruction &I, const Value *Ptr, + bool &Changed) { + // TODO: The kind should be determined at the call sites based on the + // information we have there. + AccessKind Kind = I.mayReadFromMemory() ? READ : NONE; + Kind = AccessKind(Kind | (I.mayWriteToMemory() ? WRITE : NONE)); + assert(isPowerOf2_32(MLK) && "Expected a single location set!"); + Changed |= AccessMap[MLK].insert(AccessInfo{&I, Ptr, Kind}).second; State.removeAssumedBits(MLK); } @@ -5368,32 +5434,39 @@ return true; if (auto *Arg = dyn_cast(&V)) { if (Arg->hasByValAttr()) - updateState(T, NO_LOCAL_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, NO_LOCAL_MEM, I, &V, + Changed); else - updateState(T, NO_ARGUMENT_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, NO_ARGUMENT_MEM, I, + &V, Changed); return true; } if (auto *GV = dyn_cast(&V)) { if (GV->hasLocalLinkage()) - updateState(T, NO_GLOBAL_INTERNAL_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, + NO_GLOBAL_INTERNAL_MEM, I, &V, Changed); else - updateState(T, NO_GLOBAL_EXTERNAL_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, + NO_GLOBAL_EXTERNAL_MEM, I, &V, Changed); return true; } if (isa(V)) { - updateState(T, NO_LOCAL_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, NO_LOCAL_MEM, I, &V, + Changed); return true; } if (ImmutableCallSite ICS = ImmutableCallSite(&V)) { const auto &NoAliasAA = A.getAAFor(*this, IRPosition::callsite_returned(ICS)); if (NoAliasAA.isAssumedNoAlias()) { - updateState(T, NO_MALLOCED_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, NO_MALLOCED_MEM, I, + &V, Changed); return true; } } - updateState(T, NO_UNKOWN_MEM, I, &V, Changed); + updateStateAndAccessesMap(T, AccessKindAccessesMap, NO_UNKOWN_MEM, I, &V, + Changed); LLVM_DEBUG(dbgs() << "[AAMemoryLocation] Ptr value cannot be categorized: " << V << " -> " << getMemoryLocationsAsStr(T.getAssumed()) << "\n"); @@ -5405,7 +5478,8 @@ /* MaxValues */ 32, StripGEPCB)) { LLVM_DEBUG( dbgs() << "[AAMemoryLocation] Pointer locations not categorized\n"); - updateState(State, NO_UNKOWN_MEM, I, nullptr, Changed); + updateStateAndAccessesMap(State, AccessKindAccessesMap, NO_UNKOWN_MEM, I, + nullptr, Changed); } else { LLVM_DEBUG( dbgs() @@ -5435,7 +5509,8 @@ return NO_LOCATIONS; if (ICSMemLocationAA.isAssumedInaccessibleMemOnly()) { - updateState(AccessedLocs, NO_INACCESSIBLE_MEM, I, nullptr, Changed); + updateStateAndAccessesMap(AccessedLocs, AccessKindAccessesMap, + NO_INACCESSIBLE_MEM, I, nullptr, Changed); return AccessedLocs.getAssumed(); } @@ -5449,7 +5524,8 @@ for (MemoryLocationsKind CurMLK = 1; CurMLK < NO_LOCATIONS; CurMLK *= 2) { if (ICSAssumedNotAccessedLocsNoArgMem & CurMLK) continue; - updateState(AccessedLocs, CurMLK, I, nullptr, Changed); + updateStateAndAccessesMap(AccessedLocs, AccessKindAccessesMap, CurMLK, I, + nullptr, Changed); } LLVM_DEBUG( @@ -5499,7 +5575,8 @@ LLVM_DEBUG(dbgs() << "[AAMemoryLocation] Failed to categorize instruction: " << I << "\n"); - updateState(AccessedLocs, NO_UNKOWN_MEM, I, nullptr, Changed); + updateStateAndAccessesMap(AccessedLocs, AccessKindAccessesMap, NO_UNKOWN_MEM, + I, nullptr, Changed); return AccessedLocs.getAssumed(); } @@ -5574,9 +5651,16 @@ Function *F = getAssociatedFunction(); const IRPosition &FnPos = IRPosition::function(*F); auto &FnAA = A.getAAFor(*this, FnPos); - return clampStateAndIndicateChange( - getState(), - static_cast(FnAA.getState())); + bool Changed = false; + auto AccessPred = [&](const Instruction &I, const Value *Ptr, + AccessKind Kind, MemoryLocationsKind MLK) { + updateStateAndAccessesMap(getState(), AccessKindAccessesMap, MLK, I, Ptr, + Changed); + return true; + }; + if (!FnAA.checkForAllAccessesToMemoryKind(AccessPred, ALL_LOCATIONS)) + return indicatePessimisticFixpoint(); + return Changed ? ChangeStatus::CHANGED : ChangeStatus::UNCHANGED; } /// See AbstractAttribute::trackStatistics() diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll --- a/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll +++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll @@ -1,6 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes -; RUN: opt -S -passes='attributor' -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=3 < %s | FileCheck %s --check-prefixes=CHECK,ATTRIBUTOR -; RUN: opt -S -passes='cgscc(inline),attributor' -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=3 < %s | FileCheck %s --check-prefixes=CHECK,INLINE_ATTRIBUTOR +; RUN: opt -S -passes='attributor' -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 < %s | FileCheck %s --check-prefixes=CHECK,ATTRIBUTOR +; RUN: opt -S -passes='cgscc(inline),attributor' -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 < %s | FileCheck %s --check-prefixes=CHECK,INLINE_ATTRIBUTOR %S = type { %S* } diff --git a/llvm/test/Transforms/Attributor/liveness.ll b/llvm/test/Transforms/Attributor/liveness.ll --- a/llvm/test/Transforms/Attributor/liveness.ll +++ b/llvm/test/Transforms/Attributor/liveness.ll @@ -1,6 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes -; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,OLDPM -; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,NEWPM +; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefixes=CHECK,OLDPM +; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefixes=CHECK,NEWPM ; UTC_ARGS: --turn off ; CHECK: @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* inttoptr (i32 1 to i8*), i8* inttoptr (i32 1 to i8*)] diff --git a/llvm/test/Transforms/Attributor/nofree.ll b/llvm/test/Transforms/Attributor/nofree.ll --- a/llvm/test/Transforms/Attributor/nofree.ll +++ b/llvm/test/Transforms/Attributor/nofree.ll @@ -1,4 +1,4 @@ -; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR +; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR ; Copied from Transforms/FunctoinAttrs/nofree-attributor.ll target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"