[ThinLTO] Remove post-pass metadata linking support

Since we have moved to a model where functions are imported in bulk from
each source module after making summary-based importing decisions, there
is no longer a need to link metadata as a postpass, and all users have
been removed.

This essentially reverts r255909 and follow-on fixes.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@264763 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/IR/GVMaterializer.h

@@ -18,14 +18,12 @@
 #ifndef LLVM_IR_GVMATERIALIZER_H
 #define LLVM_IR_GVMATERIALIZER_H
 
-#include "llvm/ADT/DenseMap.h"
 #include <system_error>
 #include <vector>
 
 namespace llvm {
 class Function;
 class GlobalValue;
-class Metadata;
 class Module;
 class StructType;
 
@@ -47,14 +45,6 @@ public:
   virtual std::error_code materializeMetadata() = 0;
   virtual void setStripDebugInfo() = 0;
 
-  /// Client should define this interface if the mapping between metadata
-  /// values and value ids needs to be preserved, e.g. across materializer
-  /// instantiations. If OnlyTempMD is true, only those that have remained
-  /// temporary metadata are recorded in the map.
-  virtual void
-  saveMetadataList(DenseMap<const Metadata *, unsigned> &MetadataToIDs,
-                   bool OnlyTempMD) {}
-
   virtual std::vector<StructType *> getIdentifiedStructTypes() const = 0;
 };
 

include/llvm/IR/Metadata.h

@@ -283,20 +283,14 @@ private:
   LLVMContext &Context;
   uint64_t NextIndex;
   SmallDenseMap<void *, std::pair<OwnerTy, uint64_t>, 4> UseMap;
-  /// Flag that can be set to false if this metadata should not be
-  /// RAUW'ed, e.g. if it is used as the key of a map.
-  bool CanReplace;
 
 public:
   ReplaceableMetadataImpl(LLVMContext &Context)
-      : Context(Context), NextIndex(0), CanReplace(true) {}
+      : Context(Context), NextIndex(0) {}
   ~ReplaceableMetadataImpl() {
     assert(UseMap.empty() && "Cannot destroy in-use replaceable metadata");
   }
 
-  /// Set the CanReplace flag to the given value.
-  void setCanReplace(bool Replaceable) { CanReplace = Replaceable; }
-
   LLVMContext &getContext() const { return Context; }
 
   /// \brief Replace all uses of this with MD.
@@ -906,29 +900,13 @@ public:
     Context.getReplaceableUses()->replaceAllUsesWith(MD);
   }
 
-  /// Set the CanReplace flag to the given value.
-  void setCanReplace(bool Replaceable) {
-    Context.getReplaceableUses()->setCanReplace(Replaceable);
-  }
-
   /// \brief Resolve cycles.
   ///
   /// Once all forward declarations have been resolved, force cycles to be
-  /// resolved. This interface is used when there are no more temporaries,
+  /// resolved.
-  /// and thus unresolved nodes are part of cycles and no longer need RAUW
-  /// support.
   ///
   /// \pre No operands (or operands' operands, etc.) have \a isTemporary().
-  void resolveCycles() { resolveRecursivelyImpl(/* AllowTemps */ false); }
+  void resolveCycles();
-
-  /// \brief Resolve cycles while ignoring temporaries.
-  ///
-  /// This drops RAUW support for any temporaries, which can no longer
-  /// be uniqued.
-  ///
-  void resolveNonTemporaries() {
-    resolveRecursivelyImpl(/* AllowTemps */ true);
-  }
 
   /// \brief Replace a temporary node with a permanent one.
   ///
@@ -986,11 +964,6 @@ private:
   void decrementUnresolvedOperandCount();
   unsigned countUnresolvedOperands();
 
-  /// Resolve cycles recursively. If \p AllowTemps is true, then any temporary
-  /// metadata is ignored, otherwise it asserts when encountering temporary
-  /// metadata.
-  void resolveRecursivelyImpl(bool AllowTemps);
-
   /// \brief Mutate this to be "uniqued".
   ///
   /// Mutate this so that \a isUniqued().

include/llvm/Linker/IRMover.h

@@ -16,7 +16,6 @@
 
 namespace llvm {
 class GlobalValue;
-class MDNode;
 class Module;
 class StructType;
 class Type;
@@ -69,9 +68,7 @@ public:
   ///
   /// Returns true on error.
   bool move(std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
-            std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor,
+            std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor);
-            DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr,
-            bool IsMetadataLinkingPostpass = false);
   Module &getModule() { return Composite; }
 
 private:

include/llvm/Linker/Linker.h

@@ -41,23 +41,13 @@ public:
   /// For ThinLTO function importing/exporting the \p ModuleSummaryIndex
   /// is passed. If \p GlobalsToImport is provided, only the globals that
   /// are part of the set will be imported from the source module.
