[PATCH] D16124: Add to the split module utility an SCC based method which allows not to globalize any local variables.

[Sergei Larin via llvm-commits]

slarin added a comment.

Excellent suggestions. New patch will be up shortly.


================
Comment at: lib/Transforms/Utils/SplitModule.cpp:64-79
@@ +63,18 @@
+                          const GlobalValue *GV, const Comdat *C) {
+  // If this variable shares comdat marker with some other function, it is the
+  // same as having a "use" for our purposes.
+  // Note that it includes both external and local objects.
+  ComdatMembers.insert(std::make_pair(C, GV));
+
+  for (auto &&CM : make_range(ComdatMembers.equal_range(C))) {
+    if (CM.second == GV)
+      continue;
+    DEBUG(dbgs() << "In the same comdat: (" << GV->getName() << ") and ("
+                 << CM.second->getName() << ")\n");
+    if (GV->hasLocalLinkage()) {
+      GVtoClusterMap.unionSets(GV, CM.second);
+    } else if (CM.second->hasLocalLinkage()) {
+      GVtoClusterMap.unionSets(CM.second, GV);
+    }
+  }
+}
----------------
pcc wrote:
> I realized that you can reimplement this function in a simpler way that also avoids non-determinism in your loop (`unordered_multimap` can re-order values non-deterministically).
> 
> ```
> llvm::DenseMap<Comdat *, GlobalVariable *> ComdatMembers;
> [...]
> auto &Member = ComdatMembers[C];
> if (Member)
>   GVToClusterMap.unionSets(Member, GV);
> else
>   Member = GV;
> ```
Yes, I like this!

================
Comment at: lib/Transforms/Utils/SplitModule.cpp:162
@@ +161,3 @@
+  // When size is the same, use leader's name.
+  for (ClusterMapType::iterator I = GVtoClusterMap.begin(),
+                                E = GVtoClusterMap.end();
----------------
pcc wrote:
> Did you look into this?
Yes.. It is needed. Tobias is describing why now... 


http://reviews.llvm.org/D16124