[Lldb-commits] [PATCH] D139910: [NFC] Cleanup: Remove Function::getBasicBlockList() when not required.

2022-12-12 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo created this revision.
vporpo added reviewers: aeubanks, asbirlea.
Herald added subscribers: Moerafaat, zero9178, Enna1, bzcheeseman, mattd, 
awarzynski, sdasgup3, wenzhicui, wrengr, ormris, cota, teijeong, rdzhabarov, 
tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, 
mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini, hiraditya.
Herald added a reviewer: ftynse.
Herald added a project: All.
vporpo requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, 
stephenneuendorffer, nicolasvasilache.
Herald added a reviewer: dcaballe.
Herald added projects: clang, LLDB, MLIR, LLVM.

This is part of a series of patches that aim at making 
Function::getBasicBlockList() private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139910

Files:
  clang/lib/CodeGen/CGVTables.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
  llvm/lib/Target/AArch64/SMEABIPass.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/IPO/InlineSimple.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/unittests/FuzzMutate/StrategiesTest.cpp
  llvm/unittests/IR/InstructionsTest.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -321,8 +321,7 @@
   blocks.insert(traversal.begin(), traversal.end());
 }
   }
-  assert(blocks.size() == func->getBasicBlockList().size() &&
- "some blocks are not sorted");
+  assert(blocks.size() == func->size() && "some blocks are not sorted");
 
   return blocks;
 }
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -1516,7 +1516,7 @@
 }
 )");
   Function *Foo = M->getFunction("foo");
-  auto BBs = Foo->getBasicBlockList().begin();
+  auto BBs = Foo->begin();
   CallBrInst &CBI = cast(BBs->front());
   ++BBs;
   ++BBs;
Index: llvm/unittests/FuzzMutate/StrategiesTest.cpp
===
--- llvm/unittests/FuzzMutate/StrategiesTest.cpp
+++ llvm/unittests/FuzzMutate/StrategiesTest.cpp
@@ -505,14 +505,14 @@
   std::unique_ptr M = parseAssembly(Source.data(), Ctx);
   Function *F = &*M->begin();
   DenseMap PreShuffleInstCnt;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 PreShuffleInstCnt.insert({&BB, BB.size()});
   }
   std::mt19937 mt(Seed);
   std::uniform_int_distribution RandInt(INT_MIN, INT_MAX);
   for (int i = 0; i < 100; i++) {
 Mutator->mutateModule(*M, RandInt(mt), Source.size(), Source.size() + 1024);
-for (BasicBlock &BB : F->getBasicBlockList()) {
+for (BasicBlock &BB : *F) {
   int PostShuffleIntCnt = BB.size();
   EXPECT_EQ(PostShuffleIntCnt, PreShuffleInstCnt[&BB]);
 }
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2758,7 +2758,7 @@
 OrigBB->splice(CB.getIterator(), &*FirstNewBlock, FirstNewBlock->begin(),
FirstNewBlock->end());
 // Remove the cloned basic block.
-Caller->getBasicBlockList().pop_back();
+Caller->back().eraseFromParent();
 
 // If the call site was an invoke instruction, add a branch to the normal
 // destination.
@@ -2932,7 +2932,7 @@
   Br->eraseFromParent();
 
   // Now we can remove the CalleeEntry block, which is now empty.
-  Caller->getBasicBlockList().erase(CalleeEntry);
+  CalleeEntry->eraseFromParent();
 
   // If we inserted a phi node, check to see if it has a single value (e.g. all
   // the entries are the same or undef).  If so, remove the PHI so it doesn't
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -381,7 +381,7 @@
   BasicBlock *RBB = RBA->getBasicBlock();
   if (LBB == RBB)
 return 0;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 if (&BB == LBB) {
   assert(&BB != RBB);
   return -1;
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Tran

[Lldb-commits] [PATCH] D139910: [NFC] Cleanup: Remove Function::getBasicBlockList() when not required.

2022-12-13 Thread Vasileios Porpodas via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGadfb23c607ce: [NFC] Cleanup: Remove 
Function::getBasicBlockList() when not required. (authored by vporpo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139910/new/

https://reviews.llvm.org/D139910

Files:
  clang/lib/CodeGen/CGVTables.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
  llvm/lib/Target/AArch64/SMEABIPass.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/IPO/InlineSimple.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/unittests/FuzzMutate/StrategiesTest.cpp
  llvm/unittests/IR/InstructionsTest.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -321,8 +321,7 @@
   blocks.insert(traversal.begin(), traversal.end());
 }
   }
-  assert(blocks.size() == func->getBasicBlockList().size() &&
- "some blocks are not sorted");
+  assert(blocks.size() == func->size() && "some blocks are not sorted");
 
   return blocks;
 }
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -1516,7 +1516,7 @@
 }
 )");
   Function *Foo = M->getFunction("foo");
