[llvm] r265229 - Linker: Remove IRMover::isMetadataUnneeded indirection; almost NFC

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 2 10:12:00 PDT 2016


Author: dexonsmith
Date: Sat Apr  2 12:12:00 2016
New Revision: 265229

URL: http://llvm.org/viewvc/llvm-project?rev=265229&view=rev
Log:
Linker: Remove IRMover::isMetadataUnneeded indirection; almost NFC

Instead of checking live during MapMetadata whether a subprogram is
needed, seed the ValueMap with `nullptr` up-front.

There is a small hypothetical functionality change.  Previously, calling
MapMetadataOp on a node whose "scope:" chain led to an unneeded
subprogram would return nullptr.  However, if that were ever called,
then the subprogram would be needed; a situation that the IRMover is
supposed to avoid a priori!

Besides cleaning up the code a little, this restores a nice property:
MapMetadataOp returns the same as MapMetadata.

Modified:
    llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h
    llvm/trunk/lib/Linker/IRMover.cpp
    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp

Modified: llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h?rev=265229&r1=265228&r2=265229&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h Sat Apr  2 12:12:00 2016
@@ -55,11 +55,6 @@ namespace llvm {
     /// It is called after the mapping is recorded, so it doesn't need to worry
     /// about recursion.
     virtual void materializeInitFor(GlobalValue *New, GlobalValue *Old);
-
-    /// 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.
-    virtual bool isMetadataNeeded(Metadata *MD) { return true; }
   };
 
   /// RemapFlags - These are flags that the value mapping APIs allow.

Modified: llvm/trunk/lib/Linker/IRMover.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=265229&r1=265228&r2=265229&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/IRMover.cpp (original)
+++ llvm/trunk/lib/Linker/IRMover.cpp Sat Apr  2 12:12:00 2016
@@ -351,7 +351,6 @@ public:
   GlobalValueMaterializer(IRLinker &TheIRLinker) : TheIRLinker(TheIRLinker) {}
   Value *materializeDeclFor(Value *V) override;
   void materializeInitFor(GlobalValue *New, GlobalValue *Old) override;
-  bool isMetadataNeeded(Metadata *MD) override;
 };
 
 class LocalValueMaterializer final : public ValueMaterializer {
@@ -361,7 +360,6 @@ public:
   LocalValueMaterializer(IRLinker &TheIRLinker) : TheIRLinker(TheIRLinker) {}
   Value *materializeDeclFor(Value *V) override;
   void materializeInitFor(GlobalValue *New, GlobalValue *Old) override;
-  bool isMetadataNeeded(Metadata *MD) override;
 };
 
 /// This is responsible for keeping track of the state used for moving data
@@ -405,7 +403,7 @@ class IRLinker {
   /// 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.
-  SmallPtrSet<const Metadata *, 16> UnneededSubprograms;
+  bool HasUnneededSPs = false;
 
   /// Handles cloning of a global values from the source module into
   /// the destination module, including setting the attributes and visibility.
@@ -472,15 +470,13 @@ class IRLinker {
 
   void linkNamedMDNodes();
 
-  /// Populate the UnneededSubprograms set with the DISubprogram metadata
-  /// from the source module that we don't need to link into the dest module,
-  /// because the functions were not imported directly or via an inlined body
-  /// in an imported function.
-  void findNeededSubprograms();
-
-  /// The value mapper leaves nulls in the list of subprograms for any
-  /// in the UnneededSubprograms map. Strip those out of the mapped
-  /// compile unit.
+  /// Look for subprograms referenced from !llvm.dbg.cu that we don't want to
+  /// link in and map it to nullptr.
+  ///
+  /// \post HasUnneededSPs is true iff any unneeded subprograms were found.
+  void mapUnneededSubprograms();
+
+  /// Remove null subprograms from !llvm.dbg.cu.
   void stripNullSubprograms(DICompileUnit *CU);
 
 public:
@@ -496,11 +492,6 @@ public:
   bool run();
   Value *materializeDeclFor(Value *V, bool ForAlias);
   void materializeInitFor(GlobalValue *New, GlobalValue *Old, bool ForAlias);
-
-  /// 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.
-  bool isMetadataNeeded(Metadata *MD);
 };
 }
 
@@ -534,10 +525,6 @@ void GlobalValueMaterializer::materializ
   TheIRLinker.materializeInitFor(New, Old, false);
 }
 
-bool GlobalValueMaterializer::isMetadataNeeded(Metadata *MD) {
-  return TheIRLinker.isMetadataNeeded(MD);
-}
-
 Value *LocalValueMaterializer::materializeDeclFor(Value *V) {
   return TheIRLinker.materializeDeclFor(V, true);
 }
@@ -547,10 +534,6 @@ void LocalValueMaterializer::materialize
   TheIRLinker.materializeInitFor(New, Old, true);
 }
 
-bool LocalValueMaterializer::isMetadataNeeded(Metadata *MD) {
-  return TheIRLinker.isMetadataNeeded(MD);
-}
-
 Value *IRLinker::materializeDeclFor(Value *V, bool ForAlias) {
   auto *SGV = dyn_cast<GlobalValue>(V);
   if (!SGV)
@@ -578,19 +561,6 @@ void IRLinker::materializeInitFor(Global
     linkGlobalValueBody(*New, *Old);
 }
 
-bool IRLinker::isMetadataNeeded(Metadata *MD) {
-  // Currently only DISubprogram metadata is marked as being unneeded.
-  if (UnneededSubprograms.empty())
-    return true;
-  MDNode *Node = dyn_cast<MDNode>(MD);
-  if (!Node)
-    return true;
-  DISubprogram *SP = getDISubprogram(Node);
-  if (!SP)
-    return true;
-  return !UnneededSubprograms.count(SP);
-}
-
 /// Loop through the global variables in the src module and merge them into the
 /// dest module.
 GlobalVariable *IRLinker::copyGlobalVariableProto(const GlobalVariable *SGVar) {
@@ -1064,7 +1034,7 @@ bool IRLinker::linkGlobalValueBody(Globa
   return false;
 }
 
-void IRLinker::findNeededSubprograms() {
+void IRLinker::mapUnneededSubprograms() {
   // Track unneeded nodes to make it simpler to handle the case
   // where we are checking if an already-mapped SP is needed.
   NamedMDNode *CompileUnits = SrcM->getNamedMetadata("llvm.dbg.cu");
@@ -1087,13 +1057,14 @@ void IRLinker::findNeededSubprograms() {
       if (auto *SP = getDISubprogram(dyn_cast<MDNode>(IE->getScope())))
         ImportedEntitySPs.insert(SP);
     }
-    for (auto *Op : CU->getSubprograms()) {
-      // 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).
-      if (!ValueMap.getMappedMD(Op) && !ImportedEntitySPs.count(Op))
-        UnneededSubprograms.insert(Op);
-    }
+
+    // Try to insert nullptr into the map for any SP not referenced from
+    // functions and not in the imported entities.  If the insertino succeeded,
+    // set HasUnneededSPs.
+    for (auto *Op : CU->getSubprograms())
+      if (!ImportedEntitySPs.count(Op))
+        if (ValueMap.MD().insert(std::make_pair(Op, TrackingMDRef())).second)
+          HasUnneededSPs = true;
   }
 }
 
@@ -1101,7 +1072,7 @@ void IRLinker::findNeededSubprograms() {
 void IRLinker::stripNullSubprograms(DICompileUnit *CU) {
   // There won't be any nulls if we didn't have any subprograms marked
   // as unneeded.
-  if (UnneededSubprograms.empty())
+  if (!HasUnneededSPs)
     return;
   SmallVector<Metadata *, 16> NewSPs;
   NewSPs.reserve(CU->getSubprograms().size());
@@ -1119,7 +1090,7 @@ void IRLinker::stripNullSubprograms(DICo
 
 /// Insert all of the named MDNodes in Src into the Dest module.
 void IRLinker::linkNamedMDNodes() {
-  findNeededSubprograms();
+  mapUnneededSubprograms();
   const NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata();
   for (const NamedMDNode &NMD : SrcM->named_metadata()) {
     // Don't link module flags here. Do them separately.
@@ -1132,7 +1103,7 @@ void IRLinker::linkNamedMDNodes() {
           op, ValueMap, ValueMapperFlags | RF_NullMapMissingGlobalValues,
           &TypeMap, &GValMaterializer);
       // For each newly mapped compile unit remove any null subprograms,
-      // which occur when findNeededSubprograms identified any as unneeded
+      // which occur when mapUnneededSubprograms identified any as unneeded
       // in the dest module.
       if (auto *CU = dyn_cast<DICompileUnit>(DestMD))
         stripNullSubprograms(CU);

Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265229&r1=265228&r2=265229&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Sat Apr  2 12:12:00 2016
@@ -190,9 +190,6 @@ static Metadata *mapMetadataOp(Metadata
   if (!Op)
     return nullptr;
 
-  if (Materializer && !Materializer->isMetadataNeeded(Op))
-    return nullptr;
-
   if (Metadata *MappedOp = MapMetadataImpl(Op, DistinctWorklist, VM, Flags,
                                            TypeMapper, Materializer))
     return MappedOp;




More information about the llvm-commits mailing list