[GlobalOpt] Revert valgrind hacks

Differential revision: https://reviews.llvm.org/D69428
This commit is contained in:
Evgeny Leviant 2021-04-13 19:11:10 +03:00
parent 0f24c0118d
commit dbc16ed199
5 changed files with 9 additions and 239 deletions

View File

@ -106,175 +106,6 @@ 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.
@ -823,12 +654,8 @@ static bool OptimizeAwayTrappingUsesOfLoads(
// If we nuked all of the loads, then none of the stores are needed either,
// nor is the global.
if (AllNonStoreUsesGone) {
if (isLeakCheckerRoot(GV)) {
Changed |= CleanupPointerRootUsers(GV, GetTLI);
} else {
Changed = true;
CleanupConstantGlobalUsers(GV, nullptr, DL, GetTLI);
}
Changed = true;
CleanupConstantGlobalUsers(GV, nullptr, DL, GetTLI);
if (GV->use_empty()) {
LLVM_DEBUG(dbgs() << " *** GLOBAL NOW DEAD!\n");
Changed = true;
@ -1997,15 +1824,10 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
if (!GS.IsLoaded) {
LLVM_DEBUG(dbgs() << "GLOBAL NEVER LOADED: " << *GV << "\n");
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);
}
// 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()) {

View File

@ -32,11 +32,8 @@
; 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

View File

@ -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-NOT: call i8* @malloc(i64
; CHECK: 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

View File

@ -1,49 +0,0 @@
; 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)

View File

@ -1,10 +1,10 @@
; RUN: opt < %s -globalopt -S | FileCheck %s
; RUN: opt < %s -globalopt -instcombine -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: @global = internal unnamed_addr global i16* null, align 1
; CHECK-NOT: @global = internal unnamed_addr global i16* null, align 1
; CHECK-LABEL: @foo
; CHECK-NEXT: entry: