From d4258dce35b57539bc3cfb2d7233367cd0bf63e9 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Wed, 4 May 2016 00:20:48 +0000 Subject: [PATCH] [GlobalDCE, Misc] Don't remove functions referenced by ifuncs We forgot to consider the target of ifuncs when considering if a function was alive or dead. N.B. Also update a few auxiliary tools like bugpoint and verify-uselistorder. This fixes PR27593. llvm-svn: 268468 --- include/llvm/IR/GlobalAlias.h | 15 ----------- include/llvm/IR/GlobalIndirectSymbol.h | 16 ++++++++++++ lib/Transforms/IPO/GlobalDCE.cpp | 26 +++++++++++++++++++ lib/Transforms/Utils/SplitModule.cpp | 12 +++++---- test/Transforms/GlobalDCE/global-ifunc.ll | 13 ++++++++++ tools/bugpoint/CrashDebugger.cpp | 4 +-- .../verify-uselistorder.cpp | 8 ++++++ 7 files changed, 72 insertions(+), 22 deletions(-) create mode 100644 test/Transforms/GlobalDCE/global-ifunc.ll diff --git a/include/llvm/IR/GlobalAlias.h b/include/llvm/IR/GlobalAlias.h index a78532cebf2..3ae3e4a001e 100644 --- a/include/llvm/IR/GlobalAlias.h +++ b/include/llvm/IR/GlobalAlias.h @@ -78,21 +78,6 @@ public: return getIndirectSymbol(); } - const GlobalObject *getBaseObject() const { - return const_cast(this)->getBaseObject(); - } - GlobalObject *getBaseObject() { - return dyn_cast(getAliasee()->stripInBoundsOffsets()); - } - - const GlobalObject *getBaseObject(const DataLayout &DL, APInt &Offset) const { - return const_cast(this)->getBaseObject(DL, Offset); - } - GlobalObject *getBaseObject(const DataLayout &DL, APInt &Offset) { - return dyn_cast( - getAliasee()->stripAndAccumulateInBoundsConstantOffsets(DL, Offset)); - } - static bool isValidLinkage(LinkageTypes L) { return isExternalLinkage(L) || isLocalLinkage(L) || isWeakLinkage(L) || isLinkOnceLinkage(L); diff --git a/include/llvm/IR/GlobalIndirectSymbol.h b/include/llvm/IR/GlobalIndirectSymbol.h index 5948d3c1567..8edb3d1dbf4 100644 --- a/include/llvm/IR/GlobalIndirectSymbol.h +++ b/include/llvm/IR/GlobalIndirectSymbol.h @@ -49,6 +49,22 @@ public: return getOperand(0); } + const GlobalObject *getBaseObject() const { + return const_cast(this)->getBaseObject(); + } + GlobalObject *getBaseObject() { + return dyn_cast(getIndirectSymbol()->stripInBoundsOffsets()); + } + + const GlobalObject *getBaseObject(const DataLayout &DL, APInt &Offset) const { + return const_cast(this)->getBaseObject(DL, Offset); + } + GlobalObject *getBaseObject(const DataLayout &DL, APInt &Offset) { + return dyn_cast( + getIndirectSymbol()->stripAndAccumulateInBoundsConstantOffsets(DL, + Offset)); + } + // Methods for support type inquiry through isa, cast, and dyn_cast: static inline bool classof(const Value *V) { return V->getValueID() == Value::GlobalAliasVal || diff --git a/lib/Transforms/IPO/GlobalDCE.cpp b/lib/Transforms/IPO/GlobalDCE.cpp index 3b773010bb6..fca549947c6 100644 --- a/lib/Transforms/IPO/GlobalDCE.cpp +++ b/lib/Transforms/IPO/GlobalDCE.cpp @@ -32,6 +32,7 @@ using namespace llvm; STATISTIC(NumAliases , "Number of global aliases removed"); STATISTIC(NumFunctions, "Number of functions removed"); +STATISTIC(NumIFuncs, "Number of indirect functions removed"); STATISTIC(NumVariables, "Number of global variables removed"); namespace { @@ -118,6 +119,13 @@ PreservedAnalyses GlobalDCEPass::run(Module &M) { GlobalIsNeeded(&GA); } + for (GlobalIFunc &GIF : M.ifuncs()) { + Changed |= RemoveUnusedGlobalValue(GIF); + // Externally visible ifuncs are needed. + if (!GIF.isDiscardableIfUnused()) + GlobalIsNeeded(&GIF); + } + // Now that all globals which are needed are in the AliveGlobals set, we loop // through the program, deleting those which are not alive. // @@ -152,6 +160,14 @@ PreservedAnalyses GlobalDCEPass::run(Module &M) { GA.setAliasee(nullptr); } + // The third pass drops targets of ifuncs which are dead... + std::vector DeadIFuncs; + for (GlobalIFunc &GIF : M.ifuncs()) + if (!AliveGlobals.count(&GIF)) { + DeadIFuncs.push_back(&GIF); + GIF.setResolver(nullptr); + } + if (!DeadFunctions.empty()) { // Now that all interferences have been dropped, delete the actual objects // themselves. @@ -182,6 +198,16 @@ PreservedAnalyses GlobalDCEPass::run(Module &M) { Changed = true; } + // Now delete any dead aliases. + if (!DeadIFuncs.empty()) { + for (GlobalIFunc *GIF : DeadIFuncs) { + RemoveUnusedGlobalValue(*GIF); + M.getIFuncList().erase(GIF); + } + NumIFuncs += DeadIFuncs.size(); + Changed = true; + } + // Make sure that all memory is released AliveGlobals.clear(); SeenConstants.clear(); diff --git a/lib/Transforms/Utils/SplitModule.cpp b/lib/Transforms/Utils/SplitModule.cpp index 6b0ce9c2573..3db04b8b34c 100644 --- a/lib/Transforms/Utils/SplitModule.cpp +++ b/lib/Transforms/Utils/SplitModule.cpp @@ -47,7 +47,7 @@ static void addNonConstUser(ClusterMapType &GVtoClusterMap, if (const Instruction *I = dyn_cast(U)) { const GlobalValue *F = I->getParent()->getParent(); GVtoClusterMap.unionSets(GV, F); - } else if (isa(U) || isa(U) || + } else if (isa(U) || isa(U) || isa(U)) { GVtoClusterMap.unionSets(GV, cast(U)); } else { @@ -107,8 +107,8 @@ static void findPartitions(Module *M, ClusterIDMapType &ClusterIDMap, // For aliases we should not separate them from their aliasees regardless // of linkage. - if (GlobalAlias *GA = dyn_cast(&GV)) { - if (const GlobalObject *Base = GA->getBaseObject()) + if (auto *GIS = dyn_cast(&GV)) { + if (const GlobalObject *Base = GIS->getBaseObject()) GVtoClusterMap.unionSets(&GV, Base); } @@ -205,8 +205,8 @@ static void externalize(GlobalValue *GV) { // Returns whether GV should be in partition (0-based) I of N. static bool isInPartition(const GlobalValue *GV, unsigned I, unsigned N) { - if (auto GA = dyn_cast(GV)) - if (const GlobalObject *Base = GA->getBaseObject()) + if (auto *GIS = dyn_cast(GV)) + if (const GlobalObject *Base = GIS->getBaseObject()) GV = Base; StringRef Name; @@ -236,6 +236,8 @@ void llvm::SplitModule( externalize(&GV); for (GlobalAlias &GA : M->aliases()) externalize(&GA); + for (GlobalIFunc &GIF : M->ifuncs()) + externalize(&GIF); } // This performs splitting without a need for externalization, which might not diff --git a/test/Transforms/GlobalDCE/global-ifunc.ll b/test/Transforms/GlobalDCE/global-ifunc.ll new file mode 100644 index 00000000000..8022452c348 --- /dev/null +++ b/test/Transforms/GlobalDCE/global-ifunc.ll @@ -0,0 +1,13 @@ +; RUN: opt -S -globaldce < %s | FileCheck %s +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +@if = ifunc void (), void ()* @fn + +define internal void @fn() { +entry: + ret void +} + +; CHECK-DAG: @if = ifunc void (), void ()* @fn +; CHECK-DAG: define internal void @fn( diff --git a/tools/bugpoint/CrashDebugger.cpp b/tools/bugpoint/CrashDebugger.cpp index d494007665a..b161d88882a 100644 --- a/tools/bugpoint/CrashDebugger.cpp +++ b/tools/bugpoint/CrashDebugger.cpp @@ -264,8 +264,8 @@ bool ReduceCrashingFunctions::TestFuncs(std::vector &Funcs) { std::vector ToRemove; // First, remove aliases to functions we're about to purge. for (GlobalAlias &Alias : M->aliases()) { - Constant *Root = Alias.getAliasee()->stripPointerCasts(); - Function *F = dyn_cast(Root); + GlobalObject *Root = Alias.getBaseObject(); + Function *F = dyn_cast_or_null(Root); if (F) { if (Functions.count(F)) // We're keeping this function. diff --git a/tools/verify-uselistorder/verify-uselistorder.cpp b/tools/verify-uselistorder/verify-uselistorder.cpp index a2f80996416..b7118a09024 100644 --- a/tools/verify-uselistorder/verify-uselistorder.cpp +++ b/tools/verify-uselistorder/verify-uselistorder.cpp @@ -191,6 +191,8 @@ ValueMapping::ValueMapping(const Module &M) { map(&G); for (const GlobalAlias &A : M.aliases()) map(&A); + for (const GlobalIFunc &IF : M.ifuncs()) + map(&IF); for (const Function &F : M) map(&F); @@ -200,6 +202,8 @@ ValueMapping::ValueMapping(const Module &M) { map(G.getInitializer()); for (const GlobalAlias &A : M.aliases()) map(A.getAliasee()); + for (const GlobalIFunc &IF : M.ifuncs()) + map(IF.getResolver()); for (const Function &F : M) { if (F.hasPrefixData()) map(F.getPrefixData()); @@ -463,6 +467,8 @@ static void changeUseLists(Module &M, Changer changeValueUseList) { changeValueUseList(&G); for (GlobalAlias &A : M.aliases()) changeValueUseList(&A); + for (GlobalIFunc &IF : M.ifuncs()) + changeValueUseList(&IF); for (Function &F : M) changeValueUseList(&F); @@ -472,6 +478,8 @@ static void changeUseLists(Module &M, Changer changeValueUseList) { changeValueUseList(G.getInitializer()); for (GlobalAlias &A : M.aliases()) changeValueUseList(A.getAliasee()); + for (GlobalIFunc &IF : M.ifuncs()) + changeValueUseList(IF.getResolver()); for (Function &F : M) { if (F.hasPrefixData()) changeValueUseList(F.getPrefixData());