[PATCH] Fix libLTO.so -disable-opt and other gold plugin options recently moved to lto.cpp

Rafael EspĂ­ndola rafael.espindola at gmail.com
Wed Oct 2 07:34:03 PDT 2013


On 1 October 2013 19:26, Tom Roeder <tmroeder at google.com> wrote:
> I just discovered that my recent patch (the one that ended up as
> r191680) that moved some options around in the LTO code permanently
> disabled these options for the gold plugin. The options affected are
> the ones that were moved to tools/lto/lto.cpp: -disable-opt,
> -disable-inlining, and -disable-gvn-loadpre.
>
> The problem was that the command-line option parsing wasn't happening
> until just before code generation in lib/LTO/LTOCodeGenerator.cpp, and
> this was too late for the options to be set, since they are now passed
> down from lto.cpp. So, this patch pulls out the command-line parsing
> into a separate function in LTOCodeGenerator and calls it right before
> code generation or merged module generation.

The patch LGTM. I will commit it in one sec.

> However, I don't have a test case for this; llvm-lto wasn't broken by
> the change; its options work correctly. And we don't currently have a
> test directory for LLVMgold.so or any regression tests that I know of.
> This is probably because LLVMgold.so depends on the gold plugin, hence
> on binutils, which isn't guaranteed to be present. But this bug
> suggests that there needs to be some way to track regressions for
> LLVMgold.so.
>
> I suppose the right way to do this is to write tests conditional on
> the presence of gold and put them in test/tools/gold. Are there
> options in the test framework that could be used to select for this?

It would probably be better to have a dummy linker program that is
similar to llvm-lto, but links (dlopens?) libLTO.so and/or
LLVMgold.so.

I do think your work on llvm-lto already covers most of the testing we
need, so I would wait to see if other bugs show up before implementing
a new testing program.

Cheers,
Rafael



More information about the llvm-commits mailing list