[llvm] ed3e3ee - [NFC][IR] Make Module::getGlobalList() private

Vasileios Porpodas via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 14 14:25:43 PST 2023


Author: Vasileios Porpodas
Date: 2023-02-14T14:25:10-08:00
New Revision: ed3e3ee9e30dfbffd2170a770a49b36a7f444916

URL: https://github.com/llvm/llvm-project/commit/ed3e3ee9e30dfbffd2170a770a49b36a7f444916
DIFF: https://github.com/llvm/llvm-project/commit/ed3e3ee9e30dfbffd2170a770a49b36a7f444916.diff

LOG: [NFC][IR] Make Module::getGlobalList() private

This patch adds several missing GlobalList modifier functions, like
removeGlobalVariable(), eraseGlobalVariable() and insertGlobalVariable().
There is no longer need to access the list directly so it also makes
getGlobalList() private.

Differential Revision: https://reviews.llvm.org/D144027

Added: 
    

Modified: 
    clang/lib/CodeGen/CGHLSLRuntime.cpp
    clang/lib/CodeGen/CGObjCMac.cpp
    lldb/source/Expression/IRExecutionUnit.cpp
    llvm/docs/ProgrammersManual.rst
    llvm/include/llvm/IR/Module.h
    llvm/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
    llvm/lib/IR/Globals.cpp
    llvm/lib/Linker/IRMover.cpp
    llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
    llvm/lib/Transforms/IPO/GlobalOpt.cpp
    llvm/lib/Transforms/IPO/SCCP.cpp
    llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
    llvm/lib/Transforms/Utils/CtorUtils.cpp
    llvm/unittests/IR/ModuleTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 5882f491d5972..e9fa273f21cc8 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.getGlobalList().push_back(GV);
+    M.insertGlobalVariable(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 c739d3742f801..5f4cdc6d91f1d 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().getGlobalList().push_back(GV);
+    CGM.getModule().insertGlobalVariable(GV);
   }
 
   assert(GV->getLinkage() == L);