-  auto BBs = Foo->getBasicBlockList().begin();
+  auto BBs = Foo->begin();
   CallBrInst &CBI = cast(BBs->front());
   ++BBs;
   ++BBs;
Index: llvm/unittests/FuzzMutate/StrategiesTest.cpp
===
--- llvm/unittests/FuzzMutate/StrategiesTest.cpp
+++ llvm/unittests/FuzzMutate/StrategiesTest.cpp
@@ -505,14 +505,14 @@
   std::unique_ptr M = parseAssembly(Source.data(), Ctx);
   Function *F = &*M->begin();
   DenseMap PreShuffleInstCnt;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 PreShuffleInstCnt.insert({&BB, BB.size()});
   }
   std::mt19937 mt(Seed);
   std::uniform_int_distribution RandInt(INT_MIN, INT_MAX);
   for (int i = 0; i < 100; i++) {
 Mutator->mutateModule(*M, RandInt(mt), Source.size(), Source.size() + 1024);
-for (BasicBlock &BB : F->getBasicBlockList()) {
+for (BasicBlock &BB : *F) {
   int PostShuffleIntCnt = BB.size();
   EXPECT_EQ(PostShuffleIntCnt, PreShuffleInstCnt[&BB]);
 }
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2758,7 +2758,7 @@
 OrigBB->splice(CB.getIterator(), &*FirstNewBlock, FirstNewBlock->begin(),
FirstNewBlock->end());
 // Remove the cloned basic block.
-Caller->getBasicBlockList().pop_back();
+Caller->back().eraseFromParent();
 
 // If the call site was an invoke instruction, add a branch to the normal
 // destination.
@@ -2932,7 +2932,7 @@
   Br->eraseFromParent();
 
   // Now we can remove the CalleeEntry block, which is now empty.
-  Caller->getBasicBlockList().erase(CalleeEntry);
+  CalleeEntry->eraseFromParent();
 
   // If we inserted a phi node, check to see if it has a single value (e.g. all
   // the entries are the same or undef).  If so, remove the PHI so it doesn't
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -381,7 +381,7 @@
   BasicBlock *RBB = RBA->getBasicBlock();
   if (LBB == RBB)
 return 0;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 if (&BB == LBB) {
   assert(&BB != RBB);
   return -1;
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1692,7 +1692,7 @@
   IRB.CreateCall(AsanPoisonGlobals, ModuleNameAddr);
 
   // Add calls to unpoison all globals before each return instruction.
-  for (auto &BB : GlobalInit.getBasicBlockList())
+  for (auto &BB : GlobalInit)
 if (ReturnInst *RI = dyn_cast(BB.getTerminator()))
   CallInst::Create(AsanUnpoisonGlobals, "", RI);
 }
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
==

[Lldb-commits] [PATCH] D143958: [NFC][IR] Make Module::getAliasList() private

2023-02-13 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo created this revision.
vporpo added a reviewer: aeubanks.
Herald added subscribers: ormris, jeroen.dobbelaere, hiraditya.
Herald added a project: All.
vporpo requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

This patch adds several missing AliasList modifier functions, like
removeAlias(), eraseAlias() and insertAlias().
There is no longer need to access the list directly so it also makes
getAliaList() private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143958

Files:
  lldb/source/Expression/IRExecutionUnit.cpp
  llvm/include/llvm/IR/Module.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/unittests/IR/ModuleTest.cpp

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -159,4 +159,44 @@
   EXPECT_EQ(Ratio, ProfileSummary->getPartialProfileRatio());
 }
 
+TEST(ModuleTest, AliasList) {
+  // This tests all Module's functions that interact with Module::AliasList.
+  LLVMContext C;
+  SMDiagnostic Err;
+  LLVMContext Context;
+  std::unique_ptr M = parseAssemblyString(R"(
+declare void @Foo()
+@GA = alias void (), ptr @Foo
+)",
+  Err, Context);
+  Function *Foo = M->getFunction("Foo");
+  auto *GA = M->getNamedAlias("GA");
+  EXPECT_EQ(M->alias_size(), 1u);
+  auto *NewGA =
+  GlobalAlias::create(Foo->getType(), 0, GlobalValue::ExternalLinkage,
+  "NewGA", Foo, /*Parent=*/nullptr);
+  EXPECT_EQ(M->alias_size(), 1u);
+
+  M->insertAlias(M->aliases().begin(), NewGA);
+  EXPECT_EQ(&*M->aliases().begin(), NewGA);
+
+  M->removeAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+  M->insertAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 2u);
+  EXPECT_EQ(&*std::prev(M->aliases().end()), NewGA);
+
+  auto Range = M->aliases();
+  EXPECT_EQ(&*Range.begin(), GA);
+  EXPECT_EQ(&*std::next(Range.begin()), NewGA);
+  EXPECT_EQ(std::next(Range.begin(), 2), Range.end());
+
+  M->removeAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+
+  M->insertAlias(NewGA);
+  M->eraseAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+}
+
 } // end namespace
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2360,7 +2360,7 @@
   continue;
 
 // Delete the alias.
