[PATCH] D103662: [lld][MachO] Fix function starts section

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

If I understand correctly, since the previously implementation didn't sort the addresses, we could have negative deltas between the entries, but since the encoding uses unsigned LEBs, we were actually underflowing and encoding very large values. Could we make that explicit in the commit message?

Also, I realized that the FunctionStartsSection doesn't have a comment describing the format it's encoding... could we add that too?



================
Comment at: lld/MachO/SyntheticSections.cpp:602-610
+      addrs.push_back(defined->getVA());
     }
   }
+  llvm::sort(addrs);
+  uint64_t addr = in.header->addr;
+  for (uint64_t nextAddr : addrs) {
+    uint64_t delta = nextAddr - addr;
----------------
gkm wrote:
> Using `std::set<uint64_t>` and its iterator (which is ordered) will make for much more compact code, and the performance might be comparable.
std::set is explicitly discouraged for this use case: https://llvm.org/docs/ProgrammersManual.html#set

> If the elements in the set are large, then the relative overhead of the pointers and malloc traffic is not a big deal, but if the elements of the set are small, std::set is almost never a good choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103662