diff  --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp
index 73a49e552e3d2..f3cf0f623d24f 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->getGlobalList()) {
+  for (llvm::GlobalVariable &global_var : m_module->globals()) {
     RegisterOneValue(global_var);
   }
 

diff  --git a/llvm/docs/ProgrammersManual.rst b/llvm/docs/ProgrammersManual.rst
index 00d51a5a05b85..71298f4a87828 100644
--- a/llvm/docs/ProgrammersManual.rst
+++ b/llvm/docs/ProgrammersManual.rst
@@ -3429,17 +3429,14 @@ 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 dbd3eae0f134c..aacdbbf08b945 100644
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -542,6 +542,24 @@ 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
 /// @{
@@ -554,7 +572,9 @@ class LLVM_EXTERNAL_VISIBILITY Module {
   static GlobalListType Module::*getSublistAccess(GlobalVariable*) {
     return &Module::GlobalList;
   }
+  friend class llvm::SymbolTableListTraits<llvm::GlobalVariable>;
 
+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 ef9a486eb5c90..35607df04495b 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->getGlobalList().push_back(Pair.second);
+    TheModule->insertGlobalVariable(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 b21a9a7eeb0d1..9381f42454f8b 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 
diff erence to the existing solution while we transition.
-    for (GlobalVariable &GV : M.getGlobalList())
+    for (GlobalVariable &GV : M.globals())
       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
     // 
diff erence to the existing solution while we transition.
-    for (GlobalVariable &GV : M.getGlobalList())
+    for (GlobalVariable &GV : M.globals())
       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 667f591c9c1cb..a37143fa69161 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()->getGlobalList().insert(Before->getIterator(), this);
+    Before->getParent()->insertGlobalVariable(Before->getIterator(), this);
   else
-    M.getGlobalList().push_back(this);
+    M.insertGlobalVariable(this);
 }
 
 void GlobalVariable::removeFromParent() {
-  getParent()->getGlobalList().remove(getIterator());
+  getParent()->removeGlobalVariable(this);
 }
 
 void GlobalVariable::eraseFromParent() {
-  getParent()->getGlobalList().erase(getIterator());
+  getParent()->eraseGlobalVariable(this);
 }
 
 void GlobalVariable::setInitializer(Constant *InitVal) {

diff  --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 51fe687349416..721acb1ea02ca 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1671,15 +1671,16 @@ 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<GlobalVariable>(NewValue->stripPointerCasts());
-      if (NewGV)
-        Globals.splice(Globals.end(), Globals, NewGV->getIterator());
+      if (NewGV) {
+        NewGV->removeFromParent();
+        DstM.insertGlobalVariable(NewGV);
+      }
     }
   }
 

diff  --git a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
index 9fa3fb5cb211f..a4d77994dec5c 100644
--- a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -826,8 +826,7 @@ void NVPTXAsmPrinter::emitGlobals(const Module &M) {
   for (const GlobalVariable &I : M.globals())
     VisitGlobalVariableForEmission(&I, Globals, GVVisited, GVVisiting);
 
-  assert(GVVisited.size() == M.getGlobalList().size() &&
-         "Missed a global variable");
+  assert(GVVisited.size() == M.global_size() && "Missed a global variable");
   assert(GVVisiting.size() == 0 && "Did not fully process a global variable");
 
   const NVPTXTargetMachine &NTM = static_cast<const NVPTXTargetMachine &>(TM);

diff  --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 6f35da193926f..11621d9b67a9d 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<StoreInst>(InitBool->user_back())->eraseFromParent();
     delete InitBool;
   } else
-    GV->getParent()->getGlobalList().insert(GV->getIterator(), InitBool);
+    GV->getParent()->insertGlobalVariable(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()->getGlobalList().insert(GV->getIterator(), NewGV);
+  GV->getParent()->insertGlobalVariable(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 9675ec025cf9e..c8665a9e91736 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.getGlobalList().erase(GV);
+    M.eraseGlobalVariable(GV);
     ++NumGlobalConst;
   }
 
@@ -407,3 +407,72 @@ PreservedAnalyses IPSCCPPass::run(Module &M, ModuleAnalysisManager &AM) {
   PA.preserve<FunctionAnalysisManagerModuleProxy>();
   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<TargetLibraryInfoWrapperPass>().getTLI(F);
+    };
+    auto GetTTI = [this](Function &F) -> TargetTransformInfo & {
+      return this->getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
+    };
+    auto GetAC = [this](Function &F) -> AssumptionCache & {
+      return this->getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
+    };
+    auto getAnalysis = [this](Function &F) -> AnalysisResultsForFn {
+      DominatorTree &DT =
+          this->getAnalysis<DominatorTreeWrapperPass>(F).getDomTree();
+      return {
+          std::make_unique<PredicateInfo>(
+              F, DT,
+              this->getAnalysis<AssumptionCacheTracker>().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<AssumptionCacheTracker>();
+    AU.addRequired<DominatorTreeWrapperPass>();
+    AU.addRequired<TargetLibraryInfoWrapperPass>();
+    AU.addRequired<TargetTransformInfoWrapperPass>();
+  }
+};
+
+} // 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 080ee7665e29e..7410b37c2d9b0 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -958,7 +958,7 @@ void DevirtModule::buildTypeIdentifierMap(
     std::vector<VTableBits> &Bits,
     DenseMap<Metadata *, std::set<TypeMemberInfo>> &TypeIdMap) {
   DenseMap<GlobalVariable *, VTableBits *> GVToBits;
-  Bits.reserve(M.getGlobalList().size());
+  Bits.reserve(M.global_size());
   SmallVector<MDNode *, 2> 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 c997f39508e35..e07c92df2265e 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()->getGlobalList().insert(GCL->getIterator(), NGV);
+  GCL->getParent()->insertGlobalVariable(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 af899c33f0da8..da684b85a4dfb 100644
--- a/llvm/unittests/IR/ModuleTest.cpp
+++ b/llvm/unittests/IR/ModuleTest.cpp
@@ -46,8 +46,6 @@ 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));
   }
 }
 
@@ -273,4 +271,43 @@ 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<Module> M = parseAssemblyString(R"(
+ at GV = external global i32
+)",
+                                                  Err, Context);
+  auto *GV = cast<GlobalVariable>(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


        


More information about the llvm-commits mailing list