[PATCH] D158875: [ADT] Fix IntEqClasses::join to return the leader in all cases.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/lib/Support/IntEqClasses.cpp:36-52
   // Update pointers while searching for the leaders, compressing the paths
   // incrementally. The larger leader will eventually be updated, joining the
   // classes.
   while (eca != ecb)
     if (eca < ecb) {
       EC[b] = eca;
       b = ecb;
----------------
jcranmer-intel wrote:
> MatzeB wrote:
> > Not sure how this would affect worst-case complexity given `findLeader` does not do any path compression. How about this instead?
> The changes I made wouldn't make the worst-case complexity any worse; it seems to me that it would work better to have `findLeader` do path compression instead (I further note that there appear to be no callers of `IntEqClasses::findLeader` in LLVM at present, so there's definitely no compile time regression here).
> The changes I made wouldn't make the worst-case complexity any worse

They obviously won't affect the case when we only call `join` on members of different sets. However without the changes I proposed (or adding path compression to `findLeader`) it's not obvious to me that we won't cannot hit bad complexity for the cases of calling `join` with on members of the same set (this is only a gut feeling without proof, but you're not presenting an argument against that either).

Anyway any reason not to go with my proposed changes? Anyway adding path-compression to `findLeader` should also work... Either way you should upload one variant :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158875/new/

https://reviews.llvm.org/D158875