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

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
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);
+    }
+  }
+}
----------------
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;
```

================
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();
----------------
Did you look into this?

================
Comment at: lib/Transforms/Utils/SplitModule.cpp:187
@@ +186,3 @@
+    for (ClusterMapType::member_iterator MI =
+             GVtoClusterMap.findLeader(I.second);
+         MI != GVtoClusterMap.member_end(); ++MI) {
----------------
If you store a `ClusterMapType::member_iterator` in `Sets` instead of a `GlobalValue *` you won't have to look it up again.


http://reviews.llvm.org/D16124