[PATCH] D93351: [llvm-shlib] Build backend libraries as loadable modules

[Stella Laurenzo via Phabricator via llvm-commits]

stellaraccident added inline comments.


================
Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:34
+      list(REMOVE_ITEM LIB_NAMES "LLVM${component}")
+      list(APPEND objects "$<TARGET_OBJECTS:LLVM${component}>")
+    endforeach()
----------------
tstellar wrote:
> stellaraccident wrote:
> > I am somewhat surprised this works -- I usually try to avoid this kind of layering issue, because while it can work, it usually has weird edge cases.
> > 
> > In this case, each of your target backends will directly embed (and export!) its dependencies, which, are probably also embedded and exported by the libLLVM that loads it. Aside from bloat, this can cause issues with module statics being duplicated depending on how the child library references things in the module (tends to manifest with cl conflicts in the support library at runtime). Based on the sizes you report, I suspect this is adding ~a megabyte to each of your backends (completely guessing based on what I have seen).
> > 
> > There are two ways to later this better:
> > - arrange for the dependencies to always exist in libLLVM and then just don't include them here. Because you are dlopen'ing from there, this should work on Linux, even without an ld-level link. Might not work on osx. Can't work on Windows.
> > - my rework of the cmake bits for libLLVM layers this properly but will take some time/consensus. I feel like that is the better approach, but imo, of this works for the cases your targeting and stems some issues, I'm not opposed.
> > There are two ways to later this better:
> > - arrange for the dependencies to always exist in libLLVM and then just don't include them here. Because you are dlopen'ing from there, this should work on Linux, even without an ld-level link. Might not work on osx. Can't work on Windows.
> > - my rework of the cmake bits for libLLVM layers this properly but will take some time/consensus. I feel like that is the better approach, but imo, of this works for the cases your targeting and stems some issues, I'm not opposed.
> 
> If you have a better solution that is great.  Right now, I'm not in a rush to get this in.  
> 
> 
I don't want to hold up progress, but I do think that the approach I'm advocating in this RFC will get us the library organization we want, will work across platforms/downstreams/etc: http://lists.llvm.org/pipermail/llvm-dev/2021-January/147567.html

Modulo the name of the target .so file, what I prototyped there already breaks the libraries up in roughly this way. Actually wiring it up for dynamically loading the target libraries vs hard linking to them would be similar to what you have here (the loader cpp file would be almost the same), but we would likely need to put it somewhere else (into its own component). If we go forward with my RFC, I can just design that in as an incremental thing.

There are some risks with my approach, mainly around timing and my availability to finish it. Given the number of moving pieces and age of the code, I could easily see the transition taking weeks to a small number of months to land (it isn't that much work but needs to be sequenced carefully/thoughtfully). Like I said, I don't want to block progress, but it would be somewhat easier to add this feature to what I'm doing than to land this patch first, reverse it and re-implement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93351