[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:247
+
+  return lexicographicalCompareRelocs(A, B, Less);
+}
----------------
It looks like this `lexicographicalCompareRelocs` function doesn't help code readability that much. You might want to just do things in-place instead of passing a callback function to another function. I'd do something like this:

  for (size_t I = 0; I != A->NumRelocs; ++I) {
    const coff_relocation *R1 = A->Relocs.begin() + I;
    const coff_relocation *R2 = B->Relocs.begin() + I;

    if (R1.Type != R2.Type)
      return R1.Type < R2.Type;
    if (R1.VirtualAddress != R2.VirtualAddress)
      return R1.VirtualAddress < R2.VirtualAddress;

    SymbolBody *B1 = A->File->getSymbolBody(R1.SymbolTableIndex);
    SymbolBody *B2 = B->File->getSymbolBody(R2.SymbolTableIndex);
    if (B1 == B2)
      continue;

    auto *D1 = dyn_cast<DefinedRegular>(B1);
    auto *D2 = dyn_cast<DefinedRegular>(B2);
    if (!D1 || D2)
      continue;
    if (D1->getValue() != D2->getValue())
      return D1->getValue() < D2->getValue();

    uint32_t C1 = D1->getChunk()->Class[Cnt % 2];
    uint32_t C2 = D2->getChunk()->Class[Cnt % 2];
    if (C1 != C2)
      return C1 < C2;
  }
  return false;

Note that when the control reaches this loop, A->NumReloc == B->NumReloc is guaranteed because we do have a check for that at the beginning of this function.


================
Comment at: lld/COFF/ICF.cpp:284
+  };
+  return lexicographicalCompareRelocs(A, B, Less);
+}
----------------
Ditto


https://reviews.llvm.org/D36963