[PATCH] D75382: [lld] Initial commit for new Mach-O backend

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 5 18:35:07 PST 2020


alexshap added a subscriber: dblaikie.
alexshap added inline comments.


================
Comment at: lld/MachO/OutputSegment.cpp:16
+
+std::vector<OutputSegment *> macho::outputSegments;
+
----------------
ruiu wrote:
> MaskRay wrote:
> > alexshap wrote:
> > > @ruiu, @pcc - just curious, would you mind adding some comments regarding the design considerations which motivated this approach (using global variables to store various parts of the object file, storing the state globally etc) ? 
> > I started to be heavily involved in lld/ELF since mid 2018. I do not know much about the previous history.
> > 
> > You find the discussions in D70421 and https://reviews.llvm.org/D70766#1761624 helpful. If we can avoid global variables, that will be nice.
> This variable is essentially a singleton, as you need only one list of output segments for each instance of the linker, and in many places you'll need to access it, so I decided to make it a global variable. Other singletons are also represented by global variables. In addition to that, this code is simply written in an old-fashioned way to reflect my taste, which shouldn't be necessarily bad. Actually I'm fairly satisfied with this design as it is pretty straightforward.
> 
> An alternative design is to define a "context" object and make that object to have all singletons like this. That design isn't bad too and is perhaps arguably better, but if you do that you'll end up having to add one extra parameter to virtually all functions.
> 
> Anyway, this topic tends to become a bikesheddy, so I'd focus on bringing up the mach-o linker while reusing the exact same design as the other ports.
@ruiu, for what it's worth, my 0.02$.
it seems like there are multiple downsides of this "design", starting from readability, clarity of the internal model of the output file, ending with, for example,  if one wanted to use LLD as a library and construct multiple instances of different output files, or, e.g. write unit-tests in memory to verify some parts of the functionality separately. Probably, the list can be continued. So it's not clear at all what dictates them to be singletons. 
If i am not mistaken at some point it was also mentioned in the LLVM Coding Standards https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors, some it would be interesting to find out why it was ignored for ELF/COFF.
Regarding the argument "I'd focus on bringing up the mach-o linker while reusing the exact same design as the other ports" - I fully support this effort of writing a modern solid implementation of LLD for MachO and, in particular, don't want to block this diff, however, this argument looks quite weak,  in the future when more complexity will be accumulated in this code it'll be much harder to change the course, at least it is hard to see how it can get any simpler.  cc: David @dblaikie 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75382





More information about the llvm-commits mailing list