[lld] registry design proposal

Rui Ueyama ruiu at google.com
Wed Dec 4 00:00:15 PST 2013


It generally seems good and cleaner than the current architecture.


On Tue, Dec 3, 2013 at 6:18 PM, Nick Kledzik <kledzik at apple.com> wrote:

> I’m working on mach-o support for the linker and want transparent support
> for mach-o as yaml or binary.  I also want to make sure that it is possible
> to make a mach-o linker that does *not* support yaml or archives (such as
> for an embedded JIT linker).  In working out how to do this I noticed a
> bunch of design issues in lld.
>
> 1) LinkingContext has a bunch of oddball methods that have just been added
> because it was convenient (such as getDefaultHandler which is really only
> needed by the Archive Reader).
> 2) The native file format does not record the “universe” for the kind
> field of References.  That is, a mach-o file could be converted to
> native and an ELF file converted to native and both may have a reference of
> kind 42, but the same 42 has different semantics.
> 3) identify_magic() is called redundantly in lots of places.
> 4) We don’t have a way for yaml to hand off control if a yaml test case
> contains heterogenous yaml file types.
> 5) Readers have been refactored into a trivial role.  They just
> instantiate some File subclass.
>
>
> There is a design pattern that solves all these problems.  Have a global
> registry of “handlers”.  Require clients to call functions to which
> register the handlers they request.  Then any code that needs a handler
> calls the global registry to call the needed handler.
>
> I see the need for three registries:  a file parser registry, a yaml
> document tag registry, and a kind string registry.
>
> Here is one way to implement a registry for the file parser:
>
> class InputParserRegistry {
> public:
>   // A function to quickly check if a file can be parsed.  Usually only
> the
>   // magic parameter is checked.  But it can also check the file
> extension, or
>   // even look at more of the buffer if needed.
>   typedef bool (*InputFileChecker)(file_magic magic, StringRef
> fileExtension,
>                                    const MemoryBuffer &mb);
>
>   // A function to parse the supplied input file.
>   typedef error_code (*InputFileParser)(std::unique_ptr<MemoryBuffer> &mb,
>                                 std::vector<std::unique_ptr<File>>
> &result);
>
>   // Registers a checker and parser with the global registry.
>   // Usually, the checker and parser are static methods in some lld::File
>   // subclass that will be instantiated.
>   static void registerParser(InputFileChecker checker,
>                              InputFileParser parser);
>
>   // Walks registered checkers to parse the specified file.
>   static error_code parseFile(std::unique_ptr<MemoryBuffer> &mb,
>                               std::vector<std::unique_ptr<File>> &result);
> };
>

Is it intentional that these methods are static? By making them static, you
will have only one global registry for file handers. If your process calls
LLD APIs multiple times with different configurations, you need to reset
the handlers after one iteration. It wouldn't work if it's multi-threaded.

The mach-o linker tool would call:
>    registryAddSupportMachO();
>    registryAddSupportArchives();
>    registryAddSupportNative();
>
> The implementation of registryAddSupportMachO would look like:
>   void registryAddSupportMachO() {
>     InputParserRegistry::registerParser(FileMachO::check,
> FileMachO::parse);
>   }
>
> The code (in custom InputGraph nodes) no longer needs to call
> identify_magic, or look at the file extension, or handle archives.  All it
> does is:
>   InputParserRegistry::parseFile(_buffer, _files);
>
> I’ve looked at the llvm::Registry<> classes.  It does not seem to be a
> good match for what we need here:  It is designed around using static
> variables whose construction does the registration.  I prefer explicit
> calls to register to enable clients to *not* drag in support they don’t
> want.  The  Registry<> model also assume you want to instantiate the object
> with the default constructor, which we don’t.  And we have no need for
> listeners or name/description. Yes, by overriding SimpleRegistryEntry and
> RegistryTraits we could use  llvm::Registry<>, but I’m not sure it is worth
> the effort.
>
>
> Overall, I want to see what you think of this pattern.  Once we have it
> worked out, I want to do a similar registry for:
> 1) yaml doc tags (so that various kinds of yaml (archive, atom, mach-o,
> elf, etc) can be intermixed in one test case.
> 2) Add a namespace registry for kind values.
>
> -Nick
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131204/fa714130/attachment.html>


More information about the llvm-commits mailing list