[Clang][LLVMGold] Passing LLVM arguments to gold plugin

[bunty2020]

Summary

Currently CLANG does not pass LLVM options to gold plugin.
Ex: -mllvm <some-option> is not passed on to gold plugin.
This patch enables passing LLVM arguments to gold plugin

[tejohnson]

Please add llvm-commits as a subscriber for all patches, so that the patch
and review gets posted to the mailing list. I've cc'ed llvm-commits here,
which might be enough to get llvm-commits added.

[tejohnson]

lib/Driver/Tools.cpp

8375

Should be moved to AddGoldPlugin called just above.

Seems reasonable to pass these along to the plugin, rather than require the user to know to pass them via -Wl,-plugin-opt.

[bunty2020]

Moved changes to AddGoldPlugin function

[bunty2020]

This comment was removed by bunty2020.

[bunty2020]

lib/Driver/Tools.cpp

8375

Moved to AddGoldPlugin

[tejohnson]

Please add a test to test/Driver/gold-lto.c.

[pcc]

Plugin-opt (and mllvm) aren't really meant for users, I'm not sure that we
need to be making them easier to specify like that.

Peter
bunty2020 updated this revision to Diff 57813.
bunty2020 added a comment.

Moved changes to AddGoldPlugin function

http://reviews.llvm.org/D20423

Files:

lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp

• lib/Driver/Tools.cpp

+++ lib/Driver/Tools.cpp
@@ -1923,6 +1923,10 @@

  else
    CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
}

+ for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+ CmdArgs.push_back(Args.MakeArgString("-plugin-opt="
+ + StringRef(A->getValue(0))));
+ }
}

/// This is a helper function for validating the optional refinement step

---

llvm-commits mailing list
[e-mail]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

[tejohnson]

In D20423#434665, @pcc wrote:

Plugin-opt (and mllvm) aren't really meant for users, I'm not sure that we
need to be making them easier to specify like that.

True but from a developer's standpoint this increases usability quite a bit. We're passing one developer option to another to make the handling of the -mllvm developer option uniform with and without -flto.

[pcc]

In D20423#434683, @tejohnson wrote:

In D20423#434665, @pcc wrote:

Plugin-opt (and mllvm) aren't really meant for users, I'm not sure that we
need to be making them easier to specify like that.

True but from a developer's standpoint this increases usability quite a bit. We're passing one developer option to another to make the handling of the -mllvm developer option uniform with and without -flto.

Okay, but It isn't really uniform though because the flag only works with gold (and not ld64, lld etc).

That said I don't feel too strongly about it, so while it would be nice if the flag worked with those linkers, that doesn't need to happen in this patch.

[bunty2020]

Added test in test/Driver/gold-lto.c

[mehdi_amini]

These are developer options, doing -Wl, explicitly seems appropriate to me.

[tejohnson]

In D20423#435268, @joker.eph wrote:

These are developer options, doing -Wl, explicitly seems appropriate to me.

As a developer it is easier if I don't need to change the way I pass the developer options between a regular -O2 and a "-O2 -flto" build. Also, it would be nice not to have change the way I pass the developer option between the -flto compile and link steps e.g. the "-flto -c foo.c" and "-flto foo.o" steps. Why not provide the added convenience for developers?

(The patch currently LGTM, but I would like to wait until there is agreement with joker.eph before I accept)

[mehdi_amini]

I'm don't have a strong opinion, let me ask what Duncan think of that.

[llvm-commits]

This seems strange to me. For example, it breaks the otherwise very convenient:

$ clang -flto t.c -mllvm -some-internal-option-for-cc1

I don't understand how it's better.

[bunty2020]

Clang's help shows the following description:
-mllvm <value> Additional arguments to forward to LLVM's option processing

If -mllvm <some-option> sets some internal option for cc1 then its strange for front-end parser using some option which is supposed to be LLVM's option.

[mehdi_amini]

In D20423#440539, @bunty2020 wrote:

Clang's help shows the following description:
-mllvm <value> Additional arguments to forward to LLVM's option processing

If -mllvm <some-option> sets some internal option for cc1 then its strange for front-end parser using some option which is supposed to be LLVM's option.

cc1 is not just the front-end, it is the front-end, the optimizer, and the codegen (unless emitting bitcode).

lib/Driver/Tools.cpp

if (Arg *A = Args.getLastArg(options::OPT_gTune_Group,
                                options::OPT_ggdbN_Group)) {
     if (A->getOption().matches(options::OPT_glldb))
       CmdArgs.push_back("-plugin-opt=-debugger-tune=lldb");
     else if (A->getOption().matches(options::OPT_gsce))
       CmdArgs.push_back("-plugin-opt=-debugger-tune=sce");
     else
       CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
   }
+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+    CmdArgs.push_back(Args.MakeArgString("-plugin-opt="
+                                         + StringRef(A->getValue(0))));
+  }
 }
 
 /// This is a helper function for validating the optional refinement step
 /// parameter in reciprocal argument strings. Return false if there is an error
 /// parsing the refinement step. Otherwise, return true and set the Position
 /// of the refinement step in the input string.
 static bool getRefinementStep(StringRef In, const Driver &D,
                               const Arg &A, size_t &Position) {
void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
   Args.AddAllArgs(CmdArgs, options::OPT_r);
 
   if (D.isUsingLTO())
     AddGoldPlugin(ToolChain, Args, CmdArgs, D.getLTOMode() == LTOK_Thin);
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     addOpenMPRuntime(CmdArgs, ToolChain, Args);
     if (D.CCCIsCXX()) {
       ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
       if (Args.hasArg(options::OPT_pg))
         CmdArgs.push_back("-lm_p");
       else
         CmdArgs.push_back("-lm");

test/Driver/gold-lto.c

 // RUN: touch %t.o
 //
 // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
 // RUN:     -Wl,-plugin-opt=foo -O3 \
 // RUN:     | FileCheck %s --check-prefix=CHECK-X86-64-BASIC
 // CHECK-X86-64-BASIC: "-plugin" "{{.*}}/LLVMgold.so"
 // CHECK-X86-64-BASIC: "-plugin-opt=O3"
 // CHECK-X86-64-BASIC: "-plugin-opt=foo"
 //
 // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
+// RUN:     -mllvm -debug \
+// RUN:     | FileCheck %s --check-prefix=CHECK-X86-64-MLLVM
+// CHECK-X86-64-MLLVM: "-plugin" "{{.*}}/LLVMgold.so"
+// CHECK-X86-64-MLLVM: "-plugin-opt=-debug"
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
 // RUN:     -march=corei7 -Wl,-plugin-opt=foo -Ofast \
 // RUN:     | FileCheck %s --check-prefix=CHECK-X86-64-COREI7
 // CHECK-X86-64-COREI7: "-plugin" "{{.*}}/LLVMgold.so"
 // CHECK-X86-64-COREI7: "-plugin-opt=mcpu=corei7"
 // CHECK-X86-64-COREI7: "-plugin-opt=O3"
 // CHECK-X86-64-COREI7: "-plugin-opt=foo"
 //
 // RUN: %clang -target arm-unknown-linux -### %t.o -flto 2>&1 \