Temporarily revert "Reapply [LVI] Normalize pointer behavior" and "[LVI] Restructure caching"

This reverts commits 7e18aeba50 (D70376) 21fbd5587c (D69914) due to increased memory usage.

llvm/lib/Analysis/LazyValueInfo.cpp

@@ -136,9 +136,12 @@ namespace {
   /// A callback value handle updates the cache when values are erased.
   class LazyValueInfoCache;
   struct LVIValueHandle final : public CallbackVH {
+    // Needs to access getValPtr(), which is protected.
+    friend struct DenseMapInfo<LVIValueHandle>;
+
     LazyValueInfoCache *Parent;
 
-    LVIValueHandle(Value *V, LazyValueInfoCache *P = nullptr)
+    LVIValueHandle(Value *V, LazyValueInfoCache *P)
       : CallbackVH(V), Parent(P) { }
 
     void deleted() override;
@@ -152,83 +155,89 @@ namespace {
   /// This is the cache kept by LazyValueInfo which
   /// maintains information about queries across the clients' queries.
   class LazyValueInfoCache {
-    /// This is all of the cached information for one basic block. It contains
+    /// This is all of the cached block information for exactly one Value*.
-    /// the per-value lattice elements, as well as a separate set for
+    /// The entries are sorted by the BasicBlock* of the
-    /// overdefined values to reduce memory usage. Additionally pointers
+    /// entries, allowing us to do a lookup with a binary search.
-    /// dereferenced in the block are cached for nullability queries.
+    /// Over-defined lattice values are recorded in OverDefinedCache to reduce
-    struct BlockCacheEntryTy {
+    /// memory overhead.
-      SmallDenseMap<AssertingVH<Value>, ValueLatticeElement, 4> LatticeElements;
+    struct ValueCacheEntryTy {
-      SmallDenseSet<AssertingVH<Value>, 4> OverDefined;
+      ValueCacheEntryTy(Value *V, LazyValueInfoCache *P) : Handle(V, P) {}
-      // None indicates that the dereferenced pointers for this basic block
+      LVIValueHandle Handle;
-      // block have not been computed yet.
+      SmallDenseMap<PoisoningVH<BasicBlock>, ValueLatticeElement, 4> BlockVals;
-      Optional<DenseSet<AssertingVH<Value>>> DereferencedPointers;
     };
 
-    /// Cached information per basic block.
+    /// This tracks, on a per-block basis, the set of values that are
-    DenseMap<PoisoningVH<BasicBlock>, BlockCacheEntryTy> BlockCache;
+    /// over-defined at the end of that block.
-    /// Set of value handles used to erase values from the cache on deletion.
+    typedef DenseMap<PoisoningVH<BasicBlock>, SmallPtrSet<Value *, 4>>
-    DenseSet<LVIValueHandle, DenseMapInfo<Value *>> ValueHandles;
+        OverDefinedCacheTy;
+    /// Keep track of all blocks that we have ever seen, so we
+    /// don't spend time removing unused blocks from our caches.
+    DenseSet<PoisoningVH<BasicBlock> > SeenBlocks;
+
+    /// This is all of the cached information for all values,
+    /// mapped from Value* to key information.
+    DenseMap<Value *, std::unique_ptr<ValueCacheEntryTy>> ValueCache;
+    OverDefinedCacheTy OverDefinedCache;
+
 
   public:
     void insertResult(Value *Val, BasicBlock *BB,
                       const ValueLatticeElement &Result) {
-      auto &CacheEntry = BlockCache.try_emplace(BB).first->second;
+      SeenBlocks.insert(BB);
+
       // Insert over-defined values into their own cache to reduce memory
       // overhead.
       if (Result.isOverdefined())
-        CacheEntry.OverDefined.insert(Val);
+        OverDefinedCache[BB].insert(Val);
-      else
+      else {
-        CacheEntry.LatticeElements.insert({ Val, Result });
+        auto It = ValueCache.find_as(Val);
+        if (It == ValueCache.end()) {
+          ValueCache[Val] = std::make_unique<ValueCacheEntryTy>(Val, this);
+          It = ValueCache.find_as(Val);
+          assert(It != ValueCache.end() && "Val was just added to the map!");
+        }
+        It->second->BlockVals[BB] = Result;
+      }
+    }
 
-      auto HandleIt = ValueHandles.find_as(Val);
+    bool isOverdefined(Value *V, BasicBlock *BB) const {
-      if (HandleIt == ValueHandles.end())
+      auto ODI = OverDefinedCache.find(BB);
-        ValueHandles.insert({ Val, this });
+
+      if (ODI == OverDefinedCache.end())
+        return false;
+
+      return ODI->second.count(V);
     }
 
     bool hasCachedValueInfo(Value *V, BasicBlock *BB) const {
-      auto It = BlockCache.find(BB);
+      if (isOverdefined(V, BB))
-      if (It == BlockCache.end())
+        return true;
+
+      auto I = ValueCache.find_as(V);
+      if (I == ValueCache.end())
         return false;
 
-      return It->second.OverDefined.count(V) ||
+      return I->second->BlockVals.count(BB);
-             It->second.LatticeElements.count(V);
     }
 
     ValueLatticeElement getCachedValueInfo(Value *V, BasicBlock *BB) const {
-      auto It = BlockCache.find(BB);
+      if (isOverdefined(V, BB))
-      if (It == BlockCache.end())
-        return ValueLatticeElement();
-
-      if (It->second.OverDefined.count(V))
         return ValueLatticeElement::getOverdefined();
 
-      auto LatticeIt = It->second.LatticeElements.find(V);
+      auto I = ValueCache.find_as(V);
-      if (LatticeIt == It->second.LatticeElements.end())
+      if (I == ValueCache.end())
         return ValueLatticeElement();
-
+      auto BBI = I->second->BlockVals.find(BB);
-      return LatticeIt->second;
+      if (BBI == I->second->BlockVals.end())
-    }
+        return ValueLatticeElement();
-
+      return BBI->second;
-    bool isPointerDereferencedInBlock(
-        Value *V, BasicBlock *BB,
-        std::function<DenseSet<AssertingVH<Value>>(BasicBlock *)> InitFn) {
-      auto &CacheEntry = BlockCache.try_emplace(BB).first->second;
-      if (!CacheEntry.DereferencedPointers) {
-        CacheEntry.DereferencedPointers = InitFn(BB);
-        for (Value *V : *CacheEntry.DereferencedPointers) {
-          auto HandleIt = ValueHandles.find_as(V);
-          if (HandleIt == ValueHandles.end())
-            ValueHandles.insert({ V, this });
-        }
-      }
-
-      return CacheEntry.DereferencedPointers->count(V);
     }
 
     /// clear - Empty the cache.
     void clear() {
-      BlockCache.clear();
+      SeenBlocks.clear();
-      ValueHandles.clear();
+      ValueCache.clear();
+      OverDefinedCache.clear();
     }
 
     /// Inform the cache that a given value has been deleted.
@@ -242,20 +251,23 @@ namespace {
     /// OldSucc might have (unless also overdefined in NewSucc).  This just
     /// flushes elements from the cache and does not add any.
     void threadEdgeImpl(BasicBlock *OldSucc,BasicBlock *NewSucc);
+
+    friend struct LVIValueHandle;
   };
 }
 
 void LazyValueInfoCache::eraseValue(Value *V) {
-  for (auto &Pair : BlockCache) {
+  for (auto I = OverDefinedCache.begin(), E = OverDefinedCache.end(); I != E;) {
-    Pair.second.LatticeElements.erase(V);
+    // Copy and increment the iterator immediately so we can erase behind
-    Pair.second.OverDefined.erase(V);
+    // ourselves.
-    if (Pair.second.DereferencedPointers)
+    auto Iter = I++;
-      Pair.second.DereferencedPointers->erase(V);
+    SmallPtrSetImpl<Value *> &ValueSet = Iter->second;
+    ValueSet.erase(V);
+    if (ValueSet.empty())
+      OverDefinedCache.erase(Iter);
   }
 
-  auto HandleIt = ValueHandles.find_as(V);
+  ValueCache.erase(V);
-  if (HandleIt != ValueHandles.end())
-    ValueHandles.erase(HandleIt);
 }
 
 void LVIValueHandle::deleted() {
@@ -265,7 +277,18 @@ void LVIValueHandle::deleted() {
 }
 
 void LazyValueInfoCache::eraseBlock(BasicBlock *BB) {
-  BlockCache.erase(BB);
+  // Shortcut if we have never seen this block.
+  DenseSet<PoisoningVH<BasicBlock> >::iterator I = SeenBlocks.find(BB);
+  if (I == SeenBlocks.end())
+    return;
+  SeenBlocks.erase(I);
+
+  auto ODI = OverDefinedCache.find(BB);
+  if (ODI != OverDefinedCache.end())
+    OverDefinedCache.erase(ODI);
+
+  for (auto &I : ValueCache)
+    I.second->BlockVals.erase(BB);
 }
 
 void LazyValueInfoCache::threadEdgeImpl(BasicBlock *OldSucc,
@@ -283,11 +306,10 @@ void LazyValueInfoCache::threadEdgeImpl(BasicBlock *OldSucc,
   std::vector<BasicBlock*> worklist;
   worklist.push_back(OldSucc);
 
-  auto I = BlockCache.find(OldSucc);
+  auto I = OverDefinedCache.find(OldSucc);
-  if (I == BlockCache.end() || I->second.OverDefined.empty())
+  if (I == OverDefinedCache.end())
     return; // Nothing to process here.
-  SmallVector<Value *, 4> ValsToClear(I->second.OverDefined.begin(),
+  SmallVector<Value *, 4> ValsToClear(I->second.begin(), I->second.end());
-                                      I->second.OverDefined.end());
 
   // Use a worklist to perform a depth-first search of OldSucc's successors.
   // NOTE: We do not need a visited list since any blocks we have already
@@ -301,10 +323,10 @@ void LazyValueInfoCache::threadEdgeImpl(BasicBlock *OldSucc,
     if (ToUpdate == NewSucc) continue;
 
     // If a value was marked overdefined in OldSucc, and is here too...
-    auto OI = BlockCache.find(ToUpdate);
+    auto OI = OverDefinedCache.find(ToUpdate);
-    if (OI == BlockCache.end() || OI->second.OverDefined.empty())
+    if (OI == OverDefinedCache.end())
       continue;
-    auto &ValueSet = OI->second.OverDefined;
+    SmallPtrSetImpl<Value *> &ValueSet = OI->second;
 
     bool changed = false;
     for (Value *V : ValsToClear) {
@@ -314,6 +336,11 @@ void LazyValueInfoCache::threadEdgeImpl(BasicBlock *OldSucc,
       // If we removed anything, then we potentially need to update
       // blocks successors too.
       changed = true;
+
+      if (ValueSet.empty()) {
+        OverDefinedCache.erase(OI);
+        break;
+      }
     }
 
     if (!changed) continue;
@@ -415,7 +442,6 @@ namespace {
                                 BasicBlock *BB);
   bool solveBlockValueExtractValue(ValueLatticeElement &BBLV,
                                    ExtractValueInst *EVI, BasicBlock *BB);
-  bool isNonNullDueToDereferenceInBlock(Value *Val, BasicBlock *BB);
   void intersectAssumeOrGuardBlockValueConstantRange(Value *Val,
                                                      ValueLatticeElement &BBLV,
                                                      Instruction *BBI);
@@ -597,20 +623,6 @@ bool LazyValueInfoImpl::solveBlockValue(Value *Val, BasicBlock *BB) {
 
 bool LazyValueInfoImpl::solveBlockValueImpl(ValueLatticeElement &Res,
                                             Value *Val, BasicBlock *BB) {
-  // If this value is a nonnull pointer, record it's range and bailout.  Note
-  // that for all other pointer typed values, we terminate the search at the
-  // definition.  We could easily extend this to look through geps, bitcasts,
-  // and the like to prove non-nullness, but it's not clear that's worth it
-  // compile time wise.  The context-insensitive value walk done inside
-  // isKnownNonZero gets most of the profitable cases at much less expense.
-  // This does mean that we have a sensitivity to where the defining
-  // instruction is placed, even if it could legally be hoisted much higher.
-  // That is unfortunate.
-  PointerType *PT = dyn_cast<PointerType>(Val->getType());
-  if (PT && isKnownNonZero(Val, DL)) {
-    Res = ValueLatticeElement::getNot(ConstantPointerNull::get(PT));
-    return true;
-  }
 
   Instruction *BBI = dyn_cast<Instruction>(Val);
   if (!BBI || BBI->getParent() != BB)
@@ -622,6 +634,20 @@ bool LazyValueInfoImpl::solveBlockValueImpl(ValueLatticeElement &Res,
   if (auto *SI = dyn_cast<SelectInst>(BBI))
     return solveBlockValueSelect(Res, SI, BB);
 
+  // If this value is a nonnull pointer, record it's range and bailout.  Note
+  // that for all other pointer typed values, we terminate the search at the
+  // definition.  We could easily extend this to look through geps, bitcasts,
+  // and the like to prove non-nullness, but it's not clear that's worth it
+  // compile time wise.  The context-insensitive value walk done inside
+  // isKnownNonZero gets most of the profitable cases at much less expense.
+  // This does mean that we have a sensitivity to where the defining
+  // instruction is placed, even if it could legally be hoisted much higher.
+  // That is unfortunate.
+  PointerType *PT = dyn_cast<PointerType>(BBI->getType());
+  if (PT && isKnownNonZero(BBI, DL)) {
+    Res = ValueLatticeElement::getNot(ConstantPointerNull::get(PT));
+    return true;
+  }
   if (BBI->getType()->isIntegerTy()) {
     if (auto *CI = dyn_cast<CastInst>(BBI))
       return solveBlockValueCast(Res, CI, BB);
@@ -642,61 +668,75 @@ bool LazyValueInfoImpl::solveBlockValueImpl(ValueLatticeElement &Res,
   return true;
 }
 
-static void AddDereferencedPointer(
+static bool InstructionDereferencesPointer(Instruction *I, Value *Ptr) {
-    Value *Ptr, DenseSet<AssertingVH<Value>> &PtrSet, const DataLayout &DL) {
-  // TODO: Use NullPointerIsDefined instead.
-  if (Ptr->getType()->getPointerAddressSpace() == 0) {
-    Ptr = GetUnderlyingObject(Ptr, DL);
-    PtrSet.insert(Ptr);
-  }
-}
-
-static void AddPointersDereferencedByInstruction(
-    Instruction *I, DenseSet<AssertingVH<Value>> &PtrSet,
-    const DataLayout &DL) {
   if (LoadInst *L = dyn_cast<LoadInst>(I)) {
-    AddDereferencedPointer(L->getPointerOperand(), PtrSet, DL);
+    return L->getPointerAddressSpace() == 0 &&
-  } else if (StoreInst *S = dyn_cast<StoreInst>(I)) {
+           GetUnderlyingObject(L->getPointerOperand(),
-    AddDereferencedPointer(S->getPointerOperand(), PtrSet, DL);
+                               L->getModule()->getDataLayout()) == Ptr;
-  } else if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(I)) {
+  }
-    if (MI->isVolatile()) return;
+  if (StoreInst *S = dyn_cast<StoreInst>(I)) {
+    return S->getPointerAddressSpace() == 0 &&
+           GetUnderlyingObject(S->getPointerOperand(),
+                               S->getModule()->getDataLayout()) == Ptr;
+  }
+  if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(I)) {
+    if (MI->isVolatile()) return false;
 
     // FIXME: check whether it has a valuerange that excludes zero?
     ConstantInt *Len = dyn_cast<ConstantInt>(MI->getLength());
-    if (!Len || Len->isZero()) return;
+    if (!Len || Len->isZero()) return false;
 
-    AddDereferencedPointer(MI->getRawDest(), PtrSet, DL);
+    if (MI->getDestAddressSpace() == 0)
+      if (GetUnderlyingObject(MI->getRawDest(),
+                              MI->getModule()->getDataLayout()) == Ptr)
+        return true;
     if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(MI))
-      AddDereferencedPointer(MTI->getRawSource(), PtrSet, DL);
+      if (MTI->getSourceAddressSpace() == 0)
+        if (GetUnderlyingObject(MTI->getRawSource(),
+                                MTI->getModule()->getDataLayout()) == Ptr)
+          return true;
   }
+  return false;
 }
 
-bool LazyValueInfoImpl::isNonNullDueToDereferenceInBlock(
+/// Return true if the allocation associated with Val is ever dereferenced
-    Value *Val, BasicBlock *BB) {
+/// within the given basic block.  This establishes the fact Val is not null,
-  if (NullPointerIsDefined(BB->getParent(),
+/// but does not imply that the memory at Val is dereferenceable.  (Val may
-                           Val->getType()->getPointerAddressSpace()))
+/// point off the end of the dereferenceable part of the object.)
-    return false;
+static bool isObjectDereferencedInBlock(Value *Val, BasicBlock *BB) {
+  assert(Val->getType()->isPointerTy());
 
   const DataLayout &DL = BB->getModule()->getDataLayout();
-  Val = GetUnderlyingObject(Val, DL);
+  Value *UnderlyingVal = GetUnderlyingObject(Val, DL);
-
+  // If 'GetUnderlyingObject' didn't converge, skip it. It won't converge
-  return TheCache.isPointerDereferencedInBlock(Val, BB, [DL](BasicBlock *BB) {
+  // inside InstructionDereferencesPointer either.
-    DenseSet<AssertingVH<Value>> DereferencedPointers;
+  if (UnderlyingVal == GetUnderlyingObject(UnderlyingVal, DL, 1))
     for (Instruction &I : *BB)
-      AddPointersDereferencedByInstruction(&I, DereferencedPointers, DL);
+      if (InstructionDereferencesPointer(&I, UnderlyingVal))
-    return DereferencedPointers;
+        return true;
-  });
+  return false;
 }
 
 bool LazyValueInfoImpl::solveBlockValueNonLocal(ValueLatticeElement &BBLV,
-                                                Value *Val, BasicBlock *BB) {
+                                                 Value *Val, BasicBlock *BB) {
   ValueLatticeElement Result;  // Start Undefined.
 
   // If this is the entry block, we must be asking about an argument.  The
   // value is overdefined.
   if (BB == &BB->getParent()->getEntryBlock()) {
     assert(isa<Argument>(Val) && "Unknown live-in to the entry block");
-    BBLV = ValueLatticeElement::getOverdefined();
+    // Before giving up, see if we can prove the pointer non-null local to
+    // this particular block.
+    PointerType *PTy = dyn_cast<PointerType>(Val->getType());
+    if (PTy &&
+        (isKnownNonZero(Val, DL) ||
+          (isObjectDereferencedInBlock(Val, BB) &&
+           !NullPointerIsDefined(BB->getParent(), PTy->getAddressSpace())))) {
+      Result = ValueLatticeElement::getNot(ConstantPointerNull::get(PTy));
+    } else {
+      Result = ValueLatticeElement::getOverdefined();
+    }
+    BBLV = Result;
     return true;
   }
 
@@ -722,6 +762,14 @@ bool LazyValueInfoImpl::solveBlockValueNonLocal(ValueLatticeElement &BBLV,
     if (Result.isOverdefined()) {
       LLVM_DEBUG(dbgs() << " compute BB '" << BB->getName()
                         << "' - overdefined because of pred (non local).\n");
+      // Before giving up, see if we can prove the pointer non-null local to
+      // this particular block.
+      PointerType *PTy = dyn_cast<PointerType>(Val->getType());
+      if (PTy && isObjectDereferencedInBlock(Val, BB) &&
+          !NullPointerIsDefined(BB->getParent(), PTy->getAddressSpace())) {
+        Result = ValueLatticeElement::getNot(ConstantPointerNull::get(PTy));
+      }
+
       BBLV = Result;
       return true;
     }
@@ -794,24 +842,16 @@ void LazyValueInfoImpl::intersectAssumeOrGuardBlockValueConstantRange(
   // If guards are not used in the module, don't spend time looking for them
   auto *GuardDecl = BBI->getModule()->getFunction(
           Intrinsic::getName(Intrinsic::experimental_guard));
-  if (GuardDecl && !GuardDecl->use_empty()) {
+  if (!GuardDecl || GuardDecl->use_empty())
-    if (BBI->getIterator() == BBI->getParent()->begin())
+    return;
-      return;
-    for (Instruction &I : make_range(std::next(BBI->getIterator().getReverse()),
-                                     BBI->getParent()->rend())) {
-      Value *Cond = nullptr;
-      if (match(&I, m_Intrinsic<Intrinsic::experimental_guard>(m_Value(Cond))))
-        BBLV = intersect(BBLV, getValueFromCondition(Val, Cond));
-    }
-  }
 
-  if (BBLV.isOverdefined()) {
+  if (BBI->getIterator() == BBI->getParent()->begin())
-    // Check whether we're checking at the terminator, and the pointer has
+    return;
-    // been dereferenced in this block.
+  for (Instruction &I : make_range(std::next(BBI->getIterator().getReverse()),
-    PointerType *PTy = dyn_cast<PointerType>(Val->getType());
+                                   BBI->getParent()->rend())) {
-    if (PTy && BBI->getParent()->getTerminator() == BBI &&
+    Value *Cond = nullptr;
-        isNonNullDueToDereferenceInBlock(Val, BBI->getParent()))
+    if (match(&I, m_Intrinsic<Intrinsic::experimental_guard>(m_Value(Cond))))
-      BBLV = ValueLatticeElement::getNot(ConstantPointerNull::get(PTy));
+      BBLV = intersect(BBLV, getValueFromCondition(Val, Cond));
   }
 }
 

llvm/test/Transforms/JumpThreading/combine-metadata.ll

@@ -108,7 +108,7 @@ d2:
 d3:
   %y = load i32*, i32** %ptr
   store i32 1, i32* %y
-  %c2 = icmp eq i32* %y, @p
+  %c2 = icmp eq i32* %y, null
   br i1 %c2, label %ret1, label %ret2
 
 ret1:
@@ -118,6 +118,5 @@ ret2:
   ret void
 }
 
-@p = external global i32
 
 !0 = !{}