Extend the GlobalObject metadata interface

- Make eraseMetadata return whether it changed something
- Wire getMetadata for a single MDNode efficiently into the attachment
map
- Add hasMetadata, which is less weird than checking getMetadata ==
nullptr on a multimap.

Use it to simplify code.

llvm-svn: 333649

llvm/include/llvm/IR/GlobalObject.h

@@ -105,6 +105,14 @@ public:
   /// Check if this has any metadata.
   bool hasMetadata() const { return hasMetadataHashEntry(); }
 
+  /// Check if this has any metadata of the given kind.
+  bool hasMetadata(unsigned KindID) const {
+    return getMetadata(KindID) != nullptr;
+  }
+  bool hasMetadata(StringRef Kind) const {
+    return getMetadata(Kind) != nullptr;
+  }
+
   /// Get the current metadata attachments for the given kind, if any.
   ///
   /// These functions require that the function have at most a single attachment
@@ -143,7 +151,9 @@ public:
   getAllMetadata(SmallVectorImpl<std::pair<unsigned, MDNode *>> &MDs) const;
 
   /// Erase all metadata attachments with the given kind.
-  void eraseMetadata(unsigned KindID);
+  ///
+  /// \returns true if any metadata was removed.
+  bool eraseMetadata(unsigned KindID);
 
   /// Copy metadata from Src, adjusting offsets by Offset.
   void copyMetadata(const GlobalObject *Src, unsigned Offset);

llvm/lib/IR/DebugInfo.cpp

