[PATCH] D116913: [lld-macho] Add --start-lib --end-lib

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 18 19:33:12 PST 2022


MaskRay added a comment.

In D116913#3250279 <https://reviews.llvm.org/D116913#3250279>, @int3 wrote:

> So I realize that the choice of class hierarchy here was chosen to follow the LLD-ELF backend's design... but I'm not the biggest fan of it for two reasons:
>
> 1. `ObjFile`s and `BitcodeFile`s with `lazy = true` aren't proper instances of their classes; e.g. calling `->getDataInCode()` on a lazy ObjFile isn't a valid operation. IMO it would make more sense to have a separate LazyFile instance that gets converted to ObjFile / BitcodeFile as necessary. Kind of like an ArchiveFile but with only one member.
> 2. I have the opposite objection to `LazyObject`: it doesn't really merit the creation of a new class. Implementation-wise, it's indeed different from `LazyArchive`, but interface-wise, they both expose a single common operation: the fetching of their associated file. IMO it would be cleaner to have a single `LazySymbol` class with a `fetchFile()` method. It can use a PointerUnion to store a pointer to the `llvm::object::Archive::Symbol` or InputFile instance as necessary.
>
> All that said, I do see the sense in not deviating from LLD-ELF's architecture. If you prefer the hierarchy as-is, I'm good with that too.

A separate `LazyFile` class makes sharing information (symbols/sections) between the lazy and non-lazy states difficult.
(https://maskray.me/blog/2021-12-19-why-isnt-ld.lld-faster "initialization of sections" and "initialization of non-local symbols")
I think we are currently a bit far away from the goal, but I am carefully writing code not to make it harder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116913



More information about the llvm-commits mailing list