[cfe-commits] Patch to support gold on FreeBSD

[Stephen Checkoway]

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 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:

[hilbert:~] steve$ uname -a
Linux hilbert 2.6.35-28-generic #50-Ubuntu SMP Fri Mar 18 18:42:20 UTC 2011 x86_64 x86_64 x86_64 GNU/Linux
[hilbert:~] steve$ ld --version
GNU ld (GNU Binutils for Ubuntu) 2.22
Copyright 2011 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.
[hilbert:~] steve$ clang -flto -x c - <<< 'int main(){}'
/tmp/--nsAnN9.o: file not recognized: File format not recognized
clang: error: linker command failed with exit code 1 (use -v to see invocation)

My Debian-based systems also have gold installed as ld.gold. Maybe it'd make sense for configure to look for gold by checking ld --version and if that's not gold then checking for ld.gold. Possibly allow the user to specify an alternative linker to use for LTO (the README in llvm/tools/gold mentions ld-new).

But back to tests, I'm not familiar enough with the test infrastructure to write this sort of conditional test. I've read <http://llvm.org/docs/TestingGuide.html>, but I'm not sure that's up to date. In particular, it says 2>&1 isn't supported but many tests do that. An XFAIL almost seems appropriate except that the expected failure isn't for a particular target but whether it has gold or not.

> 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).

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");

Thoughts?

-- 
Stephen Checkoway