[cfe-commits] Patch to support gold on FreeBSD

[Chandler Carruth]

On Tue, Jul 31, 2012 at 2:21 AM, Stephen Checkoway <s at pahtak.org> wrote:

>
> On Jul 31, 2012, at 4:10 AM, Chandler Carruth wrote:
>
> > Cool.
>
> Thanks for taking a look at this so quickly! I apologize for the length of
> this email.
>
> > First, this needs a test in the 'test/Driver/...' tree. Likely with
> other BSD 'ld' invocation tests. There are lots of examples to crib from,
> in particular the Linux ones.
>
> I'm actually not sure what the right way to test this is.


The tests shouldn't have anything to do with the actual system. Look at the
linux-ld.c tests. We use fake system root stub trees checked into the test
suite to simulate a sysroot, and then inspect the command line which would
be used to invoke ld without over invoking it. This is what you should
extend.

There are piles of tests for different OSes here, you should be able to
copy one to get a test for this feature.


> The base FreeBSD does not have gold so any test of gold shouldn't run. In
> fact, it seems like LTO support should be tested at configure time.
> Currently, Darwin and Linux (and FreeBSD with my patch) just report that
> they support LTO, even though the rest of the tool chain may not. This
> causes errors later:
>

Bleh, this is gross.

First, it is an explicit goal to avoid having the ./configure used to build
the clang binary really change much of its behavior. That makes it harder
to test and debug.

The driver tries to inspect the system in a very efficient way (filesystem
probes mostly) to determine the correct way to invoke 'ld'. I think it is
fine for the driver to look for 'ld.gold' and use that instead of 'ld' on
OSes where that is a plausible thing to do (it sounds like FreeBSD and
Debian, but I would want one of the FreeBSD toolchain maintainers to chime
in on FreeBSD there).


> > Second, can we factor out the logic to add the actual plugin flag? It
> should already exist somewhere for Linux?
>
> The freebsd::Link and linux::Link classes don't have much in the way of
> shared ancestry. They're both subclasses of Tool which seems like the wrong
> place to put such common code. Maybe they, and others that use GNU binutils
> should inherit from something more specific than Tool that could handle
> both adding adding the -plugin LLVMgold.so arguments as well as the
> appropriate selection of linker.
>
> Otherwise, it seems like factoring this out wouldn't save much. Here are
> the two implementations currently (I clearly copied from the Linux
> implementation, comment and all).
>

I'm really just looking for sharing the fiddly path logic, nothing really
comprehensive.


>
> Linux:
>   if (D.IsUsingLTO(Args) || Args.hasArg(options::OPT_use_gold_plugin)) {
>     CmdArgs.push_back("-plugin");
>     std::string Plugin = ToolChain.getDriver().Dir + "/../lib/LLVMgold.so";
>     CmdArgs.push_back(Args.MakeArgString(Plugin));
>   }
>
> FreeBSD:
>   const bool UseGold = D.IsUsingLTO(Args) ||
> Args.hasArg(options::OPT_use_gold_plugin);
>   if (UseGold) {
>     CmdArgs.push_back("-plugin");
>     std::string Plugin = getToolChain().getDriver().Dir +
> "/../lib/LLVMgold.so";
>     CmdArgs.push_back(Args.MakeArgString(Plugin));
>   }
>
> Something like
>
> static void addGoldPlugin(ArgStringList &CmdArgs, StringRef Plugin) {
>   CmdArgs.push_back("-plugin");
>   CmdArgs.push_back(Args.MakeArgString(Plugin.str()));
> }
>
> // ...
>   if (UseGold)
>     addGoldPlugin(CmdArgs, getToolChain().getDriver().Dir +
> "/../lib/LLVMgold.so");
>

How about sinking the relative path and file name into the static helper?

  addGoldPlugin(CmdArgs, getToolChain().getDriver().Dir);
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120731/cc4cd8e4/attachment.html>