Revert "[DSE] Track earliest escape, use for loads in isReadClobber."

nico · fhahn · commit c39c786ba664 · 2021-09-27T10:47:38.000+01:00
This reverts commit 5ce8927. Makes clang crash, see comments on https://reviews.llvm.org/D109844 (cherry-picked from df56fc6)
diff --git a/llvm/include/llvm/Analysis/CaptureTracking.h b/llvm/include/llvm/Analysis/CaptureTracking.h
@@ -23,7 +23,6 @@ namespace llvm {
   class Instruction;
   class DominatorTree;
   class LoopInfo;
-  class Function;
 
   /// getDefaultMaxUsesToExploreForCaptureTracking - Return default value of
   /// the maximal number of uses to explore before giving up. It is used by
@@ -64,19 +63,6 @@ namespace llvm {
                                   unsigned MaxUsesToExplore = 0,
                                   const LoopInfo *LI = nullptr);
 
-  // Returns the 'earliest' instruction that captures \p V in \F. An instruction
-  // A is considered earlier than instruction B, if A dominates B. If 2 escapes
-  // do not dominate each other, the terminator of the common dominator is
-  // chosen. If not all uses can be analyzed, the earliest escape is set to
-  // the first instruction in the function entry block. If \p V does not escape,
-  // nullptr is returned. Note that the caller of the function has to ensure
-  // that the instruction the result value is compared against is not in a
-  // cycle.
-  Instruction *FindEarliestCapture(const Value *V, Function &F,
-                                   bool ReturnCaptures, bool StoreCaptures,
-                                   const DominatorTree &DT,
-                                   unsigned MaxUsesToExplore = 0);
-
   /// This callback is used in conjunction with PointerMayBeCaptured. In
   /// addition to the interface here, you'll need to provide your own getters
   /// to see whether anything was captured.
diff --git a/llvm/lib/Analysis/CaptureTracking.cpp b/llvm/lib/Analysis/CaptureTracking.cpp
@@ -143,66 +143,6 @@ namespace {
 
     const LoopInfo *LI;
   };
-
-  /// Find the 'earliest' instruction before which the pointer is known not to
-  /// be captured. Here an instruction A is considered earlier than instruction
-  /// B, if A dominates B. If 2 escapes do not dominate each other, the
-  /// terminator of the common dominator is chosen. If not all uses cannot be
-  /// analyzed, the earliest escape is set to the first instruction in the
-  /// function entry block.
-  // NOTE: Users have to make sure instructions compared against the earliest
-  // escape are not in a cycle.
-  struct EarliestCaptures : public CaptureTracker {
-
-    EarliestCaptures(bool ReturnCaptures, Function &F, const DominatorTree &DT)
-        : DT(DT), ReturnCaptures(ReturnCaptures), Captured(false), F(F) {}
-
-    void tooManyUses() override {
-      Captured = true;
-      EarliestCapture = &*F.getEntryBlock().begin();
-    }
-
-    bool captured(const Use *U) override {
-      Instruction *I = cast<Instruction>(U->getUser());
-      if (isa<ReturnInst>(I) && !ReturnCaptures)
-        return false;
-
-      if (!EarliestCapture) {
-        EarliestCapture = I;
-      } else if (EarliestCapture->getParent() == I->getParent()) {
-        if (I->comesBefore(EarliestCapture))
-          EarliestCapture = I;
-      } else {
-        BasicBlock *CurrentBB = I->getParent();
-        BasicBlock *EarliestBB = EarliestCapture->getParent();
-        if (DT.dominates(EarliestBB, CurrentBB)) {
-          // EarliestCapture already comes before the current use.
-        } else if (DT.dominates(CurrentBB, EarliestBB)) {
-          EarliestCapture = I;
-        } else {
-          // Otherwise find the nearest common dominator and use its terminator.
-          auto *NearestCommonDom =
-              DT.findNearestCommonDominator(CurrentBB, EarliestBB);
-          EarliestCapture = NearestCommonDom->getTerminator();
-        }
-      }
-      Captured = true;
-
-      // Return false to continue analysis; we need to see all potential
-      // captures.
-      return false;
-    }
-
-    Instruction *EarliestCapture = nullptr;
-
-    const DominatorTree &DT;
-
-    bool ReturnCaptures;
-
-    bool Captured;
-
-    Function &F;
-  };
 }
 
 /// PointerMayBeCaptured - Return true if this pointer value may be captured
@@ -266,22 +206,6 @@ bool llvm::PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
   return CB.Captured;
 }
 
-Instruction *llvm::FindEarliestCapture(const Value *V, Function &F,
-                                       bool ReturnCaptures, bool StoreCaptures,
-                                       const DominatorTree &DT,
-                                       unsigned MaxUsesToExplore) {
-  assert(!isa<GlobalValue>(V) &&
-         "It doesn't make sense to ask whether a global is captured.");
-
-  EarliestCaptures CB(ReturnCaptures, F, DT);
-  PointerMayBeCaptured(V, &CB, MaxUsesToExplore);
-  if (CB.Captured)
-    ++NumCapturedBefore;
-  else
-    ++NumNotCapturedBefore;
-  return CB.EarliestCapture;
-}
-
 void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker,
                                 unsigned MaxUsesToExplore) {
   assert(V->getType()->isPointerTy() && "Capture is for pointers only!");
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -38,7 +38,6 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/AliasAnalysis.h"
-#include "llvm/Analysis/CFG.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/LoopInfo.h"
@@ -893,9 +892,6 @@ struct DSEState {
   /// basic block.
   DenseMap<BasicBlock *, InstOverlapIntervalsTy> IOLs;
 
-  DenseMap<const Value *, Instruction *> EarliestEscapes;
-  DenseMap<Instruction *, TinyPtrVector<const Value *>> Inst2Obj;
-
   DSEState(Function &F, AliasAnalysis &AA, MemorySSA &MSSA, DominatorTree &DT,
            PostDominatorTree &PDT, const TargetLibraryInfo &TLI,
            const LoopInfo &LI)
@@ -1257,30 +1253,6 @@ struct DSEState {
                        InstWriteOffset) == OW_Complete;
   }
 
-  /// Returns true if \p Object is not captured before or by \p I.
-  bool notCapturedBeforeOrAt(const Value *Object, Instruction *I) {
-    if (!isIdentifiedFunctionLocal(Object))
-      return false;
-
-    auto Iter = EarliestEscapes.insert({Object, nullptr});
-    if (Iter.second) {
-      Instruction *EarliestCapture = FindEarliestCapture(
-          Object, F, /*ReturnCaptures=*/false, /*StoreCaptures=*/true, DT);
-      if (EarliestCapture) {
-        auto Ins = Inst2Obj.insert({EarliestCapture, {}});
-        Ins.first->second.push_back(Object);
-      }
-      Iter.first->second = EarliestCapture;
-    }
-
-    // No capturing instruction.
-    if (!Iter.first->second)
-      return true;
-
-    return I != Iter.first->second &&
-           !isPotentiallyReachable(Iter.first->second, I, nullptr, &DT, &LI);
-  }
-
   // Returns true if \p Use may read from \p DefLoc.
   bool isReadClobber(const MemoryLocation &DefLoc, Instruction *UseInst) {
     if (isNoopIntrinsic(UseInst))
@@ -1298,25 +1270,6 @@ struct DSEState {
       if (CB->onlyAccessesInaccessibleMemory())
         return false;
 
-    // BasicAA does not spend linear time to check whether local objects escape
-    // before potentially aliasing accesses. To improve DSE results, compute and
-    // cache escape info for local objects in certain circumstances.
-    if (auto *LI = dyn_cast<LoadInst>(UseInst)) {
-      // If the loads reads from a loaded underlying object accesses the load
-      // cannot alias DefLoc, if DefUO is a local object that has not escaped
-      // before the load.
-      auto *ReadUO = getUnderlyingObject(LI->getPointerOperand());
-      auto *DefUO = getUnderlyingObject(DefLoc.Ptr);
-      if (DefUO && ReadUO && isa<LoadInst>(ReadUO) &&
-          notCapturedBeforeOrAt(DefUO, UseInst)) {
-        assert(
-            !PointerMayBeCapturedBefore(DefLoc.Ptr, false, true, UseInst, &DT,
-                                        false, 0, &this->LI) &&
-            "cached analysis disagrees with fresh PointerMayBeCapturedBefore");
-        return false;
-      }
-    }
-
     // NOTE: For calls, the number of stores removed could be slightly improved
     // by using AA.callCapturesBefore(UseInst, DefLoc, &DT), but that showed to
     // be expensive compared to the benefits in practice. For now, avoid more
@@ -1747,17 +1700,7 @@ struct DSEState {
       if (MemoryAccess *MA = MSSA.getMemoryAccess(DeadInst)) {
         if (MemoryDef *MD = dyn_cast<MemoryDef>(MA)) {
           SkipStores.insert(MD);
-
-          // Clear any cached escape info for objects associated with the
-          // removed instructions.
-          auto Iter = Inst2Obj.find(DeadInst);
-          if (Iter != Inst2Obj.end()) {
-            for (const Value *Obj : Iter->second)
-              EarliestEscapes.erase(Obj);
-            Inst2Obj.erase(DeadInst);
-          }
         }
-
         Updater.removeMemoryAccess(MA);
       }
 
diff --git a/llvm/test/Transforms/DeadStoreElimination/captures-before-load.ll b/llvm/test/Transforms/DeadStoreElimination/captures-before-load.ll
