[GlobalOpt] Handle operators separately when removing GV users (#84694)

Refactor globalopt by eliminating redundant code. Fix
https://github.com/llvm/llvm-project/issues/64680.
This commit is contained in:
Anshil Gandhi 2025-03-25 11:14:33 -07:00 committed by GitHub
parent dfb6c761f7
commit 51dad714e8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 42 additions and 159 deletions

View File

@ -114,55 +114,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 (Type *InnerTy : STy->elements()) {
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.
@ -171,7 +122,7 @@ static bool IsSafeComputationToRemove(
do {
if (isa<Constant>(V))
return true;
if (!V->hasOneUse())
if (V->hasNUsesOrMore(1))
return false;
if (isa<LoadInst>(V) || isa<InvokeInst>(V) || isa<Argument>(V) ||
isa<GlobalValue>(V))
@ -193,90 +144,12 @@ static bool IsSafeComputationToRemove(
} 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;
SmallVector<User *> Worklist(GV->users());
// Constants can't be pointers to dynamically allocated memory.
while (!Worklist.empty()) {
User *U = Worklist.pop_back_val();
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>(MTI->getSource())) {
if (I->hasOneUse())
Dead.push_back(std::make_pair(I, MTI));
}
} else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U)) {
if (isa<GEPOperator>(CE))
append_range(Worklist, CE->users());
}
}
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;
}
}
GV->removeDeadConstantUsers();
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.
static bool CleanupConstantGlobalUsers(GlobalVariable *GV,
const DataLayout &DL) {
static bool CleanupConstantGlobalUsers(
GlobalVariable *GV, const DataLayout &DL,
function_ref<TargetLibraryInfo &(Function &)> GetTLI) {
Constant *Init = GV->getInitializer();
SmallVector<User *, 8> WorkList(GV->users());
SmallPtrSet<User *, 8> Visited;
@ -326,11 +199,30 @@ static bool CleanupConstantGlobalUsers(GlobalVariable *GV,
}
}
} else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
// Store must be unreachable or storing Init into the global.
EraseFromParent(SI);
} else if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(U)) { // memset/cpy/mv
if (getUnderlyingObject(MI->getRawDest()) == GV)
EraseFromParent(MI);
auto *V = SI->getValueOperand();
if (isa<Constant>(V)) {
EraseFromParent(SI);
} else if (isa<Instruction>(V)) {
EraseFromParent(SI);
if (IsSafeComputationToRemove(V, GetTLI))
RecursivelyDeleteTriviallyDeadInstructions(V);
} else if (isa<Argument>(V)) {
if (!V->getType()->isPointerTy())
EraseFromParent(SI);
}
} else if (auto *MSI = dyn_cast<MemSetInst>(U)) { // memset/cpy/mv
if (getUnderlyingObject(MSI->getRawDest()) == GV)
EraseFromParent(MSI);
} else if (auto *MTI = dyn_cast<MemTransferInst>(U)) {
auto *Src = MTI->getRawSource();
auto *Dst = MTI->getRawDest();
if (getUnderlyingObject(Dst) != GV)
continue;
if (isa<Instruction, Operator>(Src)) {
EraseFromParent(MTI);
if (IsSafeComputationToRemove(Src, GetTLI))
RecursivelyDeleteTriviallyDeadInstructions(Src);
}
} else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(U)) {
if (II->getIntrinsicID() == Intrinsic::threadlocal_address)
append_range(WorkList, II->users());
@ -878,12 +770,7 @@ 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, DL);
}
Changed |= CleanupConstantGlobalUsers(GV, DL, GetTLI);
if (GV->use_empty()) {
LLVM_DEBUG(dbgs() << " *** GLOBAL NOW DEAD!\n");
Changed = true;
@ -1497,15 +1384,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
// Delete it now.
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, DL);
}
Changed = CleanupConstantGlobalUsers(GV, DL, GetTLI);
// If the global is dead now, delete it.
if (GV->use_empty()) {
@ -1529,7 +1408,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
}
// Clean up any obviously simplifiable users now.
Changed |= CleanupConstantGlobalUsers(GV, DL);
Changed |= CleanupConstantGlobalUsers(GV, DL, GetTLI);
// If the global is dead now, just nuke it.
if (GV->use_empty()) {
@ -1584,7 +1463,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
}
// Clean up any obviously simplifiable users now.
CleanupConstantGlobalUsers(GV, DL);
CleanupConstantGlobalUsers(GV, DL, GetTLI);
if (GV->use_empty()) {
LLVM_DEBUG(dbgs() << " *** Substituting initializer allowed us to "

View File

@ -99,7 +99,6 @@ define i32 @load_gv_from_op_remove_store(ptr %p) local_unnamed_addr {
; CHECK-NEXT: call void @fn0()
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: store ptr [[E]], ptr getelementptr inbounds ([3 x ptr], ptr @b, i64 0, i64 2), align 16
; CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr @a, align 4
; CHECK-NEXT: [[INC:%.*]] = add nsw i32 [[TMP1]], 1
; CHECK-NEXT: store i32 [[INC]], ptr @a, align 4

View File

@ -4,8 +4,6 @@
; false. This was caught by the pass return status check that is hidden under
; EXPENSIVE_CHECKS.
; CHECK: @global = internal unnamed_addr global ptr null, align 1
; CHECK-LABEL: @foo
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i16 undef

View File

@ -7,17 +7,24 @@
declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg)
;.
; CHECK: @[[B:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr global ptr null
; CHECK: @[[C:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr constant [2 x ptr] zeroinitializer
; CHECK: @b = internal unnamed_addr global ptr null
;.
define void @test() {
; CHECK-LABEL: @test(
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr getelementptr inbounds ([2 x ptr], ptr @c, i64 0, i64 1), i64 8, i1 false)
; CHECK-NEXT: ret void
;
call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr getelementptr inbounds ([2 x ptr], ptr @c, i64 0, i64 1), i64 8, i1 false)
ret void
}
define void @neg_test(ptr %arg) {
; CHECK-LABEL: @neg_test(
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr [[ARG:%.*]], i64 8, i1 false)
; CHECK-NEXT: ret void
;
call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr %arg, i64 8, i1 false)
ret void
}
;.
; CHECK: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
;.