[llvm] r191042 - Fix LTO handling of module-level assembly (PR14152).

[Tom Roeder]

Here is a new version of the patch that removes the all-targets
dependency from lib/LTO, drops the changes to
utils/llvm-build/llvmbuild, and updates tools/llvm-lto, tools/lto, and
tools/gold to take the new initialization/all-targets dependency into
account.

I removed LTODisassembler.cpp from tools/lto, since its only task
seemed to be initialization, and I needed that independently, and
nothing seemed to be using it. At the same time, I added the function
lto_initialize to tools/lto and llvm-c/lto.h and used it for
initialization in gold.

I've tested this on Linux with cmake/ninja and configure/make, and
I've tested the gold plugin with the simple test case from
test/lib/LTO/cfi_endproc.ll.

Please take a look.

Thanks,

Tom

On Mon, Sep 23, 2013 at 1:36 PM, Daniel Dunbar <daniel at zuster.org> wrote:
> On Mon, Sep 23, 2013 at 12:20 PM, Tom Roeder <tmroeder at google.com> wrote:
>>
>> I think it could be refactored along these lines. I was hoping for
>> clients of the library not to have to know about the dependency but
>> simply to get it implicitly by depending on lib/LTO. Is there a way to
>> do this using llvm-build?
>
>
> No, but thats a feature. The goal is that the ultimate library clients
> always get to choose what targets they support, so individual libraries
> aren't supposed to force any particular target linkage.
>
>>
>> After looking through the code a bit, I see that the only callers of
>> the InitializeAll* routines are in tools/. So, if this is the existing
>> design direction, then I'll see about pulling those parts up into
>> clients of lib/LTO.
>
>
> Cool, thanks!
>
>  - Daniel
>
>>
>>
>> Tom
>>
>> On Mon, Sep 23, 2013 at 11:50 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>> > Hi Tom,
>> >
>> > Can you explain why lib/LTO needs to depend on all-targets? I'm not sure
>> > we
>> > actually want this behavior, what we generally want is that clients can
>> > pick
>> > the library and then pick the list of targets that get included. In this
>> > case, I think the right thing that should happen is that both tools/lto
>> > and
>> > llvm-lto depend on all targets and handle the target initialization,
>> > whereas
>> > lib/LTO itself just relies on the targets already having been
>> > initialized.
>> >
>> > Is it possible to factor lib/LTO in that fashion?
>> >
>> >  - Daniel
>> >
>> >
>> > On Fri, Sep 20, 2013 at 11:59 PM, Tom Roeder <tmroeder at google.com>
>> > wrote:
>> >>
>> >> I've written a patch now (in consultation with Peter) that does
>> >> exactly this: tools/llvm-lto is split into a lib/LTO component and a
>> >> tools/llvm-lto component, and tools/llvm-lto is written to depend only
>> >> on lib/LTO (and not tools/lto, hence not libLTO.so). This should solve
>> >> the problems with the freebsd build and additionally be a better
>> >> factorization of the code.
>> >>
>> >> However, this design led to an interesting problem: lib/LTO needs to
>> >> depend on all-targets, and having a library that depends on
>> >> all-targets wasn't supported by the existing llvm-build scripts (the
>> >> required_libraries of all-targets came up with empty
>> >> required_libraries of their own). So, I've modified
>> >> llvm-build/llvmbuild/main.py to fill out these implicit dependencies,
>> >> using the information from the add_to_library_groups entries in the
>> >> LLVMBuild.txt files.
>> >>
>> >> I've run clean builds with cmake/ninja, cmake/make, and Makefile and
>> >> run check successfully on each of them (on Linux). And I've built
>> >> LLVMgold.so against the Makefile version and tested it on a simple
>> >> test file (the test case from this patch).
>> >>
>> >> However, my patch as it stands combines svn mv operations with edits
>> >> on the moved files, so it's far more verbose than necessary and loses
>> >> some of the history information (by not exactly capturing the move
>> >> information). Would you like me to formulate this as two patches: a
>> >> move followed by edits?
>> >>
>> >> Please let me know what you think.
>> >>
>> >> Thanks,
>> >>
>> >> Tom
>> >>
>> >> On Fri, Sep 20, 2013 at 12:50 PM, Peter Collingbourne <peter at pcc.me.uk>
>> >> wrote:
>> >> > On Fri, Sep 20, 2013 at 12:25:17PM -0700, Tom Roeder wrote:
>> >> >> Actually, it looks like libLTO.so simply isn't getting built or
>> >> >> found
>> >> >> here
>> >> >> and that's why the test is failing.
>> >> >
>> >> > Hm?  The compile log:
>> >> >
>> >> >
>> >> > http://lab.llvm.org:8011/builders/clang-X86_64-freebsd/builds/10381/steps/compile/logs/stdio
>> >> > seems to indicate that libLTO is being built.
>> >> >
>> >> > I suspect that the problem is that the rpath isn't being embedded
>> >> > in the llvm-lto binary on FreeBSD in the correct way, though I can't
>> >> > tell for sure, since I don't have access to a FreeBSD machine.
>> >> >
>> >> > I'm beginning to think that the right way to solve this problem is
>> >> > to move LTOCodeGenerator.cpp and LTOModule.cpp in tools/lto into
>> >> > a new component 'lto' in lib/LTO and have tools/lto (which would
>> >> > then contain only the C interface) and tools/llvm-lto (and maybe
>> >> > tools/gold in the future) depend on the lto component.  That way,
>> >> > the LTO support library is no different to any other component of
>> >> > LLVM and is less likely to cause problems across platforms.
>> >> >
>> >> > Thanks,
>> >> > --
>> >> > Peter
>> >>
>> >> _______________________________________________
>> >> llvm-commits mailing list
>> >> llvm-commits at cs.uiuc.edu
>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >>
>> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lib_lto.patch
Type: application/octet-stream
Size: 135452 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130923/93afb178/attachment.obj>