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

This reverts commit cb5f239363a3c94db5425c105fcd45e77d2a16a9.

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 5882f491d5972ea36fe632b3ecfdf4a99f7a1b52..e9fa273f21cc8d023a98aa933e47fdaba6f68236 100644 (file)
--- 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 c739d3742f801759848c25c6d3085d02bb226a61..5f4cdc6d91f1d47b2b8cec7d0003f4c15938ff68 100644 (file)
--- 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 73a49e552e3d22014e186b456e1a92812c4abf6f..f3cf0f623d24fc905a5053b022a52da71ed7a6f9 100644 (file)
--- 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 00d51a5a05b857b17abaad6128c49e37720305f0..71298f4a87828c6e094de33452cfc0e9ba638e37 100644 (file)
--- 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 dbd3eae0f134cc89a16727ad2ee3446cd9375b4e..aacdbbf08b9451fc5cc9554ff8ffe57fa859c77a 100644 (file)
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -542,6 +542,24 @@ public:
 
   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 @@ public:
   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 ef9a486eb5c901142cf58ba5c42557ba18cd05e8..35607df04495bf45f87112d7d3af833e35e3499a 100644 (file)
--- 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 b21a9a7eeb0d156c46a7e44caac200748b053eaa..9381f42454f8bf71c21577b833b63e4ca6681d9a 100644 (file)
--- 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.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
     // difference 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 667f591c9c1cba3658441f0d017477b1c72f9ae6..a37143fa69161622bbb272973b96a92231c4e5ee 100644 (file)
--- 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 51fe68734941635d0476b5c081525ece44b1de1a..721acb1ea02ca80fdaddaf6eed1fdb480d2ef8b5 100644 (file)
--- 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 9fa3fb5cb211fc20ca89b446d216ef7a243a50fb..a4d77994dec5c7c57cfa5e94130386bf246d0151 100644 (file)
--- 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 6f35da193926fa154fd24f382491659b32c44a80..11621d9b67a9d51fd01f3520794beb0d28efcc37 100644 (file)
--- 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 9675ec025cf9e74296239eefd3704529ed8793fa..8d19bf6e003fdfc2f7742e4eb120a745f0f4d728 100644 (file)
--- 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;
   }
 

diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index 080ee7665e29e391a15bff0e50e1261011d6e68a..7410b37c2d9b0ba10519644b3a6d7406d97d389d 100644 (file)
--- 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 c997f39508e35b1f9719416a45e6bd765c9f7a57..e07c92df2265ea3219a03275f2c8db0dbe9bbad7 100644 (file)
--- 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 af899c33f0da83f540be8da232478dc139948b13..da684b85a4dfb52245130b4ec1da465c846ef208 100644 (file)
--- 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"(
+@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