[PATCH] D136374: [lld-macho] Don't put entries with less than 2 usages into the common table.

Mon Oct 24 10:31:53 PDT 2022

oontvoo added a comment.

In D136374#3872249 <https://reviews.llvm.org/D136374#3872249>, @int3 wrote:

> Does this behavior difference really matter? We're not saving any bytes with this, right

This change isn't about saving the bytes - but rather, theoretically the size of the common-encoding table could change the paging logic. I'm still chasing down the missing exception-handling-code bug and noticed that this is one of a few differences.... just trying to narrow down the issue.

> But I'm wondering if we could at least reuse the compact-unwind.s test instead of adding a new file -- this seems like a lot of additional testing for a trivial behavior change. Or will your upcoming patch not fit into that test file?

it could kind of fit into compact-unwind.s but will need additional sub-files (or modifying the existing one to have more symbols to demonstrate this change), which I think doesn't really help with reducing complexity.
What is the concern? IMHO, it's easier to debug failures in general when there are multiple **separate** test files with single-purpose testings rather than trying to pack as many related things as possible into one giant file 🤔

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136374