-M.getAliasList().erase(&J);
+M.eraseAlias(&J);
 ++NumAliasesRemoved;
 Changed = true;
   }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -514,7 +514,7 @@
   AddressSpace) {
   setAliasee(Aliasee);
   if (ParentModule)
-ParentModule->getAliasList().push_back(this);
+ParentModule->insertAlias(this);
 }
 
 GlobalAlias *GlobalAlias::create(Type *Ty, unsigned AddressSpace,
@@ -546,11 +546,11 @@
 }
 
 void GlobalAlias::removeFromParent() {
-  getParent()->getAliasList().remove(getIterator());
+  getParent()->removeAlias(this);
 }
 
 void GlobalAlias::eraseFromParent() {
-  getParent()->getAliasList().erase(getIterator());
+  getParent()->eraseAlias(this);
 }
 
 void GlobalAlias::setAliasee(Constant *Aliasee) {
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1141,7 +1141,7 @@
 
   // Insert into the module, we know its name won't collide now.
   if (IsAlias)
-M->getAliasList().push_back(GA.release());
+M->insertAlias(GA.release());
   else
 M->getIFuncList().push_back(GI.release());
   assert(GV->getName() == Name && "Should not be a name conflict!");
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -563,6 +563,20 @@
 return &Module::FunctionList;
   }
 
+  /// Detaches \p Alias from the list but does not delete it.
+  void removeAlias(GlobalAlias *Alias) { AliasList.remove(Alias); }
+  /// Removes \p Alias from the list and deletes it.
+  void eraseAlias(GlobalAlias *Alias) { AliasList.erase(Alias); }
+  /// Inserts \p Alias at the end of the alias list and takes ownership.
+  void insertAlias(GlobalAlias *Alias) { insertAlias(AliasList.end(), Alias); }
+  /// Inserts \p Alias into the alias list before \p Where and takes ownership.
+  void insertAlias(AliasListType::iterator Where, GlobalAlias *Alias) {
+AliasList.insert(Where, Alias);
+  }
+  // Please use alias_size() to get the size of AliasList.
+  // Please use aliases() to get a range of all tha Alias objects in AliasList.
+
+private: // Please use functions like insertAlias(), removeA

[Lldb-commits] [PATCH] D143958: [NFC][IR] Make Module::getAliasList() private

2023-02-13 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo added inline comments.



Comment at: llvm/include/llvm/IR/Module.h:573
+  /// Inserts \p Alias into the alias list before \p Where and takes ownership.
+  void insertAlias(AliasListType::iterator Where, GlobalAlias *Alias) {
+AliasList.insert(Where, Alias);

aeubanks wrote:
> this isn't used outside tests, remove?
Isn't it better to keep it for completeness? I don't feel strongly about it 
though, either way is fine with me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143958/new/

https://reviews.llvm.org/D143958

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143958: [NFC][IR] Make Module::getAliasList() private

2023-02-13 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo updated this revision to Diff 497150.
vporpo marked an inline comment as done.
vporpo added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143958/new/

https://reviews.llvm.org/D143958

Files:
  lldb/source/Expression/IRExecutionUnit.cpp
  llvm/include/llvm/IR/Module.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/unittests/IR/ModuleTest.cpp

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -159,4 +159,44 @@
   EXPECT_EQ(Ratio, ProfileSummary->getPartialProfileRatio());
 }
 
+TEST(ModuleTest, AliasList) {
+  // This tests all Module's functions that interact with Module::AliasList.
+  LLVMContext C;
+  SMDiagnostic Err;
+  LLVMContext Context;
+  std::unique_ptr M = parseAssemblyString(R"(
+declare void @Foo()
+@GA = alias void (), ptr @Foo
+)",
+  Err, Context);
+  Function *Foo = M->getFunction("Foo");
+  auto *GA = M->getNamedAlias("GA");
+  EXPECT_EQ(M->alias_size(), 1u);
+  auto *NewGA =
+  GlobalAlias::create(Foo->getType(), 0, GlobalValue::ExternalLinkage,
+  "NewGA", Foo, /*Parent=*/nullptr);
+  EXPECT_EQ(M->alias_size(), 1u);
+
+  M->insertAlias(NewGA);
+  EXPECT_EQ(&*std::prev(M->aliases().end()), NewGA);
+
+  M->removeAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+  M->insertAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 2u);
+  EXPECT_EQ(&*std::prev(M->aliases().end()), NewGA);
+
+  auto Range = M->aliases();
+  EXPECT_EQ(&*Range.begin(), GA);
+  EXPECT_EQ(&*std::next(Range.begin()), NewGA);
+  EXPECT_EQ(std::next(Range.begin(), 2), Range.end());
+
+  M->removeAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+
+  M->insertAlias(NewGA);
+  M->eraseAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+}
+
 } // end namespace
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2360,7 +2360,7 @@
   continue;
 
 // Delete the alias.
-M.getAliasList().erase(&J);
+M.eraseAlias(&J);
 ++NumAliasesRemoved;
 Changed = true;
   }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -514,7 +514,7 @@
   AddressSpace) {
   setAliasee(Aliasee);
   if (ParentModule)
-ParentModule->getAliasList().push_back(this);
+ParentModule->insertAlias(this);
 }
 
 GlobalAlias *GlobalAlias::create(Type *Ty, unsigned AddressSpace,
@@ -546,11 +546,11 @@
 }
 
 void GlobalAlias::removeFromParent() {
-  getParent()->getAliasList().remove(getIterator());
+  getParent()->removeAlias(this);
 }
 
 void GlobalAlias::eraseFromParent() {
-  getParent()->getAliasList().erase(getIterator());
+  getParent()->eraseAlias(this);
 }
 
 void GlobalAlias::setAliasee(Constant *Aliasee) {
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1141,7 +1141,7 @@
 
   // Insert into the module, we know its name won't collide now.
   if (IsAlias)
-M->getAliasList().push_back(GA.release());
+M->insertAlias(GA.release());
   else
 M->getIFuncList().push_back(GI.release());
   assert(GV->getName() == Name && "Should not be a name conflict!");
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -563,6 +563,16 @@
 return &Module::FunctionList;
   }
 
+  /// Detach \p Alias from the list but don't delete it.
+  void removeAlias(GlobalAlias *Alias) { AliasList.remove(Alias); }
+  /// Remove \p Alias from the list and delete it.
+  void eraseAlias(GlobalAlias *Alias) { AliasList.erase(Alias); }
+  /// Insert \p Alias at the end of the alias list and take ownership.
+  void insertAlias(GlobalAlias *Alias) { AliasList.insert(AliasList.end(), Alias); }
+  // Use alias_size() to get the size of AliasList.
+  // Use aliases() to get a range of all Alias objects in AliasList.
+
+private: // Please use functions like insertAlias(), removeAlias() etc.
   /// Get the Module's list of aliases (constant).
   const AliasListType&getAliasList() const{ return AliasList; }
   /// Get the Module's list of aliases.
@@ -571,7 +581,9 @@
   static AliasListType Module::*getSublistAccess(GlobalAlias*) {
 return &Module::AliasList;
   }
+  friend class llvm::SymbolTableListTraits;
 
+public:
   /// Get the Module's list of ifuncs (constant).
   const IFuncListType&getIFuncList() const{ return IFuncList; }
   /// Get the Module's list of ifuncs.
I

[Lldb-commits] [PATCH] D143958: [NFC][IR] Make Module::getAliasList() private

2023-02-13 Thread Vasileios Porpodas via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb64f7d028bdc: [NFC][IR] Make Module::getAliasList() private 
(authored by vporpo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143958/new/

https://reviews.llvm.org/D143958

Files:
  lldb/source/Expression/IRExecutionUnit.cpp
  llvm/include/llvm/IR/Module.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/unittests/IR/ModuleTest.cpp

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -159,4 +159,44 @@
   EXPECT_EQ(Ratio, ProfileSummary->getPartialProfileRatio());
 }
 
+TEST(ModuleTest, AliasList) {
+  // This tests all Module's functions that interact with Module::AliasList.
+  LLVMContext C;
+  SMDiagnostic Err;
+  LLVMContext Context;
+  std::unique_ptr M = parseAssemblyString(R"(
+declare void @Foo()
+@GA = alias void (), ptr @Foo
+)",
+  Err, Context);
+  Function *Foo = M->getFunction("Foo");
+  auto *GA = M->getNamedAlias("GA");
+  EXPECT_EQ(M->alias_size(), 1u);
+  auto *NewGA =
+  GlobalAlias::create(Foo->getType(), 0, GlobalValue::ExternalLinkage,
+  "NewGA", Foo, /*Parent=*/nullptr);
+  EXPECT_EQ(M->alias_size(), 1u);
+
+  M->insertAlias(NewGA);
+  EXPECT_EQ(&*std::prev(M->aliases().end()), NewGA);
+
+  M->removeAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+  M->insertAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 2u);
+  EXPECT_EQ(&*std::prev(M->aliases().end()), NewGA);
+
+  auto Range = M->aliases();
+  EXPECT_EQ(&*Range.begin(), GA);
+  EXPECT_EQ(&*std::next(Range.begin()), NewGA);
+  EXPECT_EQ(std::next(Range.begin(), 2), Range.end());
+
+  M->removeAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+
+  M->insertAlias(NewGA);
+  M->eraseAlias(NewGA);
+  EXPECT_EQ(M->alias_size(), 1u);
+}
+
 } // end namespace
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2360,7 +2360,7 @@
   continue;
 
 // Delete the alias.
-M.getAliasList().erase(&J);
+M.eraseAlias(&J);
 ++NumAliasesRemoved;
 Changed = true;
   }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -514,7 +514,7 @@
   AddressSpace) {
   setAliasee(Aliasee);
   if (ParentModule)
-ParentModule->getAliasList().push_back(this);
+ParentModule->insertAlias(this);
 }
 
 GlobalAlias *GlobalAlias::create(Type *Ty, unsigned AddressSpace,
@@ -546,11 +546,11 @@
 }
 
 void GlobalAlias::removeFromParent() {
-  getParent()->getAliasList().remove(getIterator());
+  getParent()->removeAlias(this);
 }
 
 void GlobalAlias::eraseFromParent() {
-  getParent()->getAliasList().erase(getIterator());
+  getParent()->eraseAlias(this);
 }
 
 void GlobalAlias::setAliasee(Constant *Aliasee) {
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1141,7 +1141,7 @@
 
   // Insert into the module, we know its name won't collide now.
   if (IsAlias)
-M->getAliasList().push_back(GA.release());
+M->insertAlias(GA.release());
   else
 M->getIFuncList().push_back(GI.release());
   assert(GV->getName() == Name && "Should not be a name conflict!");
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -563,6 +563,16 @@
 return &Module::FunctionList;
   }
 
+  /// Detach \p Alias from the list but don't delete it.
+  void removeAlias(GlobalAlias *Alias) { AliasList.remove(Alias); }
+  /// Remove \p Alias from the list and delete it.
+  void eraseAlias(GlobalAlias *Alias) { AliasList.erase(Alias); }
+  /// Insert \p Alias at the end of the alias list and take ownership.
+  void insertAlias(GlobalAlias *Alias) { AliasList.insert(AliasList.end(), Alias); }
+  // Use alias_size() to get the size of AliasList.
+  // Use aliases() to get a range of all Alias objects in AliasList.
+
+private: // Please use functions like insertAlias(), removeAlias() etc.
   /// Get the Module's list of aliases (constant).
   const AliasListType&getAliasList() const{ return AliasList; }
   /// Get the Module's list of aliases.
@@ -571,7 +581,9 @@
   static AliasListType Module::*getSublistAccess(GlobalAlias*) {
 return &Module::AliasList;
   }
+  friend class llvm::SymbolTableListTraits;
 
+public:
   /// Get the Module's list of ifuncs (constant).
   const IFuncLis

[Lldb-commits] [PATCH] D144027: [NFC][IR] Make Module::getGlobalList() private

2023-02-14 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo created this revision.
vporpo added a reviewer: aeubanks.
Herald added subscribers: mattd, gchakrabarti, asavonic, snehasish, ormris, 
hiraditya.
Herald added a project: All.
vporpo requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, jholewinski.
Herald added projects: clang, LLDB, LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144027

Files:
  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

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -46,8 +46,11 @@
 
 // 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));
+// I removed this because it is testing whether ilist.sort() works, which is
+// not Module-specific. This requires access to the full GlobalList for no
+// real reason.
+// M.getGlobalList().sort(compare);
+// EXPECT_TRUE(std::is_sorted(M.global_begin(), M.global_end(), compare));
   }
 }
 
@@ -273,4 +276,43 @@
   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
Index: llvm/lib/Transforms/Utils/CtorUtils.cpp
===
--- llvm/lib/Transforms/Utils/CtorUtils.cpp
+++ llvm/lib/Transforms/Utils/CtorUtils.cpp
@@ -48,7 +48,7 @@
   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.
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -958,7 +958,7 @@
 std::vector &Bits,
 DenseMap> &TypeIdMap) {
   DenseMap GVToBits;
-  Bits.reserve(M.getGlobalList().size());
+  Bits.reserve(M.global_size());
   SmallVector Types;
   for (GlobalVariable &GV : M.globals()) {
 Types.clear();
Index: llvm/lib/Transforms/IPO/SCCP.cpp
===
--- llvm/lib/Transforms/IPO/SCCP.cpp
+++ llvm/lib/Transforms/IPO/SCCP.cpp
@@ -370,7 +370,7 @@
   SI->eraseFromParent();
   MadeChanges = true;
 }
-M.getGlobalList().erase(GV);
+M.eraseGlobalVariable(GV);
 ++NumGlobalConst;
   }
 
@@ -476,4 +476,3 @@
 
 // createIPSCCPPass - This is the public interface to this

[Lldb-commits] [PATCH] D144027: [NFC][IR] Make Module::getGlobalList() private

2023-02-14 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo updated this revision to Diff 497367.
vporpo marked an inline comment as done.
vporpo added a comment.

Removed commented out lines from ModuleTest.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144027/new/

https://reviews.llvm.org/D144027

Files:
  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

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -46,8 +46,6 @@
 
 // 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 @@
   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
Index: llvm/lib/Transforms/Utils/CtorUtils.cpp
===
--- llvm/lib/Transforms/Utils/CtorUtils.cpp
+++ llvm/lib/Transforms/Utils/CtorUtils.cpp
@@ -48,7 +48,7 @@
   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.
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -958,7 +958,7 @@
 std::vector &Bits,
 DenseMap> &TypeIdMap) {
   DenseMap GVToBits;
-  Bits.reserve(M.getGlobalList().size());
+  Bits.reserve(M.global_size());
   SmallVector Types;
   for (GlobalVariable &GV : M.globals()) {
 Types.clear();
Index: llvm/lib/Transforms/IPO/SCCP.cpp
===
--- llvm/lib/Transforms/IPO/SCCP.cpp
+++ llvm/lib/Transforms/IPO/SCCP.cpp
@@ -370,7 +370,7 @@
   SI->eraseFromParent();
   MadeChanges = true;
 }
-M.getGlobalList().erase(GV);
+M.eraseGlobalVariable(GV);
 ++NumGlobalConst;
   }
 
@@ -476,4 +476,3 @@
 
 // createIPSCCPPass - This is the public interface to this file.
 ModulePass *llvm::createIPSCCPPass() { return new IPSCCPLegacyPass(); }
-
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -976,7 +976,7 @@
   cast(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 @@

[Lldb-commits] [PATCH] D144027: [NFC][IR] Make Module::getGlobalList() private

2023-02-14 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo added inline comments.



Comment at: llvm/unittests/IR/ModuleTest.cpp:47-49
 // Sort the globals by name.
 EXPECT_FALSE(std::is_sorted(M.global_begin(), M.global_end(), compare));
-M.getGlobalList().sort(compare);

I removed this because it is testing whether ilist.sort() works, which is not 
Module-specific. This requires access to the full GlobalList for no real reason.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144027/new/

https://reviews.llvm.org/D144027

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D144027: [NFC][IR] Make Module::getGlobalList() private

2023-02-14 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo updated this revision to Diff 497403.
vporpo added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144027/new/

https://reviews.llvm.org/D144027

Files:
  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

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -46,8 +46,6 @@
 
 // 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 @@
   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
Index: llvm/lib/Transforms/Utils/CtorUtils.cpp
===
--- llvm/lib/Transforms/Utils/CtorUtils.cpp
+++ llvm/lib/Transforms/Utils/CtorUtils.cpp
@@ -48,7 +48,7 @@
   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.
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -958,7 +958,7 @@
 std::vector &Bits,
 DenseMap> &TypeIdMap) {
   DenseMap GVToBits;
-  Bits.reserve(M.getGlobalList().size());
+  Bits.reserve(M.global_size());
   SmallVector Types;
   for (GlobalVariable &GV : M.globals()) {
 Types.clear();
Index: llvm/lib/Transforms/IPO/SCCP.cpp
===
--- llvm/lib/Transforms/IPO/SCCP.cpp
+++ llvm/lib/Transforms/IPO/SCCP.cpp
@@ -370,7 +370,7 @@
   SI->eraseFromParent();
   MadeChanges = true;
 }
-M.getGlobalList().erase(GV);
+M.eraseGlobalVariable(GV);
 ++NumGlobalConst;
   }
 
@@ -476,4 +476,3 @@
 
 // createIPSCCPPass - This is the public interface to this file.
 ModulePass *llvm::createIPSCCPPass() { return new IPSCCPLegacyPass(); }
-
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -976,7 +976,7 @@
   cast(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 @@
  GV->getThreadLocalMode(),

[Lldb-commits] [PATCH] D144027: [NFC][IR] Make Module::getGlobalList() private

2023-02-14 Thread Vasileios Porpodas via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGed3e3ee9e30d: [NFC][IR] Make Module::getGlobalList() private 
(authored by vporpo).

Changed prior to commit:
  https://reviews.llvm.org/D144027?vs=497403&id=497445#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144027/new/

https://reviews.llvm.org/D144027

Files:
  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

Index: llvm/unittests/IR/ModuleTest.cpp
===
--- llvm/unittests/IR/ModuleTest.cpp
+++ llvm/unittests/IR/ModuleTest.cpp
@@ -46,8 +46,6 @@
 
 // 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 @@
   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
Index: llvm/lib/Transforms/Utils/CtorUtils.cpp
===
--- llvm/lib/Transforms/Utils/CtorUtils.cpp
+++ llvm/lib/Transforms/Utils/CtorUtils.cpp
@@ -48,7 +48,7 @@
   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.
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -958,7 +958,7 @@
 std::vector &Bits,
 DenseMap> &TypeIdMap) {
   DenseMap GVToBits;
-  Bits.reserve(M.getGlobalList().size());
+  Bits.reserve(M.global_size());
   SmallVector Types;
   for (GlobalVariable &GV : M.globals()) {
 Types.clear();
Index: llvm/lib/Transforms/IPO/SCCP.cpp
===
--- llvm/lib/Transforms/IPO/SCCP.cpp
+++ llvm/lib/Transforms/IPO/SCCP.cpp
@@ -370,7 +370,7 @@
   SI->eraseFromParent();
   MadeChanges = true;
 }
-M.getGlobalList().erase(GV);
+M.eraseGlobalVariable(GV);
 ++NumGlobalConst;
   }
 
@@ -407,3 +407,72 @@
   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.getDataL

[Lldb-commits] [PATCH] D144027: [NFC][IR] Make Module::getGlobalList() private

2023-02-14 Thread Vasileios Porpodas via Phabricator via lldb-commits 
vporpo added a comment.

Yeah sorry about that, I reverted it here: b5f239363a3 Revert "[NFC][IR] Make 
Module::getGlobalList() private"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144027/new/

https://reviews.llvm.org/D144027

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits