Revert "[GlobalOpt] Revert valgrind hacks"

This reverts commit dbc16ed.

Author: Sterling-Augustine

llvm/lib/Transforms/IPO/GlobalOpt.cpp

@@ -106,6 +106,175 @@ static cl::opt<int> ColdCCRelFreq(
         "entry frequency, for a call site to be considered cold for enabling"
         "coldcc"));
 
+/// Is this global variable possibly used by a leak checker as a root?  If so,
+/// we might not really want to eliminate the stores to it.
+static bool isLeakCheckerRoot(GlobalVariable *GV) {
+  // A global variable is a root if it is a pointer, or could plausibly contain
+  // a pointer.  There are two challenges; one is that we could have a struct
+  // the has an inner member which is a pointer.  We recurse through the type to
+  // detect these (up to a point).  The other is that we may actually be a union
+  // of a pointer and another type, and so our LLVM type is an integer which
+  // gets converted into a pointer, or our type is an [i8 x #] with a pointer
+  // potentially contained here.
+
+  if (GV->hasPrivateLinkage())
+    return false;
+
+  SmallVector<Type *, 4> Types;
+  Types.push_back(GV->getValueType());
+
+  unsigned Limit = 20;
+  do {
+    Type *Ty = Types.pop_back_val();
+    switch (Ty->getTypeID()) {
+      default: break;
+      case Type::PointerTyID:
+        return true;
+      case Type::FixedVectorTyID:
+      case Type::ScalableVectorTyID:
+        if (cast<VectorType>(Ty)->getElementType()->isPointerTy())
+          return true;
+        break;
+      case Type::ArrayTyID:
+        Types.push_back(cast<ArrayType>(Ty)->getElementType());
+        break;
+      case Type::StructTyID: {
+        StructType *STy = cast<StructType>(Ty);
+        if (STy->isOpaque()) return true;
+        for (StructType::element_iterator I = STy->element_begin(),
+                 E = STy->element_end(); I != E; ++I) {
+          Type *InnerTy = *I;
+          if (isa<PointerType>(InnerTy)) return true;
+          if (isa<StructType>(InnerTy) || isa<ArrayType>(InnerTy) ||
+              isa<VectorType>(InnerTy))
+            Types.push_back(InnerTy);
+        }
+        break;
+      }
+    }
+    if (--Limit == 0) return true;
+  } while (!Types.empty());
+  return false;
+}
+
+/// Given a value that is stored to a global but never read, determine whether
+/// it's safe to remove the store and the chain of computation that feeds the
+/// store.
+static bool IsSafeComputationToRemove(
+    Value *V, function_ref<TargetLibraryInfo &(Function &)> GetTLI) {
+  do {
+    if (isa<Constant>(V))
+      return true;
+    if (!V->hasOneUse())
+      return false;
+    if (isa<LoadInst>(V) || isa<InvokeInst>(V) || isa<Argument>(V) ||
+        isa<GlobalValue>(V))
+      return false;
+    if (isAllocationFn(V, GetTLI))
+      return true;
+
+    Instruction *I = cast<Instruction>(V);
+    if (I->mayHaveSideEffects())
+      return false;
+    if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I)) {
+      if (!GEP->hasAllConstantIndices())
+        return false;
+    } else if (I->getNumOperands() != 1) {
+      return false;
+    }
+
+    V = I->getOperand(0);
+  } while (true);
+}
+
+/// This GV is a pointer root.  Loop over all users of the global and clean up
+/// any that obviously don't assign the global a value that isn't dynamically
+/// allocated.
+static bool
+CleanupPointerRootUsers(GlobalVariable *GV,
+                        function_ref<TargetLibraryInfo &(Function &)> GetTLI) {
+  // A brief explanation of leak checkers.  The goal is to find bugs where
+  // pointers are forgotten, causing an accumulating growth in memory
+  // usage over time.  The common strategy for leak checkers is to explicitly
+  // allow the memory pointed to by globals at exit.  This is popular because it
+  // also solves another problem where the main thread of a C++ program may shut
+  // down before other threads that are still expecting to use those globals. To
+  // handle that case, we expect the program may create a singleton and never
+  // destroy it.
+
+  bool Changed = false;
+
+  // If Dead[n].first is the only use of a malloc result, we can delete its
+  // chain of computation and the store to the global in Dead[n].second.
+  SmallVector<std::pair<Instruction *, Instruction *>, 32> Dead;
+
+  // Constants can't be pointers to dynamically allocated memory.
+  for (Value::user_iterator UI = GV->user_begin(), E = GV->user_end();
+       UI != E;) {
+    User *U = *UI++;
+    if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
+      Value *V = SI->getValueOperand();
+      if (isa<Constant>(V)) {
+        Changed = true;
+        SI->eraseFromParent();
+      } else if (Instruction *I = dyn_cast<Instruction>(V)) {
+        if (I->hasOneUse())
+          Dead.push_back(std::make_pair(I, SI));
+      }
+    } else if (MemSetInst *MSI = dyn_cast<MemSetInst>(U)) {
+      if (isa<Constant>(MSI->getValue())) {
+        Changed = true;
+        MSI->eraseFromParent();
+      } else if (Instruction *I = dyn_cast<Instruction>(MSI->getValue())) {
+        if (I->hasOneUse())
+          Dead.push_back(std::make_pair(I, MSI));
+      }
+    } else if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(U)) {
+      GlobalVariable *MemSrc = dyn_cast<GlobalVariable>(MTI->getSource());
+      if (MemSrc && MemSrc->isConstant()) {
+        Changed = true;
+        MTI->eraseFromParent();
+      } else if (Instruction *I = dyn_cast<Instruction>(MemSrc)) {
+        if (I->hasOneUse())
+          Dead.push_back(std::make_pair(I, MTI));
+      }
+    } else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U)) {
+      if (CE->use_empty()) {
+        CE->destroyConstant();
+        Changed = true;
+      }
+    } else if (Constant *C = dyn_cast<Constant>(U)) {
+      if (isSafeToDestroyConstant(C)) {
+        C->destroyConstant();
+        // This could have invalidated UI, start over from scratch.
+        Dead.clear();
+        CleanupPointerRootUsers(GV, GetTLI);
+        return true;
+      }
+    }
+  }
+
+  for (int i = 0, e = Dead.size(); i != e; ++i) {
+    if (IsSafeComputationToRemove(Dead[i].first, GetTLI)) {
+      Dead[i].second->eraseFromParent();
+      Instruction *I = Dead[i].first;
+      do {
+        if (isAllocationFn(I, GetTLI))
+          break;
+        Instruction *J = dyn_cast<Instruction>(I->getOperand(0));
+        if (!J)
+          break;
+        I->eraseFromParent();
+        I = J;
+      } while (true);
+      I->eraseFromParent();
+      Changed = true;
+    }
+  }
+
+  return Changed;
+}
+
 /// We just marked GV constant.  Loop over all users of the global, cleaning up
 /// the obvious ones.  This is largely just a quick scan over the use list to
 /// clean up the easy and obvious cruft.  This returns true if it made a change.
