[PATCH] D36963: Use sorting instead of a lot of stable_partitions for `ICF::segregate` function

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/ICF.cpp:90-91
-  while (Begin < End) {
-    // Divide [Begin, End) into two. Let Mid be the start index of the
-    // second group.
-    auto Bound = std::stable_partition(
----------------
Could you keep this and the following comment in your new code somehow?


================
Comment at: lld/COFF/ICF.cpp:132-133
+                                              Compare Comp) {
+  auto LFirst = L->Relocs.begin(), LLast = L->Relocs.end();
+  auto RFirst = R->Relocs.begin(), RLast = R->Relocs.end();
+
----------------
One definition on each line and do not use `auto` for consistency.


================
Comment at: lld/COFF/ICF.cpp:188
       if (auto *D2 = dyn_cast<DefinedRegular>(B2))
-        return D1->getValue() == D2->getValue() &&
-               D1->getChunk()->Class[Cnt % 2] == D2->getChunk()->Class[Cnt % 2];
-    return false;
+      {
+        if (D1->getValue() != D2->getValue())
----------------
Please use clang-format, or if you do that by hand, format it as clang-format would do.


================
Comment at: lld/COFF/ICF.cpp:214
+
+    return B1 < B2;
   };
----------------
I think this comparison breaks determinism because it compares pointer values that change on every process invocation. You may be able to just return `false`?


https://reviews.llvm.org/D36963