[PATCH] Fix LTO handling of module-level assembly (Bug 14152)
tmroeder at google.com
Tue Sep 17 13:57:47 PDT 2013
Thanks for the feedback. Here's an updated patch that adds a simple
utility (llvm-lto) along these lines in utils/ and changes the test to
use this utility instead of ld.
On Mon, Sep 16, 2013 at 6:33 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> Hi Tom,
> I don't think that is the correct way to write the test, for a couple
> of reasons:
> - The test will fail if 'ld' is not gold on the user's system.
> - The test will not run on non-Linux systems.
> What I would suggest doing is writing a small command line test harness
> for the libLTO library, using the API defined in include/llvm-c/lto.h.
> The test harness could have a command line interface along the
> lines of:
> $ lto-test -o foo.o foo1.bc foo2.bc # Link foo1.bc and foo2.bc to produce foo.o
> You could then use lto-test from your lit test, instead of ld.
> On Mon, Sep 16, 2013 at 01:17:09PM -0700, Tom Roeder wrote:
>> Here's a patch that adds the test in LLVM IR to test/tools/lto with a
>> lit.local.cfg that checks if LTO is present. Also, the check for LTO
>> seems wrong to me in test/Makefile: it looks like it's checking to see
>> if -flto is in the command-line flags for the current build (rather
>> than -flto being possible with the compiler that was built). This
>> patch changes that to look for LLVMgold.so in the lib directory. No
>> other tests seem to be using the flag lto_is_enabled at the moment.
>> Please let me know what you think.
>> On Fri, Sep 13, 2013 at 5:30 PM, Tom Roeder <tmroeder at google.com> wrote:
>> > OK, thanks. I'll figure out another place to put it.
>> > On Sep 13, 2013 5:05 PM, "Bill Wendling" <wendling at apple.com> wrote:
>> >> On Sep 13, 2013, at 4:54 PM, Tom Roeder <tmroeder at google.com> wrote:
>> >> > Here is a pair of patches that fix and test
>> >> > http://llvm.org/bugs/show_bug.cgi?id=14512. The fix applies to LLVM,
>> >> > and I have currently put the test in the clang test suite. The test is
>> >> > a minor tweak to the test proposed in the bug. Please take a look.
>> >> >
>> >> > The problem in this case is that the RecordStreamer implementation of
>> >> > MCStreamer in the LTO module doesn't implement EmitCFIEndProcImpl, and
>> >> > the default implementation in MCStreamer doesn't finish the frame.
>> >> >
>> >> > This patch touches the LTO code in tools/lto, but the test uses clang.
>> >> > Is this the right way to do it? Is there a better location for LTO
>> >> > tests?
>> >> >
>> >> Hi Tom,
>> >> I haven't looked at the patch yet. However, the rule-of-thumb for where
>> >> tests should go is that they should be in located where the code that tests
>> >> them is. Clang doesn't have anything to do with LTO, so putting it in the
>> >> clang test suite isn't really good. You'll want to generate a .ll file and
>> >> place it in the LLVM part. That said, it's hard to know where such a test
>> >> would go, since we don't really have many for LTO...But just pick a good
>> >> place that's kind of related. :-)
>> >> -bw
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 7424 bytes
Desc: not available
More information about the llvm-commits