[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.

[Re-sending as Phabricator automatically replaced B<number> with a URL.]

I believe you missed a point that we use symbols differently in comparison if all the others are the same. In this case, the address. Let me describe it more concretely.

Let's say we have three chunks, C1, C2 and C3 and symbols S1, S2 and S3. S1 and S3 are DefinedRegular, but S2 is not. Their addresses are S1<S2<S3. Each chunk has only one relocation, and C1 has S1 as a relocation target, C2 has S2 as a relocation target, and so on. All other chunk attributes (size, contents, alignment, etc.) are all the same.

Now consider lessVariable(C1, C2). At line 191, lessVariable returns true because S1<S2. How about lessVariable(C2, C3)? At line 191, lessVaraible returns true because S2<S3.  At this point, it concludes that C1<C2<C3. But that's wrong because both lessVaraible(C1, C3) and lessVariable(C3, C1) return false. That's what I was saying that the comparison function is not transitive.


https://reviews.llvm.org/D36963