@@ -313,7 +313,7 @@ static MDNode *stripDebugLocFromLoopID(MDNode *N) {
 
 bool llvm::stripDebugInfo(Function &F) {
   bool Changed = false;
-  if (F.getMetadata(LLVMContext::MD_dbg)) {
+  if (F.hasMetadata(LLVMContext::MD_dbg)) {
     Changed = true;
     F.setSubprogram(nullptr);
   }
@@ -369,12 +369,7 @@ bool llvm::StripDebugInfo(Module &M) {
     Changed |= stripDebugInfo(F);
 
   for (auto &GV : M.globals()) {
-    SmallVector<MDNode *, 1> MDs;
-    GV.getMetadata(LLVMContext::MD_dbg, MDs);
-    if (!MDs.empty()) {
-      GV.eraseMetadata(LLVMContext::MD_dbg);
-      Changed = true;
-    }
+    Changed |= GV.eraseMetadata(LLVMContext::MD_dbg);
   }
 
   if (GVMaterializer *Materializer = M.getMaterializer())

llvm/lib/IR/LLVMContextImpl.h

@@ -1164,7 +1164,7 @@ public:
   /// Remove an attachment.
   ///
   /// Remove the attachment at \c ID, if any.
-  void erase(unsigned ID);
+  bool erase(unsigned ID);
 
   /// Copy out all the attachments.
   ///
@@ -1197,10 +1197,14 @@ public:
   /// Appends all attachments with the given ID to \c Result in insertion order.
   /// If the global has no attachments with the given ID, or if ID is invalid,
   /// leaves Result unchanged.
-  void get(unsigned ID, SmallVectorImpl<MDNode *> &Result);
+  void get(unsigned ID, SmallVectorImpl<MDNode *> &Result) const;
+
+  /// Returns the first attachment with the given ID or nullptr if no such
+  /// attachment exists.
+  MDNode *lookup(unsigned ID) const;
 
   void insert(unsigned ID, MDNode &MD);
-  void erase(unsigned ID);
+  bool erase(unsigned ID);
 
   /// Appends all attachments for the global to \c Result, sorting by attachment
   /// ID. Attachments with the same ID appear in insertion order. This function

llvm/lib/IR/Metadata.cpp

@@ -1110,14 +1110,14 @@ void MDAttachmentMap::set(unsigned ID, MDNode &MD) {
                            std::make_tuple(&MD));
 }
 
-void MDAttachmentMap::erase(unsigned ID) {
+bool MDAttachmentMap::erase(unsigned ID) {
   if (empty())
-    return;
+    return false;
 
   // Common case is one/last value.
   if (Attachments.back().first == ID) {
     Attachments.pop_back();
-    return;
+    return true;
   }
 
   for (auto I = Attachments.begin(), E = std::prev(Attachments.end()); I != E;
@@ -1125,8 +1125,10 @@ void MDAttachmentMap::erase(unsigned ID) {
     if (I->first == ID) {
       *I = std::move(Attachments.back());
       Attachments.pop_back();
-      return;
+      return true;
     }
+
+  return false;
 }
 
 MDNode *MDAttachmentMap::lookup(unsigned ID) const {
@@ -1149,29 +1151,31 @@ void MDGlobalAttachmentMap::insert(unsigned ID, MDNode &MD) {
   Attachments.push_back({ID, TrackingMDNodeRef(&MD)});
 }
 
+MDNode *MDGlobalAttachmentMap::lookup(unsigned ID) const {
+  for (const auto &A : Attachments)
+    if (A.MDKind == ID)
+      return A.Node;
+  return nullptr;
+}
+
 void MDGlobalAttachmentMap::get(unsigned ID,
-                                SmallVectorImpl<MDNode *> &Result) {
-  for (auto A : Attachments)
+                                SmallVectorImpl<MDNode *> &Result) const {
+  for (const auto &A : Attachments)
     if (A.MDKind == ID)
       Result.push_back(A.Node);
 }
 
-void MDGlobalAttachmentMap::erase(unsigned ID) {
-  auto Follower = Attachments.begin();
-  for (auto Leader = Attachments.begin(), E = Attachments.end(); Leader != E;
-       ++Leader) {
-    if (Leader->MDKind != ID) {
-      if (Follower != Leader)
-        *Follower = std::move(*Leader);
-      ++Follower;
-    }
-  }
-  Attachments.resize(Follower - Attachments.begin());
+bool MDGlobalAttachmentMap::erase(unsigned ID) {
+  auto I = std::remove_if(Attachments.begin(), Attachments.end(),
+                          [ID](const Attachment &A) { return A.MDKind == ID; });
+  bool Changed = I != Attachments.end();
+  Attachments.erase(I, Attachments.end());
+  return Changed;
 }
 
 void MDGlobalAttachmentMap::getAll(
     SmallVectorImpl<std::pair<unsigned, MDNode *>> &Result) const {
-  for (auto &A : Attachments)
+  for (const auto &A : Attachments)
     Result.emplace_back(A.MDKind, A.Node);
 
   // Sort the resulting array so it is stable with respect to metadata IDs. We
@@ -1398,15 +1402,16 @@ void GlobalObject::addMetadata(StringRef Kind, MDNode &MD) {
   addMetadata(getContext().getMDKindID(Kind), MD);
 }
 
-void GlobalObject::eraseMetadata(unsigned KindID) {
+bool GlobalObject::eraseMetadata(unsigned KindID) {
   // Nothing to unset.
   if (!hasMetadata())
-    return;
+    return false;
 
   auto &Store = getContext().pImpl->GlobalObjectMetadata[this];
-  Store.erase(KindID);
+  bool Changed = Store.erase(KindID);
   if (Store.empty())
     clearMetadata();
+  return Changed;
 }
 
 void GlobalObject::getAllMetadata(
@@ -1437,11 +1442,9 @@ void GlobalObject::setMetadata(StringRef Kind, MDNode *N) {
 }
 
 MDNode *GlobalObject::getMetadata(unsigned KindID) const {
-  SmallVector<MDNode *, 1> MDs;
-  getMetadata(KindID, MDs);
-  if (MDs.empty())
-    return nullptr;
-  return MDs[0];
+  if (hasMetadata())
+    return getContext().pImpl->GlobalObjectMetadata[this].lookup(KindID);
+  return nullptr;
 }
 
 MDNode *GlobalObject::getMetadata(StringRef Kind) const {

llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

@@ -130,8 +130,7 @@ void promoteTypeIds(Module &M, StringRef ModuleId) {
       }
       GO.addMetadata(
           LLVMContext::MD_type,
-          *MDNode::get(M.getContext(),
-                       ArrayRef<Metadata *>{MD->getOperand(0), I->second}));
+          *MDNode::get(M.getContext(), {MD->getOperand(0), I->second}));
     }
   }
 }
@@ -216,10 +215,8 @@ void splitAndWriteThinLTOBitcode(
   // Returns whether a global has attached type metadata. Such globals may
   // participate in CFI or whole-program devirtualization, so they need to
   // appear in the merged module instead of the thin LTO module.
-  auto HasTypeMetadata = [&](const GlobalObject *GO) {
-    SmallVector<MDNode *, 1> MDs;
-    GO->getMetadata(LLVMContext::MD_type, MDs);
-    return !MDs.empty();
+  auto HasTypeMetadata = [](const GlobalObject *GO) {
+    return GO->hasMetadata(LLVMContext::MD_type);
   };
 
   // Collect the set of virtual functions that are eligible for virtual constant
@@ -337,14 +334,15 @@ void splitAndWriteThinLTOBitcode(
       continue;
 
     auto *F = cast<Function>(A.getAliasee());
-    SmallVector<Metadata *, 4> Elts;
 
-    Elts.push_back(MDString::get(Ctx, A.getName()));
-    Elts.push_back(MDString::get(Ctx, F->getName()));
-    Elts.push_back(ConstantAsMetadata::get(
-        llvm::ConstantInt::get(Type::getInt8Ty(Ctx), A.getVisibility())));
-    Elts.push_back(ConstantAsMetadata::get(
-        llvm::ConstantInt::get(Type::getInt8Ty(Ctx), A.isWeakForLinker())));
+    Metadata *Elts[] = {
+        MDString::get(Ctx, A.getName()),
+        MDString::get(Ctx, F->getName()),
+        ConstantAsMetadata::get(
+            ConstantInt::get(Type::getInt8Ty(Ctx), A.getVisibility())),
+        ConstantAsMetadata::get(
+            ConstantInt::get(Type::getInt8Ty(Ctx), A.isWeakForLinker())),
+    };
 
     FunctionAliases.push_back(MDTuple::get(Ctx, Elts));
   }
@@ -361,11 +359,8 @@ void splitAndWriteThinLTOBitcode(
     if (!F || F->use_empty())
       return;
 
-    SmallVector<Metadata *, 2> Elts;
-    Elts.push_back(MDString::get(Ctx, Name));
-    Elts.push_back(MDString::get(Ctx, Alias));
-
-    Symvers.push_back(MDTuple::get(Ctx, Elts));
+    Symvers.push_back(MDTuple::get(
+        Ctx, {MDString::get(Ctx, Name), MDString::get(Ctx, Alias)}));
   });
 
   if (!Symvers.empty()) {
@@ -418,10 +413,8 @@ void splitAndWriteThinLTOBitcode(
 
 // Returns whether this module needs to be split because it uses type metadata.
 bool requiresSplit(Module &M) {
-  SmallVector<MDNode *, 1> MDs;
   for (auto &GO : M.global_objects()) {
-    GO.getMetadata(LLVMContext::MD_type, MDs);
-    if (!MDs.empty())
+    if (GO.hasMetadata(LLVMContext::MD_type))
       return true;
   }
 

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

@@ -1066,7 +1066,7 @@ Constant *DevirtModule::importConstant(VTableSlot Slot, ArrayRef<uint64_t> Args,
 
   // We only need to set metadata if the global is newly created, in which
   // case it would not have hidden visibility.
-  if (GV->getMetadata(LLVMContext::MD_absolute_symbol))
+  if (GV->hasMetadata(LLVMContext::MD_absolute_symbol))
     return C;
 
   auto SetAbsRange = [&](uint64_t Min, uint64_t Max) {