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

[Zachary Turner via Phabricator via llvm-commits]

zturner added reviewers: ruiu, rnk.
zturner requested changes to this revision.
zturner added a comment.
This revision now requires changes to proceed.

+ruiu and +rnk for the substance of the patch.  Some style issues inline.



================
Comment at: lld/COFF/ICF.cpp:102
+
+  auto curGroupBegin = Begin;
+  for (auto curPos = Begin + 1; curPos < End; curPos++)
----------------
We need to use LLVM naming conventions here, which is that all variables are CamelCase.


================
Comment at: lld/COFF/ICF.cpp:127-129
+bool lexicographical_compare_helper (Data add_data1, InputIt1 first1, InputIt1 last1,
+                             Data add_data2, InputIt2 first2, InputIt2 last2,
+                             Compare comp)
----------------
Same here regarding naming conventions.  Also global functions should be marked static so they don't get external linkage.


================
Comment at: lld/COFF/ICF.cpp:154-155
+
+  if (A->getContents() != B->getContents ())
+    return std::lexicographical_compare (A->getContents ().begin(), A->getContents ().end(), B->getContents ().begin(), B->getContents ().end());
 
----------------
This patch will need to be be clang-formatted.  If `clang\tools\clang-format\git-clang-format` is in your `PATH`, then you can just run `git clang-format` from the git source root and it will will format the diff.


================
Comment at: lld/COFF/ICF.cpp:178
+
+  return lexicographical_compare_helper (A, A->Relocs.begin(), A->Relocs.end(), B, B->Relocs.begin(), B->Relocs.end(),  Less);
 }
----------------
This is only ever called as `lexicographical_compare_helper(X, X->Relocs.begin(), X->Relocs.end(), Y, Y->Relocs.begin(), Y->Relocs.end())` where `X` and `Y` are of type `const SectionChunk *`.  Can you just make this function take `const SectionChunk *A, const SectionChunk *B` and have the function get the two iterators out?


https://reviews.llvm.org/D36963