@@ -654,8 +823,12 @@ static bool OptimizeAwayTrappingUsesOfLoads(
   // If we nuked all of the loads, then none of the stores are needed either,
   // nor is the global.
   if (AllNonStoreUsesGone) {
-    Changed = true;
-    CleanupConstantGlobalUsers(GV, nullptr, DL, GetTLI);
+    if (isLeakCheckerRoot(GV)) {
+      Changed |= CleanupPointerRootUsers(GV, GetTLI);
+    } else {
+      Changed = true;
+      CleanupConstantGlobalUsers(GV, nullptr, DL, GetTLI);
+    }
     if (GV->use_empty()) {
       LLVM_DEBUG(dbgs() << "  *** GLOBAL NOW DEAD!\n");
       Changed = true;
@@ -1824,10 +1997,15 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
   if (!GS.IsLoaded) {
     LLVM_DEBUG(dbgs() << "GLOBAL NEVER LOADED: " << *GV << "\n");
 
-    // Delete any stores we can find to the global.  We may not be able to
-    // make it completely dead though.
-    Changed =
-        CleanupConstantGlobalUsers(GV, GV->getInitializer(), DL, GetTLI);
+    if (isLeakCheckerRoot(GV)) {
+      // Delete any constant stores to the global.
+      Changed = CleanupPointerRootUsers(GV, GetTLI);
+    } else {
+      // Delete any stores we can find to the global.  We may not be able to
+      // make it completely dead though.
+      Changed =
+          CleanupConstantGlobalUsers(GV, GV->getInitializer(), DL, GetTLI);
+    }
 
     // If the global is dead now, delete it.
     if (GV->use_empty()) {

llvm/test/ThinLTO/X86/import-constant.ll

@@ -32,8 +32,11 @@
 ; IMPORT-NEXT: @_ZL3Obj.llvm.{{.*}} = available_externally hidden constant %struct.S { i32 4, i32 8, i32* @val }
 ; IMPORT-NEXT: @val = available_externally global i32 42
 
+; OPT: @outer = internal unnamed_addr global %struct.Q zeroinitializer
+
 ; OPT:      define dso_local i32 @main()
 ; OPT-NEXT: entry:
+; OPT-NEXT:   store %struct.S* null, %struct.S** getelementptr inbounds (%struct.Q, %struct.Q* @outer, i64 0, i32 0)
 ; OPT-NEXT:   ret i32 12
 
 ; NOREFS:      @outer = internal local_unnamed_addr global %struct.Q zeroinitializer

llvm/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll

@@ -17,7 +17,7 @@ define void @test() nounwind ssp {
   %2 = sext i32 %1 to i64                         ; <i64> [#uses=1]
   %3 = mul i64 %2, ptrtoint (%struct.strchartype* getelementptr (%struct.strchartype, %struct.strchartype* null, i64 1) to i64) ; <i64> [#uses=1]
   %4 = tail call i8* @malloc(i64 %3)              ; <i8*> [#uses=1]
-; CHECK: call i8* @malloc(i64
+; CHECK-NOT: call i8* @malloc(i64
   %5 = bitcast i8* %4 to %struct.strchartype*     ; <%struct.strchartype*> [#uses=1]
   store %struct.strchartype* %5, %struct.strchartype** @chartypes, align 8
   ret void

llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll

@@ -0,0 +1,49 @@
+; RUN: opt -globalopt -S -o - < %s | FileCheck %s
+
+@glbl = internal global i8* null
+
+define void @test1a() {
+; CHECK-LABEL: @test1a(
+; CHECK-NOT: store
+; CHECK-NEXT: ret void
+  store i8* null, i8** @glbl
+  ret void
+}
+
+define void @test1b(i8* %p) {
+; CHECK-LABEL: @test1b(
+; CHECK-NEXT: store
+; CHECK-NEXT: ret void
+  store i8* %p, i8** @glbl
+  ret void
+}
+
+define void @test2() {
+; CHECK-LABEL: @test2(
+; CHECK: alloca i8
+  %txt = alloca i8
+  call void @foo2(i8* %txt)
+  %call2 = call i8* @strdup(i8* %txt)
+  store i8* %call2, i8** @glbl
+  ret void
+}
+declare i8* @strdup(i8*)
+declare void @foo2(i8*)
+
+define void @test3() uwtable personality i32 (i32, i64, i8*, i8*)* @__gxx_personality_v0 {
+; CHECK-LABEL: @test3(
+; CHECK-NOT: bb1:
+; CHECK-NOT: bb2:
+; CHECK: invoke
+  %ptr = invoke i8* @_Znwm(i64 1)
+          to label %bb1 unwind label %bb2
+bb1:
+  store i8* %ptr, i8** @glbl
+  unreachable
+bb2:
+  %tmp1 = landingpad { i8*, i32 }
+          cleanup
+  resume { i8*, i32 } %tmp1
+}
+declare i32 @__gxx_personality_v0(i32, i64, i8*, i8*)
+declare i8* @_Znwm(i64)

llvm/test/Transforms/GlobalOpt/dead-store-status.ll

@@ -1,10 +1,10 @@
-; RUN: opt < %s -globalopt -instcombine -S | FileCheck %s
+; RUN: opt < %s -globalopt -S | FileCheck %s
 
 ; When removing the store to @global in @foo, the pass would incorrectly return
 ; false. This was caught by the pass return status check that is hidden under
 ; EXPENSIVE_CHECKS.
 
-; CHECK-NOT: @global = internal unnamed_addr global i16* null, align 1
+; CHECK: @global = internal unnamed_addr global i16* null, align 1
 
 ; CHECK-LABEL: @foo
 ; CHECK-NEXT: entry: