[llvm] daab922 - [IR] Module's NamedMD table needn't be 'void *'

[Brian Gesiak via llvm-commits]

Author: Brian Gesiak
Date: 2020-01-15T18:27:25-05:00
New Revision: daab9227ff013db431e4ab6045bdbba55b3dd4f3

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

LOG: [IR] Module's NamedMD table needn't be 'void *'

Summary:
In July 21 2010 `llvm::NamedMDNode` was refactored such that it would no
longer subclass `llvm::Value`:
https://github.com/llvm/llvm-project/commit/2637cc1a38d7336ea30caf

As part of this change, a map type from metadata names to their named
metadata, `llvm::MDSymbolTable`, was deleted. In its place, the type
of member `llvm::Module::NamedMDSymTab` was changed, from
`llvm::MDSymbolTable` to `void *`. The underlying memory allocations
for this pointer were changed to `new StringMap<NamedMDNode *>()`.

However, as far as I can tell, there's no need for obscuring the
underlying type being pointed to by the `void *`, and no need for
static casts from `void *` to `StringMap`. In fact, I don't think
there's a need for explicit calls to `new` and `delete` at all.

This commit changes `NamedMDSymTab` from a pointer to a reference, which
automatically couples its lifetime with the lifetime of its owning
`llvm::Module` instance, thus removing the explicit calls to `new` and
`delete` in the `llvm::Module` constructor and destructor. It also
changes the type from `void *` to a newly defined `NamedMDSymTabType`,
and removes the static casts.

Test Plan:
An ASAN-enabled build and run of `check-all` succeeds with this change
(aside from some tests that always fail for me in ASAN for some reason,
such as `check-clang` `SemaTemplate/stack-exhaustion.cpp`).

Reviewers: aprantl, dblaikie, chandlerc, pcc, echristo

Reviewed By: dblaikie

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

Added: 
    

Modified: 
    llvm/include/llvm/IR/Module.h
    llvm/lib/IR/Module.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h
index 68cd583c136c..46485847bdca 100644
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -79,6 +79,8 @@ class Module {
   using NamedMDListType = ilist<NamedMDNode>;
   /// The type of the comdat "symbol" table.
   using ComdatSymTabType = StringMap<Comdat>;
+  /// The type for mapping names to named metadata.
+  using NamedMDSymTabType = StringMap<NamedMDNode *>;
 
   /// The Global Variable iterator.
   using global_iterator = GlobalListType::iterator;
@@ -187,7 +189,7 @@ class Module {
                                   ///< recorded in bitcode.
   std::string TargetTriple;       ///< Platform target triple Module compiled on
                                   ///< Format: (arch)(sub)-(vendor)-(sys0-(abi)
-  void *NamedMDSymTab;            ///< NamedMDNode names.
+  NamedMDSymTabType NamedMDSymTab;  ///< NamedMDNode names.
   DataLayout DL;                  ///< DataLayout associated with the module
 
   friend class Constant;

diff  --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index 271ae126d722..8ddbffcde868 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -73,7 +73,6 @@ template class llvm::SymbolTableListTraits<GlobalIFunc>;
 Module::Module(StringRef MID, LLVMContext &C)
     : Context(C), Materializer(), ModuleID(MID), SourceFileName(MID), DL("") {
   ValSymTab = new ValueSymbolTable();
-  NamedMDSymTab = new StringMap<NamedMDNode *>();
   Context.addModule(this);
 }
 
@@ -85,8 +84,8 @@ Module::~Module() {
   AliasList.clear();
   IFuncList.clear();
   NamedMDList.clear();
+  NamedMDSymTab.clear();
   delete ValSymTab;
-  delete static_cast<StringMap<NamedMDNode *> *>(NamedMDSymTab);
 }
 
 std::unique_ptr<RandomNumberGenerator> Module::createRNG(const Pass* P) const {
@@ -250,15 +249,14 @@ GlobalIFunc *Module::getNamedIFunc(StringRef Name) const {
 NamedMDNode *Module::getNamedMetadata(const Twine &Name) const {
   SmallString<256> NameData;
   StringRef NameRef = Name.toStringRef(NameData);
-  return static_cast<StringMap<NamedMDNode*> *>(NamedMDSymTab)->lookup(NameRef);
+  return NamedMDSymTab.lookup(NameRef);
 }
 
 /// getOrInsertNamedMetadata - Return the first named MDNode in the module
 /// with the specified name. This method returns a new NamedMDNode if a
 /// NamedMDNode with the specified name is not found.
 NamedMDNode *Module::getOrInsertNamedMetadata(StringRef Name) {
-  NamedMDNode *&NMD =
-    (*static_cast<StringMap<NamedMDNode *> *>(NamedMDSymTab))[Name];
+  NamedMDNode *&NMD = NamedMDSymTab[Name];
   if (!NMD) {
     NMD = new NamedMDNode(Name);
     NMD->setParent(this);
@@ -270,7 +268,7 @@ NamedMDNode *Module::getOrInsertNamedMetadata(StringRef Name) {
 /// eraseNamedMetadata - Remove the given NamedMDNode from this module and
 /// delete it.
 void Module::eraseNamedMetadata(NamedMDNode *NMD) {
-  static_cast<StringMap<NamedMDNode *> *>(NamedMDSymTab)->erase(NMD->getName());
+  NamedMDSymTab.erase(NMD->getName());
   NamedMDList.erase(NMD->getIterator());
 }