From cb5f239363a3c94db5425c105fcd45e77d2a16a9 Mon Sep 17 00:00:00 2001 From: Vasileios Porpodas Date: Tue, 14 Feb 2023 14:28:02 -0800 Subject: [PATCH] Revert "[NFC][IR] Make Module::getGlobalList() private" This reverts commit ed3e3ee9e30dfbffd2170a770a49b36a7f444916. --- clang/lib/CodeGen/CGHLSLRuntime.cpp | 2 +- clang/lib/CodeGen/CGObjCMac.cpp | 2 +- lldb/source/Expression/IRExecutionUnit.cpp | 2 +- llvm/docs/ProgrammersManual.rst | 9 ++- llvm/include/llvm/IR/Module.h | 20 ------ llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 2 +- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 4 +- llvm/lib/IR/Globals.cpp | 8 +-- llvm/lib/Linker/IRMover.cpp | 7 +- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp | 3 +- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 4 +- llvm/lib/Transforms/IPO/SCCP.cpp | 71 +------------------ .../lib/Transforms/IPO/WholeProgramDevirt.cpp | 2 +- llvm/lib/Transforms/Utils/CtorUtils.cpp | 2 +- llvm/unittests/IR/ModuleTest.cpp | 41 +---------- 15 files changed, 28 insertions(+), 151 deletions(-) diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index e9fa273f21cc8..5882f491d5972 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -175,7 +175,7 @@ void CGHLSLRuntime::finishCodeGen() { for (auto &Buf : Buffers) { layoutBuffer(Buf, DL); GlobalVariable *GV = replaceBuffer(Buf); - M.insertGlobalVariable(GV); + M.getGlobalList().push_back(GV); llvm::hlsl::ResourceClass RC = Buf.IsCBuffer ? llvm::hlsl::ResourceClass::CBuffer : llvm::hlsl::ResourceClass::SRV; diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp index 5f4cdc6d91f1d..c739d3742f801 100644 --- a/clang/lib/CodeGen/CGObjCMac.cpp +++ b/clang/lib/CodeGen/CGObjCMac.cpp @@ -7431,7 +7431,7 @@ CGObjCNonFragileABIMac::GetClassGlobal(StringRef Name, GV->eraseFromParent(); } GV = NewGV; - CGM.getModule().insertGlobalVariable(GV); + CGM.getModule().getGlobalList().push_back(GV); } assert(GV->getLinkage() == L); diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index f3cf0f623d24f..73a49e552e3d2 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -406,7 +406,7 @@ void IRExecutionUnit::GetRunnableInfo(Status &error, lldb::addr_t &func_addr, } }; - for (llvm::GlobalVariable &global_var : m_module->globals()) { + for (llvm::GlobalVariable &global_var : m_module->getGlobalList()) { RegisterOneValue(global_var); } diff --git a/llvm/docs/ProgrammersManual.rst b/llvm/docs/ProgrammersManual.rst index 71298f4a87828..00d51a5a05b85 100644 --- a/llvm/docs/ProgrammersManual.rst +++ b/llvm/docs/ProgrammersManual.rst @@ -3429,14 +3429,17 @@ Important Public Members of the ``Module`` class * | ``Module::global_iterator`` - Typedef for global variable list iterator | ``Module::const_global_iterator`` - Typedef for const_iterator. - | ``Module::insertGlobalVariable()`` - Inserts a global variable to the list. - | ``Module::removeGlobalVariable()`` - Removes a global variable frome the list. - | ``Module::eraseGlobalVariable()`` - Removes a global variable frome the list and deletes it. | ``global_begin()``, ``global_end()``, ``global_size()``, ``global_empty()`` These are forwarding methods that make it easy to access the contents of a ``Module`` object's GlobalVariable_ list. +* ``Module::GlobalListType &getGlobalList()`` + + Returns the list of GlobalVariable_\ s. This is necessary to use when you + need to update the list or perform a complex action that doesn't have a + forwarding method. + ---------------- * ``SymbolTable *getSymbolTable()`` diff --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h index aacdbbf08b945..dbd3eae0f134c 100644 --- a/llvm/include/llvm/IR/Module.h +++ b/llvm/include/llvm/IR/Module.h @@ -542,24 +542,6 @@ class LLVM_EXTERNAL_VISIBILITY Module { llvm::Error materializeMetadata(); - /// Detach global variable \p GV from the list but don't delete it. - void removeGlobalVariable(GlobalVariable *GV) { GlobalList.remove(GV); } - /// Remove global variable \p GV from the list and delete it. - void eraseGlobalVariable(GlobalVariable *GV) { GlobalList.erase(GV); } - /// Insert global variable \p GV at the end of the global variable list and - /// take ownership. - void insertGlobalVariable(GlobalVariable *GV) { - insertGlobalVariable(GlobalList.end(), GV); - } - /// Insert global variable \p GV into the global variable list before \p - /// Where and take ownership. - void insertGlobalVariable(GlobalListType::iterator Where, GlobalVariable *GV) { - GlobalList.insert(Where, GV); - } - // Use global_size() to get the total number of global variables. - // Use globals() to get the range of all global variables. - -private: /// @} /// @name Direct access to the globals list, functions list, and symbol table /// @{ @@ -572,9 +554,7 @@ class LLVM_EXTERNAL_VISIBILITY Module { static GlobalListType Module::*getSublistAccess(GlobalVariable*) { return &Module::GlobalList; } - friend class llvm::SymbolTableListTraits; -public: /// Get the Module's list of functions (constant). const FunctionListType &getFunctionList() const { return FunctionList; } /// Get the Module's list of functions. diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 35607df04495b..ef9a486eb5c90 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -3693,7 +3693,7 @@ Error BitcodeReader::globalCleanup() { UpgradedVariables.emplace_back(&GV, Upgraded); for (auto &Pair : UpgradedVariables) { Pair.first->eraseFromParent(); - TheModule->insertGlobalVariable(Pair.second); + TheModule->getGlobalList().push_back(Pair.second); } // Force deallocation of memory for these vectors to favor the client that diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index 9381f42454f8b..b21a9a7eeb0d1 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -571,7 +571,7 @@ Constant *OpenMPIRBuilder::getOrCreateIdent(Constant *SrcLocStr, // Look for existing encoding of the location + flags, not needed but // minimizes the difference to the existing solution while we transition. - for (GlobalVariable &GV : M.globals()) + for (GlobalVariable &GV : M.getGlobalList()) if (GV.getValueType() == OpenMPIRBuilder::Ident && GV.hasInitializer()) if (GV.getInitializer() == Initializer) Ident = &GV; @@ -601,7 +601,7 @@ Constant *OpenMPIRBuilder::getOrCreateSrcLocStr(StringRef LocStr, // Look for existing encoding of the location, not needed but minimizes the // difference to the existing solution while we transition. - for (GlobalVariable &GV : M.globals()) + for (GlobalVariable &GV : M.getGlobalList()) if (GV.isConstant() && GV.hasInitializer() && GV.getInitializer() == Initializer) return SrcLocStr = ConstantExpr::getPointerCast(&GV, Int8Ptr); diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp index a37143fa69161..667f591c9c1cb 100644 --- a/llvm/lib/IR/Globals.cpp +++ b/llvm/lib/IR/Globals.cpp @@ -456,17 +456,17 @@ GlobalVariable::GlobalVariable(Module &M, Type *Ty, bool constant, } if (Before) - Before->getParent()->insertGlobalVariable(Before->getIterator(), this); + Before->getParent()->getGlobalList().insert(Before->getIterator(), this); else - M.insertGlobalVariable(this); + M.getGlobalList().push_back(this); } void GlobalVariable::removeFromParent() { - getParent()->removeGlobalVariable(this); + getParent()->getGlobalList().remove(getIterator()); } void GlobalVariable::eraseFromParent() { - getParent()->eraseGlobalVariable(this); + getParent()->getGlobalList().erase(getIterator()); } void GlobalVariable::setInitializer(Constant *InitVal) { diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp index 721acb1ea02ca..51fe687349416 100644 --- a/llvm/lib/Linker/IRMover.cpp +++ b/llvm/lib/Linker/IRMover.cpp @@ -1671,16 +1671,15 @@ Error IRLinker::run() { // Reorder the globals just added to the destination module to match their // original order in the source module. + Module::GlobalListType &Globals = DstM.getGlobalList(); for (GlobalVariable &GV : SrcM->globals()) { if (GV.hasAppendingLinkage()) continue; Value *NewValue = Mapper.mapValue(GV); if (NewValue) { auto *NewGV = dyn_cast(NewValue->stripPointerCasts()); - if (NewGV) { - NewGV->removeFromParent(); - DstM.insertGlobalVariable(NewGV); - } + if (NewGV) + Globals.splice(Globals.end(), Globals, NewGV->getIterator()); } } diff --git a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp index a4d77994dec5c..9fa3fb5cb211f 100644 --- a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp +++ b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp @@ -826,7 +826,8 @@ void NVPTXAsmPrinter::emitGlobals(const Module &M) { for (const GlobalVariable &I : M.globals()) VisitGlobalVariableForEmission(&I, Globals, GVVisited, GVVisiting); - assert(GVVisited.size() == M.global_size() && "Missed a global variable"); + assert(GVVisited.size() == M.getGlobalList().size() && + "Missed a global variable"); assert(GVVisiting.size() == 0 && "Did not fully process a global variable"); const NVPTXTargetMachine &NTM = static_cast(TM); diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index 11621d9b67a9d..6f35da193926f 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -976,7 +976,7 @@ OptimizeGlobalAddressOfAllocation(GlobalVariable *GV, CallInst *CI, cast(InitBool->user_back())->eraseFromParent(); delete InitBool; } else - GV->getParent()->insertGlobalVariable(GV->getIterator(), InitBool); + GV->getParent()->getGlobalList().insert(GV->getIterator(), InitBool); // Now the GV is dead, nuke it and the allocation.. GV->eraseFromParent(); @@ -1158,7 +1158,7 @@ static bool TryToShrinkGlobalToBoolean(GlobalVariable *GV, Constant *OtherVal) { GV->getThreadLocalMode(), GV->getType()->getAddressSpace()); NewGV->copyAttributesFrom(GV); - GV->getParent()->insertGlobalVariable(GV->getIterator(), NewGV); + GV->getParent()->getGlobalList().insert(GV->getIterator(), NewGV); Constant *InitVal = GV->getInitializer(); assert(InitVal->getType() != Type::getInt1Ty(GV->getContext()) && diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp index c8665a9e91736..9675ec025cf9e 100644 --- a/llvm/lib/Transforms/IPO/SCCP.cpp +++ b/llvm/lib/Transforms/IPO/SCCP.cpp @@ -370,7 +370,7 @@ static bool runIPSCCP( SI->eraseFromParent(); MadeChanges = true; } - M.eraseGlobalVariable(GV); + M.getGlobalList().erase(GV); ++NumGlobalConst; } @@ -407,72 +407,3 @@ PreservedAnalyses IPSCCPPass::run(Module &M, ModuleAnalysisManager &AM) { PA.preserve(); return PA; } - -namespace { - -//===--------------------------------------------------------------------===// -// -/// IPSCCP Class - This class implements interprocedural Sparse Conditional -/// Constant Propagation. -/// -class IPSCCPLegacyPass : public ModulePass { -public: - static char ID; - - IPSCCPLegacyPass() : ModulePass(ID) { - initializeIPSCCPLegacyPassPass(*PassRegistry::getPassRegistry()); - } - - bool runOnModule(Module &M) override { - if (skipModule(M)) - return false; - const DataLayout &DL = M.getDataLayout(); - auto GetTLI = [this](Function &F) -> const TargetLibraryInfo & { - return this->getAnalysis().getTLI(F); - }; - auto GetTTI = [this](Function &F) -> TargetTransformInfo & { - return this->getAnalysis().getTTI(F); - }; - auto GetAC = [this](Function &F) -> AssumptionCache & { - return this->getAnalysis().getAssumptionCache(F); - }; - auto getAnalysis = [this](Function &F) -> AnalysisResultsForFn { - DominatorTree &DT = - this->getAnalysis(F).getDomTree(); - return { - std::make_unique( - F, DT, - this->getAnalysis().getAssumptionCache( - F)), - nullptr, // We cannot preserve the LI, DT or PDT with the legacy pass - nullptr, // manager, so set them to nullptr. - nullptr}; - }; - - return runIPSCCP(M, DL, nullptr, GetTLI, GetTTI, GetAC, getAnalysis, false); - } - - void getAnalysisUsage(AnalysisUsage &AU) const override { - AU.addRequired(); - AU.addRequired(); - AU.addRequired(); - AU.addRequired(); - } -}; - -} // end anonymous namespace - -char IPSCCPLegacyPass::ID = 0; - -INITIALIZE_PASS_BEGIN(IPSCCPLegacyPass, "ipsccp", - "Interprocedural Sparse Conditional Constant Propagation", - false, false) -INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker) -INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) -INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass) -INITIALIZE_PASS_END(IPSCCPLegacyPass, "ipsccp", - "Interprocedural Sparse Conditional Constant Propagation", - false, false) - -// createIPSCCPPass - This is the public interface to this file. -ModulePass *llvm::createIPSCCPPass() { return new IPSCCPLegacyPass(); } diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp index 7410b37c2d9b0..080ee7665e29e 100644 --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -958,7 +958,7 @@ void DevirtModule::buildTypeIdentifierMap( std::vector &Bits, DenseMap> &TypeIdMap) { DenseMap GVToBits; - Bits.reserve(M.global_size()); + Bits.reserve(M.getGlobalList().size()); SmallVector Types; for (GlobalVariable &GV : M.globals()) { Types.clear(); diff --git a/llvm/lib/Transforms/Utils/CtorUtils.cpp b/llvm/lib/Transforms/Utils/CtorUtils.cpp index e07c92df2265e..c997f39508e35 100644 --- a/llvm/lib/Transforms/Utils/CtorUtils.cpp +++ b/llvm/lib/Transforms/Utils/CtorUtils.cpp @@ -48,7 +48,7 @@ static void removeGlobalCtors(GlobalVariable *GCL, const BitVector &CtorsToRemov GlobalVariable *NGV = new GlobalVariable(CA->getType(), GCL->isConstant(), GCL->getLinkage(), CA, "", GCL->getThreadLocalMode()); - GCL->getParent()->insertGlobalVariable(GCL->getIterator(), NGV); + GCL->getParent()->getGlobalList().insert(GCL->getIterator(), NGV); NGV->takeName(GCL); // Nuke the old list, replacing any uses with the new one. diff --git a/llvm/unittests/IR/ModuleTest.cpp b/llvm/unittests/IR/ModuleTest.cpp index da684b85a4dfb..af899c33f0da8 100644 --- a/llvm/unittests/IR/ModuleTest.cpp +++ b/llvm/unittests/IR/ModuleTest.cpp @@ -46,6 +46,8 @@ TEST(ModuleTest, sortGlobalsByName) { // Sort the globals by name. EXPECT_FALSE(std::is_sorted(M.global_begin(), M.global_end(), compare)); + M.getGlobalList().sort(compare); + EXPECT_TRUE(std::is_sorted(M.global_begin(), M.global_end(), compare)); } } @@ -271,43 +273,4 @@ TEST(ModuleTest, NamedMDList) { EXPECT_EQ(M->named_metadata_size(), 2u); } -TEST(ModuleTest, GlobalList) { - // This tests all Module's functions that interact with Module::GlobalList. - LLVMContext C; - SMDiagnostic Err; - LLVMContext Context; - std::unique_ptr M = parseAssemblyString(R"( -@GV = external global i32 -)", - Err, Context); - auto *GV = cast(M->getNamedValue("GV")); - EXPECT_EQ(M->global_size(), 1u); - GlobalVariable *NewGV = new GlobalVariable( - Type::getInt32Ty(C), /*isConstant=*/true, GlobalValue::InternalLinkage, - /*Initializer=*/nullptr, "NewGV"); - EXPECT_EQ(M->global_size(), 1u); - // Insert before - M->insertGlobalVariable(M->globals().begin(), NewGV); - EXPECT_EQ(M->global_size(), 2u); - EXPECT_EQ(&*M->globals().begin(), NewGV); - // Insert at end() - M->removeGlobalVariable(NewGV); - EXPECT_EQ(M->global_size(), 1u); - M->insertGlobalVariable(NewGV); - EXPECT_EQ(M->global_size(), 2u); - EXPECT_EQ(&*std::prev(M->globals().end()), NewGV); - // Check globals() - auto Range = M->globals(); - EXPECT_EQ(&*Range.begin(), GV); - EXPECT_EQ(&*std::next(Range.begin()), NewGV); - EXPECT_EQ(std::next(Range.begin(), 2), Range.end()); - // Check remove - M->removeGlobalVariable(NewGV); - EXPECT_EQ(M->global_size(), 1u); - // Check erase - M->insertGlobalVariable(NewGV); - M->eraseGlobalVariable(NewGV); - EXPECT_EQ(M->global_size(), 1u); -} - } // end namespace