[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:214
+
+    return B1 < B2;
   };
----------------
alex.telishev wrote:
> alex.telishev wrote:
> > ruiu wrote:
> > > 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`?
> > Unfortunately, no - that would break strict ordering (`A < B == false` and `B < A == false`, like NaN) and  `stable_sort` will break in this case. It's possible to `return true` and make them all be equal to each other for the purposes of sorting. But in this case loop after `stable_sort` in `segregate` function needs to use different comparison. It's possible to use `equalsConstant` from before this patch (as we only need equality there), but it'll be a lot of copy-paste. 
> Sorry, it's exact opposite to what I said, `stable_sort` will break if `return true` is used, and on `return false;` everything works, but we need to change comparison in `segregate`.
I'm not sure if I understand your point correctly. If a comparison function returns false for both (A, B) and (B, A), then it means that A == B, and std::stable_sort sort them accordingly (i.e. do not alter relative order of A and B), no?

On the other hand, I believe returning true for both (A, B) and (B, A) breaks strict weak ordering, as A<B && B<A cannot be true.


https://reviews.llvm.org/D36963