On Tue, Nov 28, 2017 at 11:23 AM, David Blaikie via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Alternatively we can really say this header is a textual header - it's > included generally only once in a whole program, the functions are called > only once, etc. Though that does seem a little unfortunate on principle but > not much practical problem with it, I think. It'd be nice in theory to be > able to depend on the right library, have that bring in the right > dependencies, etc. >As designed, TargetSelect.h doesn't fit neatly into the normal way of arranging libraries. I'd mark it textual and leave it alone. Alternatively, we could make AllTargetsDescs and AllTargetsInfos and all the other synthetic libraries in CMake into real libaries and sink the bodies of these inline functions into each tiny little library. Doesn't seem quite worth it, though. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171128/af07ebc7/attachment.html>
On Tue, Nov 28, 2017 at 11:27 AM Reid Kleckner <rnk at google.com> wrote:> On Tue, Nov 28, 2017 at 11:23 AM, David Blaikie via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Alternatively we can really say this header is a textual header - it's >> included generally only once in a whole program, the functions are called >> only once, etc. Though that does seem a little unfortunate on principle but >> not much practical problem with it, I think. It'd be nice in theory to be >> able to depend on the right library, have that bring in the right >> dependencies, etc. >> > > As designed, TargetSelect.h doesn't fit neatly into the normal way of > arranging libraries. >Not sure about that - yeah, it's a bit of the degenerate case, for sure. But in a build system like Google's, where a lib has other lib dependencies (whereas in the LLVM CMake build it seems libs don't depend on other libs - so every executable has to explicitly list its transitive lib dependencies) it's pretty nice to have these little libraries explicitly in the build graph - much like we have those synthetic library targets in the CMake rules, so it's easy to depend on the right/full things. (but because the CMake lib rules for LLVM don't actually describe lib dependencies, I think even 'fixing' this in upstream LLVM wouldn't make the dep situation better - the synthetic targets would just have to expand to the underlying libs + the wrapper/selector lib as well)> I'd mark it textual and leave it alone. >Yeah, maybe... just makes me a bit sad to have inline functions that can't be trivially out-of-lined if/when desired, because they layering isn't fully/correctly represented in the build system. Modular codegen's been a good justification to flush out & fix several of these tricksy layering violations in LLVM already.> > Alternatively, we could make AllTargetsDescs and AllTargetsInfos and all > the other synthetic libraries in CMake into real libaries and sink the > bodies of these inline functions into each tiny little library. Doesn't > seem quite worth it, though. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171128/4893582f/attachment.html>
Ping - any further/other thoughts from folks? I'm not /too/ fussed, but generally like the idea of lib layering being simple/clear/obvious, but understand these are sort of the degenerate/worst case. On Tue, Nov 28, 2017 at 12:12 PM David Blaikie <dblaikie at gmail.com> wrote:> On Tue, Nov 28, 2017 at 11:27 AM Reid Kleckner <rnk at google.com> wrote: > >> On Tue, Nov 28, 2017 at 11:23 AM, David Blaikie via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> Alternatively we can really say this header is a textual header - it's >>> included generally only once in a whole program, the functions are called >>> only once, etc. Though that does seem a little unfortunate on principle but >>> not much practical problem with it, I think. It'd be nice in theory to be >>> able to depend on the right library, have that bring in the right >>> dependencies, etc. >>> >> >> As designed, TargetSelect.h doesn't fit neatly into the normal way of >> arranging libraries. >> > > Not sure about that - yeah, it's a bit of the degenerate case, for sure. > > But in a build system like Google's, where a lib has other lib > dependencies (whereas in the LLVM CMake build it seems libs don't depend on > other libs - so every executable has to explicitly list its transitive lib > dependencies) it's pretty nice to have these little libraries explicitly in > the build graph - much like we have those synthetic library targets in the > CMake rules, so it's easy to depend on the right/full things. > > (but because the CMake lib rules for LLVM don't actually describe lib > dependencies, I think even 'fixing' this in upstream LLVM wouldn't make the > dep situation better - the synthetic targets would just have to expand to > the underlying libs + the wrapper/selector lib as well) > > >> I'd mark it textual and leave it alone. >> > > Yeah, maybe... just makes me a bit sad to have inline functions that can't > be trivially out-of-lined if/when desired, because they layering isn't > fully/correctly represented in the build system. Modular codegen's been a > good justification to flush out & fix several of these tricksy layering > violations in LLVM already. > > >> >> Alternatively, we could make AllTargetsDescs and AllTargetsInfos and all >> the other synthetic libraries in CMake into real libaries and sink the >> bodies of these inline functions into each tiny little library. Doesn't >> seem quite worth it, though. >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171205/50713dc2/attachment.html>