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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Can you upload a patch between SVN head and your patch instead of one between your last patch and your local head? On reviews.llvm.org, I'm seeing the diffs between your patches instead of the main repository and your patch, which is making code review hard.



================
Comment at: lld/COFF/ICF.cpp:99
+
+  // I'm not sure if regular sort can be used here
+  std::stable_sort (Chunks.begin() + Begin, Chunks.begin() + End, Less);
----------------
zturner wrote:
> ruiu wrote:
> > This comment is not useful because it poses a question to a reader instead of answering a question for a reader.
> > 
> > You cannot actually use std::sort because we want a deterministic output in general. For a set of input files, there are a lot of valid link results for it, but we always want to output the same file, as it is important for build reproducibility.
> Using `std::sort` should not affect determinism right?  If you call `std::sort` two times with an identical input data set, you are guaranteed to get the same results.  The only thing `stable_sort` does is preserve relative ordering.  Even though `std::sort` doesn't preserve that ordering, ti still produces deterministic output.
We want to guarantee determinism even across machines. If you are using two hosts whose std::sort are not the same, your outputs are not guaranteed to be the same, no?


https://reviews.llvm.org/D36963