-  /// The \p ValIDToTempMDMap is populated by the linker when function
-  /// importing is performed.
   ///
   /// Returns true on error.
   bool linkInModule(std::unique_ptr<Module> Src, unsigned Flags = Flags::None,
-                    DenseSet<const GlobalValue *> *GlobalsToImport = nullptr,
+                    DenseSet<const GlobalValue *> *GlobalsToImport = nullptr);
-                    DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr);
 
   static bool linkModules(Module &Dest, std::unique_ptr<Module> Src,
                           unsigned Flags = Flags::None);
-
-  /// \brief Link metadata from \p Src into the composite.
-  ///
-  /// The \p ValIDToTempMDMap sound have been populated earlier during function
-  /// importing from \p Src.
-  bool linkInMetadata(std::unique_ptr<Module> Src,
-                      DenseMap<unsigned, MDNode *> *ValIDToTempMDMap);
 };
 
 } // End llvm namespace

include/llvm/Transforms/Utils/ValueMapper.h

@@ -56,12 +56,6 @@ namespace llvm {
     /// about recursion.
     virtual void materializeInitFor(GlobalValue *New, GlobalValue *Old);
 
-    /// If the client needs to handle temporary metadata it must implement
-    /// these methods.
-    virtual Metadata *mapTemporaryMetadata(Metadata *MD) { return nullptr; }
-    virtual void replaceTemporaryMetadata(const Metadata *OrigMD,
-                                          Metadata *NewMD) {}
-
     /// The client should implement this method if some metadata need
     /// not be mapped, for example DISubprogram metadata for functions not
     /// linked into the destination module.
@@ -89,11 +83,6 @@ namespace llvm {
     /// Any global values not in value map are mapped to null instead of
     /// mapping to self. Illegal if RF_IgnoreMissingEntries is also set.
     RF_NullMapMissingGlobalValues = 8,
-
-    /// Set when there is still temporary metadata that must be handled,
-    /// such as when we are doing function importing and will materialize
-    /// and link metadata as a postpass.
-    RF_HaveUnmaterializedMetadata = 16,
   };
 
   static inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -268,14 +268,6 @@ public:
 
   void setStripDebugInfo() override;
 
-  /// Save the mapping between the metadata values and the corresponding
-  /// value id that were recorded in the MetadataList during parsing. If
-  /// OnlyTempMD is true, then only record those entries that are still
-  /// temporary metadata. This interface is used when metadata linking is
-  /// performed as a postpass, such as during function importing.
-  void saveMetadataList(DenseMap<const Metadata *, unsigned> &MetadataToIDs,
-                        bool OnlyTempMD) override;
-
 private:
   /// Parse the "IDENTIFICATION_BLOCK_ID" block, populate the
   // ProducerIdentification data member, and do some basic enforcement on the
@@ -3126,36 +3118,6 @@ std::error_code BitcodeReader::materializeMetadata() {
 
 void BitcodeReader::setStripDebugInfo() { StripDebugInfo = true; }
 
-void BitcodeReader::saveMetadataList(
-    DenseMap<const Metadata *, unsigned> &MetadataToIDs, bool OnlyTempMD) {
-  for (unsigned ID = 0; ID < MetadataList.size(); ++ID) {
-    Metadata *MD = MetadataList[ID];
-    auto *N = dyn_cast_or_null<MDNode>(MD);
-    assert((!N || (N->isResolved() || N->isTemporary())) &&
-           "Found non-resolved non-temp MDNode while saving metadata");
-    // Save all values if !OnlyTempMD, otherwise just the temporary metadata.
-    // Note that in the !OnlyTempMD case we need to save all Metadata, not
-    // just MDNode, as we may have references to other types of module-level
-    // metadata (e.g. ValueAsMetadata) from instructions.
-    if (!OnlyTempMD || (N && N->isTemporary())) {
-      // Will call this after materializing each function, in order to
-      // handle remapping of the function's instructions/metadata.
-      auto IterBool = MetadataToIDs.insert(std::make_pair(MD, ID));
-      // See if we already have an entry in that case.
-      if (OnlyTempMD && !IterBool.second) {
-        assert(IterBool.first->second == ID &&
-               "Inconsistent metadata value id");
-        continue;
-      }
-      if (N && N->isTemporary())
-        // Ensure that we assert if someone tries to RAUW this temporary
-        // metadata while it is the key of a map. The flag will be set back
-        // to true when the saved metadata list is destroyed.
-        N->setCanReplace(false);
-    }
-  }
-}
-
 /// When we see the block for a function body, remember where it is and then
 /// skip it.  This lets us lazily deserialize the functions.
 std::error_code BitcodeReader::rememberAndSkipFunctionBody() {

lib/IR/Metadata.cpp

@@ -188,9 +188,6 @@ void ReplaceableMetadataImpl::moveRef(void *Ref, void *New,
 }
 
 void ReplaceableMetadataImpl::replaceAllUsesWith(Metadata *MD) {
-  assert(CanReplace &&
-         "Attempted to replace Metadata marked for no replacement");
-
   if (UseMap.empty())
     return;
 
@@ -550,7 +547,7 @@ void MDNode::decrementUnresolvedOperandCount() {
     resolve();
 }
 
-void MDNode::resolveRecursivelyImpl(bool AllowTemps) {
+void MDNode::resolveCycles() {
   if (isResolved())
     return;
 
@@ -563,8 +560,6 @@ void MDNode::resolveRecursivelyImpl(bool AllowTemps) {
     if (!N)
       continue;
 
-    if (N->isTemporary() && AllowTemps)
-      continue;
     assert(!N->isTemporary() &&
            "Expected all forward declarations to be resolved");
     if (!N->isResolved())

lib/Linker/IRMover.cpp

@@ -351,9 +351,6 @@ public:
   GlobalValueMaterializer(IRLinker &TheIRLinker) : TheIRLinker(TheIRLinker) {}
   Value *materializeDeclFor(Value *V) override;
   void materializeInitFor(GlobalValue *New, GlobalValue *Old) override;
-  Metadata *mapTemporaryMetadata(Metadata *MD) override;
-  void replaceTemporaryMetadata(const Metadata *OrigMD,
-                                Metadata *NewMD) override;
   bool isMetadataNeeded(Metadata *MD) override;
 };
 
@@ -364,9 +361,6 @@ public:
   LocalValueMaterializer(IRLinker &TheIRLinker) : TheIRLinker(TheIRLinker) {}
   Value *materializeDeclFor(Value *V) override;
   void materializeInitFor(GlobalValue *New, GlobalValue *Old) override;
-  Metadata *mapTemporaryMetadata(Metadata *MD) override;
-  void replaceTemporaryMetadata(const Metadata *OrigMD,
-                                Metadata *NewMD) override;
   bool isMetadataNeeded(Metadata *MD) override;
 };
 
@@ -405,24 +399,9 @@ class IRLinker {
 
   bool HasError = false;
 
-  /// Flag indicating that we are just linking metadata (after function
-  /// importing).
-  bool IsMetadataLinkingPostpass;
-
   /// Flags to pass to value mapper invocations.
   RemapFlags ValueMapperFlags = RF_MoveDistinctMDs;
 
-  /// Association between metadata values created during bitcode parsing and
-  /// the value id. Used to correlate temporary metadata created during
-  /// function importing with the final metadata parsed during the subsequent
-  /// metadata linking postpass.
-  DenseMap<const Metadata *, unsigned> MetadataToIDs;
-
-  /// Association between metadata value id and temporary metadata that
-  /// remains unmapped after function importing. Saved during function
-  /// importing and consumed during the metadata linking postpass.
-  DenseMap<unsigned, MDNode *> *ValIDToTempMDMap;
-
   /// Set of subprogram metadata that does not need to be linked into the
   /// destination module, because the functions were not imported directly
   /// or via an inlined body in an imported function.
@@ -443,14 +422,6 @@ class IRLinker {
     SrcM->getContext().diagnose(LinkDiagnosticInfo(DS_Warning, Message));
   }
 
-  /// Check whether we should be linking metadata from the source module.
-  bool shouldLinkMetadata() {
-    // ValIDToTempMDMap will be non-null when we are importing or otherwise want
-    // to link metadata lazily, and then when linking the metadata.
-    // We only want to return true for the former case.
-    return ValIDToTempMDMap == nullptr || IsMetadataLinkingPostpass;
-  }
-
   /// Given a global in the source module, return the global in the
   /// destination module that is being linked to, if any.
   GlobalValue *getLinkedToGlobal(const GlobalValue *SrcGV) {
@@ -507,11 +478,6 @@ class IRLinker {
   /// in an imported function.
   void findNeededSubprograms();
 
-  /// Recursive helper for findNeededSubprograms to locate any DISubprogram
-  /// reached from the given Node, marking any found as needed.
-  void findReachedSubprograms(const MDNode *Node,
-                              SmallPtrSet<const MDNode *, 16> &Visited);
-
   /// The value mapper leaves nulls in the list of subprograms for any
   /// in the UnneededSubprograms map. Strip those out of the mapped
   /// compile unit.
@@ -520,53 +486,17 @@ class IRLinker {
 public:
   IRLinker(Module &DstM, IRMover::IdentifiedStructTypeSet &Set,
            std::unique_ptr<Module> SrcM, ArrayRef<GlobalValue *> ValuesToLink,
-           std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor,
+           std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor)
-           DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr,
-           bool IsMetadataLinkingPostpass = false)
       : DstM(DstM), SrcM(std::move(SrcM)), AddLazyFor(AddLazyFor), TypeMap(Set),
-        GValMaterializer(*this), LValMaterializer(*this),
+        GValMaterializer(*this), LValMaterializer(*this) {
-        IsMetadataLinkingPostpass(IsMetadataLinkingPostpass),
-        ValIDToTempMDMap(ValIDToTempMDMap) {
     for (GlobalValue *GV : ValuesToLink)
       maybeAdd(GV);
-
-    // If appropriate, tell the value mapper that it can expect to see
-    // temporary metadata.
-    if (!shouldLinkMetadata())
-      ValueMapperFlags = ValueMapperFlags | RF_HaveUnmaterializedMetadata;
-  }
-
-  ~IRLinker() {
-    // In the case where we are not linking metadata, we unset the CanReplace
-    // flag on all temporary metadata in the MetadataToIDs map to ensure
-    // none was replaced while being a map key. Now that we are destructing
-    // the map, set the flag back to true, so that it is replaceable during
-    // metadata linking.
-    if (!shouldLinkMetadata()) {
-      for (auto MDI : MetadataToIDs) {
-        Metadata *MD = const_cast<Metadata *>(MDI.first);
-        MDNode *Node = dyn_cast<MDNode>(MD);
-        assert((Node && Node->isTemporary()) &&
-               "Found non-temp metadata in map when not linking metadata");
-        Node->setCanReplace(true);
-      }
-    }
   }
 
   bool run();
   Value *materializeDeclFor(Value *V, bool ForAlias);
   void materializeInitFor(GlobalValue *New, GlobalValue *Old, bool ForAlias);
 
-  /// Save the mapping between the given temporary metadata and its metadata
-  /// value id. Used to support metadata linking as a postpass for function
-  /// importing.
-  Metadata *mapTemporaryMetadata(Metadata *MD);
-
-  /// Replace any temporary metadata saved for the source metadata's id with
-  /// the new non-temporary metadata. Used when metadata linking as a postpass
-  /// for function importing.
-  void replaceTemporaryMetadata(const Metadata *OrigMD, Metadata *NewMD);
-
   /// Indicates whether we need to map the given metadata into the destination
   /// module. Used to prevent linking of metadata only needed by functions not
   /// linked into the dest module.
@@ -604,15 +534,6 @@ void GlobalValueMaterializer::materializeInitFor(GlobalValue *New,
   TheIRLinker.materializeInitFor(New, Old, false);
 }
 
-Metadata *GlobalValueMaterializer::mapTemporaryMetadata(Metadata *MD) {
-  return TheIRLinker.mapTemporaryMetadata(MD);
-}
-
-void GlobalValueMaterializer::replaceTemporaryMetadata(const Metadata *OrigMD,
-                                                       Metadata *NewMD) {
-  TheIRLinker.replaceTemporaryMetadata(OrigMD, NewMD);
-}
-
 bool GlobalValueMaterializer::isMetadataNeeded(Metadata *MD) {
   return TheIRLinker.isMetadataNeeded(MD);
 }
@@ -626,15 +547,6 @@ void LocalValueMaterializer::materializeInitFor(GlobalValue *New,
   TheIRLinker.materializeInitFor(New, Old, true);
 }
 
-Metadata *LocalValueMaterializer::mapTemporaryMetadata(Metadata *MD) {
-  return TheIRLinker.mapTemporaryMetadata(MD);
-}
-
-void LocalValueMaterializer::replaceTemporaryMetadata(const Metadata *OrigMD,
-                                                      Metadata *NewMD) {
-  TheIRLinker.replaceTemporaryMetadata(OrigMD, NewMD);
-}
-
 bool LocalValueMaterializer::isMetadataNeeded(Metadata *MD) {
   return TheIRLinker.isMetadataNeeded(MD);
 }
@@ -666,50 +578,6 @@ void IRLinker::materializeInitFor(GlobalValue *New, GlobalValue *Old,
     linkGlobalValueBody(*New, *Old);
 }
 
-Metadata *IRLinker::mapTemporaryMetadata(Metadata *MD) {
-  if (!ValIDToTempMDMap)
-    return nullptr;
-  // If this temporary metadata has a value id recorded during function
-  // parsing, record that in the ValIDToTempMDMap if one was provided.
-  auto I = MetadataToIDs.find(MD);
-  if (I == MetadataToIDs.end())
-    return nullptr;
-  unsigned Idx = I->second;
-  MDNode *Node = cast<MDNode>(MD);
-  assert(Node->isTemporary());
-  // If we created a temp MD when importing a different function from
-  // this module, reuse the same temporary metadata.
-  auto IterBool = ValIDToTempMDMap->insert(std::make_pair(Idx, Node));
-  return IterBool.first->second;
-}
-
-void IRLinker::replaceTemporaryMetadata(const Metadata *OrigMD,
-                                        Metadata *NewMD) {
-  if (!ValIDToTempMDMap)
-    return;
-#ifndef NDEBUG
-  auto *N = dyn_cast_or_null<MDNode>(NewMD);
-  assert(!N || !N->isTemporary());
-#endif
-  // If a mapping between metadata value ids and temporary metadata
-  // created during function importing was provided, and the source
-  // metadata has a value id recorded during metadata parsing, replace
-  // the temporary metadata with the final mapped metadata now.
-  auto I = MetadataToIDs.find(OrigMD);
-  if (I == MetadataToIDs.end())
-    return;
-  unsigned Idx = I->second;
-  auto VI = ValIDToTempMDMap->find(Idx);
-  // Nothing to do if we didn't need to create a temporary metadata during
-  // function importing.
-  if (VI == ValIDToTempMDMap->end())
-    return;
-  MDNode *TempMD = VI->second;
-  TempMD->replaceAllUsesWith(NewMD);
-  MDNode::deleteTemporary(TempMD);
-  ValIDToTempMDMap->erase(VI);
-}
-
 bool IRLinker::isMetadataNeeded(Metadata *MD) {
   // Currently only DISubprogram metadata is marked as being unneeded.
   if (UnneededSubprograms.empty())
@@ -1017,14 +885,6 @@ Constant *IRLinker::linkAppendingVarProto(GlobalVariable *DstGV,
 }
 
 bool IRLinker::shouldLink(GlobalValue *DGV, GlobalValue &SGV) {
-  // Already imported all the values. Just map to the Dest value
-  // in case it is referenced in the metadata.
-  if (IsMetadataLinkingPostpass) {
-    assert(!ValuesToLink.count(&SGV) &&
-           "Source value unexpectedly requested for link during metadata link");
-    return false;
-  }
-
   if (ValuesToLink.count(&SGV))
     return true;
 
@@ -1133,14 +993,6 @@ bool IRLinker::linkFunctionBody(Function &Dst, Function &Src) {
   if (std::error_code EC = Src.materialize())
     return emitError(EC.message());
 
-  if (!shouldLinkMetadata())
-    // This is only supported for lazy links. Do after materialization of
-    // a function and before remapping metadata on instructions below
-    // in RemapInstruction, as the saved mapping is used to handle
-    // the temporary metadata hanging off instructions.
-    SrcM->getMaterializer()->saveMetadataList(MetadataToIDs,
-                                              /* OnlyTempMD = */ true);
-
   // Link in the prefix data.
   if (Src.hasPrefixData())
     Dst.setPrefixData(MapValue(Src.getPrefixData(), ValueMap, ValueMapperFlags,
@@ -1212,21 +1064,6 @@ bool IRLinker::linkGlobalValueBody(GlobalValue &Dst, GlobalValue &Src) {
   return false;
 }
 
-void IRLinker::findReachedSubprograms(
-    const MDNode *Node, SmallPtrSet<const MDNode *, 16> &Visited) {
-  if (!Visited.insert(Node).second)
-    return;
-  DISubprogram *SP = getDISubprogram(Node);
-  if (SP)
-    UnneededSubprograms.erase(SP);
-  for (auto &Op : Node->operands()) {
-    const MDNode *OpN = dyn_cast_or_null<MDNode>(Op.get());
-    if (!OpN)
-      continue;
-    findReachedSubprograms(OpN, Visited);
-  }
-}
-
 void IRLinker::findNeededSubprograms() {
   // Track unneeded nodes to make it simpler to handle the case
   // where we are checking if an already-mapped SP is needed.
@@ -1251,32 +1088,13 @@ void IRLinker::findNeededSubprograms() {
         ImportedEntitySPs.insert(SP);
     }
     for (auto *Op : CU->getSubprograms()) {
-      // Unless we were doing function importing and deferred metadata linking,
+      // Any needed SPs should have been mapped as they would be reached
-      // any needed SPs should have been mapped as they would be reached
       // from the function linked in (either on the function itself for linked
       // function bodies, or from DILocation on inlined instructions).
-      assert(!(ValueMap.MD()[Op] && IsMetadataLinkingPostpass) &&
-             "DISubprogram shouldn't be mapped yet");
       if (!ValueMap.MD()[Op] && !ImportedEntitySPs.count(Op))
         UnneededSubprograms.insert(Op);
     }
   }
-  if (!IsMetadataLinkingPostpass)
-    return;
-  // In the case of metadata linking as a postpass (e.g. for function
-  // importing), see which MD from the source has an associated
-  // temporary metadata node, which means that any DISubprogram
-  // reached from that MD was needed by an imported function.
-  SmallPtrSet<const MDNode *, 16> Visited;
-  for (auto MDI : MetadataToIDs) {
-    const MDNode *Node = dyn_cast<MDNode>(MDI.first);
-    if (!Node)
-      continue;
-    if (!ValIDToTempMDMap->count(MDI.second))
-      continue;
-    // Find any SP needed recursively from this needed Node.
-    findReachedSubprograms(Node, Visited);
-  }
 }
 
 // Squash null subprograms from the given compile unit's subprogram list.
@@ -1505,8 +1323,7 @@ static std::string mergeTriples(const Triple &SrcTriple,
 
 bool IRLinker::run() {
   // Ensure metadata materialized before value mapping.
-  if (shouldLinkMetadata() && SrcM->getMaterializer())
+  if (SrcM->getMaterializer() && SrcM->getMaterializer()->materializeMetadata())
-    if (SrcM->getMaterializer()->materializeMetadata())
       return true;
 
   // Inherit the target data from the source module if the destination module
@@ -1572,36 +1389,11 @@ bool IRLinker::run() {
   // Remap all of the named MDNodes in Src into the DstM module. We do this
   // after linking GlobalValues so that MDNodes that reference GlobalValues
   // are properly remapped.
-  if (shouldLinkMetadata()) {
+  linkNamedMDNodes();
-    // Even if just linking metadata we should link decls above in case
-    // any are referenced by metadata. IRLinker::shouldLink ensures that
-    // we don't actually link anything from source.
-    if (IsMetadataLinkingPostpass)
-      SrcM->getMaterializer()->saveMetadataList(MetadataToIDs,
-                                                /* OnlyTempMD = */ false);
 
-    linkNamedMDNodes();
+  // Merge the module flags into the DstM module.
-
+  if (linkModuleFlagsMetadata())
-    if (IsMetadataLinkingPostpass) {
+    return true;
-      // Handle anything left in the ValIDToTempMDMap, such as metadata nodes
-      // not reached by the dbg.cu NamedMD (i.e. only reached from
-      // instructions).
-      // Walk the MetadataToIDs once to find the set of new (imported) MD
-      // that still has corresponding temporary metadata, and invoke metadata
-      // mapping on each one.
-      for (auto MDI : MetadataToIDs) {
-        if (!ValIDToTempMDMap->count(MDI.second))
-          continue;
-        MapMetadata(MDI.first, ValueMap, ValueMapperFlags, &TypeMap,
-                    &GValMaterializer);
-      }
-      assert(ValIDToTempMDMap->empty());
-    }
-
-    // Merge the module flags into the DstM module.
-    if (linkModuleFlagsMetadata())
-      return true;
-  }
 
   return false;
 }
@@ -1709,12 +1501,9 @@ IRMover::IRMover(Module &M) : Composite(M) {
 
 bool IRMover::move(
     std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
-    std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor,
+    std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor) {
-    DenseMap<unsigned, MDNode *> *ValIDToTempMDMap,
-    bool IsMetadataLinkingPostpass) {
   IRLinker TheIRLinker(Composite, IdentifiedStructTypes, std::move(Src),
-                       ValuesToLink, AddLazyFor, ValIDToTempMDMap,
+                       ValuesToLink, AddLazyFor);
-                       IsMetadataLinkingPostpass);
   bool RetCode = TheIRLinker.run();
   Composite.dropTriviallyDeadConstantArrays();
   return RetCode;

lib/Linker/LinkModules.cpp

@@ -39,11 +39,6 @@ class ModuleLinker {
   /// imported as declarations instead of definitions.
   DenseSet<const GlobalValue *> *GlobalsToImport;
 
-  /// Association between metadata value id and temporary metadata that
-  /// remains unmapped after function importing. Saved during function
-  /// importing and consumed during the metadata linking postpass.
-  DenseMap<unsigned, MDNode *> *ValIDToTempMDMap;
-
   /// Used as the callback for lazy linking.
   /// The mover has just hit GV and we have to decide if it, and other members
   /// of the same comdat, should be linked. Every member to be linked is passed
@@ -119,10 +114,9 @@ class ModuleLinker {
 
 public:
   ModuleLinker(IRMover &Mover, std::unique_ptr<Module> SrcM, unsigned Flags,
-               DenseSet<const GlobalValue *> *GlobalsToImport = nullptr,
+               DenseSet<const GlobalValue *> *GlobalsToImport = nullptr)
-               DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr)
       : Mover(Mover), SrcM(std::move(SrcM)), Flags(Flags),
-        GlobalsToImport(GlobalsToImport), ValIDToTempMDMap(ValIDToTempMDMap) {}
+        GlobalsToImport(GlobalsToImport) {}
 
   bool run();
 };
@@ -590,8 +584,7 @@ bool ModuleLinker::run() {
   if (Mover.move(std::move(SrcM), ValuesToLink.getArrayRef(),
                  [this](GlobalValue &GV, IRMover::ValueAdder Add) {
                    addLazyFor(GV, Add);
-                 },
+                 }))
-                 ValIDToTempMDMap, false))
     return true;
   for (auto &P : Internalize) {
     GlobalValue *GV = DstM.getNamedValue(P.first());
@@ -604,24 +597,11 @@ bool ModuleLinker::run() {
 Linker::Linker(Module &M) : Mover(M) {}
 
 bool Linker::linkInModule(std::unique_ptr<Module> Src, unsigned Flags,
-                          DenseSet<const GlobalValue *> *GlobalsToImport,
+                          DenseSet<const GlobalValue *> *GlobalsToImport) {
-                          DenseMap<unsigned, MDNode *> *ValIDToTempMDMap) {
+  ModuleLinker ModLinker(Mover, std::move(Src), Flags, GlobalsToImport);
-  ModuleLinker ModLinker(Mover, std::move(Src), Flags, GlobalsToImport,
-                         ValIDToTempMDMap);
   return ModLinker.run();
 }
 
-bool Linker::linkInMetadata(std::unique_ptr<Module> Src,
-                            DenseMap<unsigned, MDNode *> *ValIDToTempMDMap) {
-  SetVector<GlobalValue *> ValuesToLink;
-  if (Mover.move(
-          std::move(Src), ValuesToLink.getArrayRef(),
-          [this](GlobalValue &GV, IRMover::ValueAdder Add) { assert(false); },
-          ValIDToTempMDMap, true))
-    return true;
-  return false;
-}
-
 //===----------------------------------------------------------------------===//
 // LinkModules entrypoint.
 //===----------------------------------------------------------------------===//

lib/Transforms/Utils/ValueMapper.cpp

@@ -167,21 +167,13 @@ Value *llvm::MapValue(const Value *V, ValueToValueMapTy &VM, RemapFlags Flags,
 }
 
 static Metadata *mapToMetadata(ValueToValueMapTy &VM, const Metadata *Key,
-                               Metadata *Val, ValueMaterializer *Materializer,
+                               Metadata *Val) {
-                               RemapFlags Flags) {
   VM.MD()[Key].reset(Val);
-  if (Materializer && !(Flags & RF_HaveUnmaterializedMetadata)) {
-    auto *N = dyn_cast_or_null<MDNode>(Val);
-    // Need to invoke this once we have non-temporary MD.
-    if (!N || !N->isTemporary())
-      Materializer->replaceTemporaryMetadata(Key, Val);
-  }
   return Val;
 }
 
-static Metadata *mapToSelf(ValueToValueMapTy &VM, const Metadata *MD,
+static Metadata *mapToSelf(ValueToValueMapTy &VM, const Metadata *MD) {
-                           ValueMaterializer *Materializer, RemapFlags Flags) {
+  return mapToMetadata(VM, MD, const_cast<Metadata *>(MD));
-  return mapToMetadata(VM, MD, const_cast<Metadata *>(MD), Materializer, Flags);
 }
 
 static Metadata *MapMetadataImpl(const Metadata *MD,
@@ -218,22 +210,10 @@ static Metadata *mapMetadataOp(Metadata *Op,
 }
 
 /// Resolve uniquing cycles involving the given metadata.
-static void resolveCycles(Metadata *MD, bool AllowTemps) {
+static void resolveCycles(Metadata *MD) {
-  if (auto *N = dyn_cast_or_null<MDNode>(MD)) {
+  if (auto *N = dyn_cast_or_null<MDNode>(MD))
-    if (AllowTemps && N->isTemporary())
+    if (!N->isResolved())
-      return;
+      N->resolveCycles();
-    if (!N->isResolved()) {
-      if (AllowTemps)
-        // Note that this will drop RAUW support on any temporaries, which
-        // blocks uniquing. If this ends up being an issue, in the future
-        // we can experiment with delaying resolving these nodes until
-        // after metadata is fully materialized (i.e. when linking metadata
-        // as a postpass after function importing).
-        N->resolveNonTemporaries();
-      else
-        N->resolveCycles();
-    }
-  }
 }
 
 /// Remap the operands of an MDNode.
@@ -262,7 +242,7 @@ static bool remapOperands(MDNode &Node,
       // Resolve uniquing cycles underneath distinct nodes on the fly so they
       // don't infect later operands.
       if (IsDistinct)
-        resolveCycles(New, Flags & RF_HaveUnmaterializedMetadata);
+        resolveCycles(New);
     }
   }
 
@@ -290,7 +270,7 @@ static Metadata *mapDistinctNode(const MDNode *Node,
 
   // Remap operands later.
   DistinctWorklist.push_back(NewMD);
-  return mapToMetadata(VM, Node, NewMD, Materializer, Flags);
+  return mapToMetadata(VM, Node, NewMD);
 }
 
 /// \brief Map a uniqued MDNode.
@@ -301,29 +281,22 @@ static Metadata *mapUniquedNode(const MDNode *Node,
                                 ValueToValueMapTy &VM, RemapFlags Flags,
                                 ValueMapTypeRemapper *TypeMapper,
                                 ValueMaterializer *Materializer) {
-  assert(((Flags & RF_HaveUnmaterializedMetadata) || Node->isUniqued()) &&
+  assert(Node->isUniqued() && "Expected uniqued node");
-         "Expected uniqued node");
 
   // Create a temporary node and map it upfront in case we have a uniquing
   // cycle.  If necessary, this mapping will get updated by RAUW logic before
   // returning.
   auto ClonedMD = Node->clone();
-  mapToMetadata(VM, Node, ClonedMD.get(), Materializer, Flags);
+  mapToMetadata(VM, Node, ClonedMD.get());
   if (!remapOperands(*ClonedMD, DistinctWorklist, VM, Flags, TypeMapper,
                      Materializer)) {
     // No operands changed, so use the original.
     ClonedMD->replaceAllUsesWith(const_cast<MDNode *>(Node));
-    // Even though replaceAllUsesWith would have replaced the value map
+    return const_cast<MDNode *>(Node);
-    // entry, we need to explictly map with the final non-temporary node
-    // to replace any temporary metadata via the callback.
-    return mapToSelf(VM, Node, Materializer, Flags);
   }
 
-  // Uniquify the cloned node. Explicitly map it with the final non-temporary
+  // Uniquify the cloned node.
-  // node so that replacement of temporary metadata via the callback occurs.
+  return MDNode::replaceWithUniqued(std::move(ClonedMD));
-  return mapToMetadata(VM, Node,
-                       MDNode::replaceWithUniqued(std::move(ClonedMD)),
-                       Materializer, Flags);
 }
 
 static Metadata *MapMetadataImpl(const Metadata *MD,
@@ -336,18 +309,18 @@ static Metadata *MapMetadataImpl(const Metadata *MD,
     return NewMD;
 
   if (isa<MDString>(MD))
-    return mapToSelf(VM, MD, Materializer, Flags);
+    return mapToSelf(VM, MD);
 
   if (isa<ConstantAsMetadata>(MD))
     if ((Flags & RF_NoModuleLevelChanges))
-      return mapToSelf(VM, MD, Materializer, Flags);
+      return mapToSelf(VM, MD);
 
   if (const auto *VMD = dyn_cast<ValueAsMetadata>(MD)) {
     Value *MappedV =
         MapValue(VMD->getValue(), VM, Flags, TypeMapper, Materializer);
     if (VMD->getValue() == MappedV ||
         (!MappedV && (Flags & RF_IgnoreMissingEntries)))
-      return mapToSelf(VM, MD, Materializer, Flags);
+      return mapToSelf(VM, MD);
 
     // FIXME: This assert crashes during bootstrap, but I think it should be
     // correct.  For now, just match behaviour from before the metadata/value
@@ -356,8 +329,7 @@ static Metadata *MapMetadataImpl(const Metadata *MD,
     //    assert((MappedV || (Flags & RF_NullMapMissingGlobalValues)) &&
     //           "Referenced metadata not in value map!");
     if (MappedV)
-      return mapToMetadata(VM, MD, ValueAsMetadata::get(MappedV), Materializer,
+      return mapToMetadata(VM, MD, ValueAsMetadata::get(MappedV));
-                           Flags);
     return nullptr;
   }
 
@@ -368,25 +340,10 @@ static Metadata *MapMetadataImpl(const Metadata *MD,
   // If this is a module-level metadata and we know that nothing at the
   // module level is changing, then use an identity mapping.
   if (Flags & RF_NoModuleLevelChanges)
-    return mapToSelf(VM, MD, Materializer, Flags);
+    return mapToSelf(VM, MD);
 
   // Require resolved nodes whenever metadata might be remapped.
-  assert(((Flags & RF_HaveUnmaterializedMetadata) || Node->isResolved()) &&
+  assert(Node->isResolved() && "Unexpected unresolved node");
-         "Unexpected unresolved node");
-
-  if (Materializer && Node->isTemporary()) {
-    assert(Flags & RF_HaveUnmaterializedMetadata);
-    Metadata *TempMD =
-        Materializer->mapTemporaryMetadata(const_cast<Metadata *>(MD));
-    // If the above callback returned an existing temporary node, use it
-    // instead of the current temporary node. This happens when earlier
-    // function importing passes already created and saved a temporary
-    // metadata node for the same value id.
-    if (TempMD) {
-      mapToMetadata(VM, MD, TempMD, Materializer, Flags);
-      return TempMD;
-    }
-  }
 
   if (Node->isDistinct())
     return mapDistinctNode(Node, DistinctWorklist, VM, Flags, TypeMapper,
@@ -410,7 +367,7 @@ Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
     return NewMD;
 
   // Resolve cycles involving the entry metadata.
-  resolveCycles(NewMD, Flags & RF_HaveUnmaterializedMetadata);
+  resolveCycles(NewMD);
 
   // Remap the operands of distinct MDNodes.
   while (!DistinctWorklist.empty())

unittests/IR/MetadataTest.cpp

@@ -494,20 +494,6 @@ TEST_F(MDNodeTest, isTemporary) {
   EXPECT_TRUE(T->isTemporary());
 }
 
-#if defined(GTEST_HAS_DEATH_TEST) && !defined(NDEBUG)
-
-TEST_F(MDNodeTest, deathOnNoReplaceTemporaryRAUW) {
-  auto Temp = MDNode::getTemporary(Context, None);
-  Temp->setCanReplace(false);
-  EXPECT_DEATH(Temp->replaceAllUsesWith(nullptr),
-               "Attempted to replace Metadata marked for no replacement");
-  Temp->setCanReplace(true);
-  // Remove the references to Temp; required for teardown.
-  Temp->replaceAllUsesWith(nullptr);
-}
-
-#endif
-
 TEST_F(MDNodeTest, getDistinctWithUnresolvedOperands) {
   // temporary !{}
   auto Temp = MDTuple::getTemporary(Context, None);