[PATCH] D136374: [lld-macho] Don't put entries with less than 2 usages into the common table.
Vy Nguyen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list