enable -flto=thin in clang-cl

[inglorion]

Summary

This enables LTO to be used with the clang-cl frontend.

[pcc]

Can you please add a ThinLTO test to lld before adding driver support?

[mehdi_amini]

In D30239#683116, @pcc wrote:

Can you please add a ThinLTO test to lld before adding driver support?

Is clang-cl using lld as default? How is the switch done? Ideally we should have a nice error message from the driver if -flto is used without lld.

[inglorion]

@mehdi_amini:

Is clang-cl using lld as default? How is the switch done? Ideally we should have a nice error message from the driver if -flto is used without lld.

I believe we use link.exe by default. You can use lld by passing -fuse-ld=lld to the compiler.

I can add an error message when -flto is used without -fuse-ld=lld at least for the case when linking is actually performed. Of course, it's possible to invoke clang-cl without it doing any linking. If you're only compiling, it's perfectly valid to use -flto without -fuse-ld=lld.

[hans]

In D30239#683697, @inglorion wrote:

@mehdi_amini:

Is clang-cl using lld as default? How is the switch done? Ideally we should have a nice error message from the driver if -flto is used without lld.

I believe we use link.exe by default. You can use lld by passing -fuse-ld=lld to the compiler.

I can add an error message when -flto is used without -fuse-ld=lld at least for the case when linking is actually performed. Of course, it's possible to invoke clang-cl without it doing any linking. If you're only compiling, it's perfectly valid to use -flto without -fuse-ld=lld.

That sounds like a good idea.

test/Driver/cl-lto.c

2 ↗

(On Diff #89315)

This test and the other file look like they're doing the same as test/Driver/(thin)lto.c.

I don't think we need to test these separately for clang-cl; it should be enough with a simple test in test/Driver/cl-options.c to check that they're exposed and that -### looks right for them.

[inglorion]

Implemented @hans's suggestion of moving the tests into cl-options.c.

Also restricted the implementation to only implement -flto=. -flto-index= doesn't seem useful without further changes (it just complains that it requires -x ir, which is not supported by clang-cl). -flto-jobs would require a fair bit of extra work compared to what is here now, and I'm not sure it's worth it. The number of jobs lld-link uses can be controlled with /link:/opt:lldltojobs=N, anyway. We can implement support for this later if we want it - for now I really just want -flto=thin.

[hans]

lgtm with a comment

test/Driver/cl-options.c

525

This needs -- before %s, otherwise if %s expands to e.g. /Users/foo it will be interpreted as the /U option :-)

Same thing below.

[inglorion]

test/Driver/cl-options.c

525

Good catch. That's what I get for not copy-pasting it. ;-)

[inglorion]

added missing --

[inglorion]

fail early with a friendlier message when using -flto without -fuse-ld=lld

[hans]

test/Driver/cl-options.c

532

This is nit-picky, but "invalid argument" doesn't sound great to me here; -flto isn't invalid, it's the lack of -fuse-ld=lld that's the problem.

Maybe just adding a new "err_drv_flto_without_lld" that says somethihng like "'-flto' without '-fuse-ld=lld' is not supported" would be better?

[inglorion]

changed error message

[hans]

lgtm2

[rnk]

Do you guys think that maybe -flto should imply -fuse-ld=lld, or is that too much magic?

[mehdi_amini]

In D30239#691972, @rnk wrote:

Do you guys think that maybe -flto should imply -fuse-ld=lld, or is that too much magic?

I don't know enough about windows, but I would think it may be surprising for a user that -flto changes the linker behind his back?

I'm biased but as a user I rather have to opt-in instead of the compiler doing stuff without me knowing about it.

include/clang/Basic/DiagnosticDriverKinds.td

 def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
 def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;
 def err_drv_invalid_remap_file : Error<
     "invalid option '%0' not of the form <from-file>;<to-file>">;
 def err_drv_invalid_gcc_output_type : Error<
     "invalid output type '%0' for use with gcc tool">;
 def err_drv_cc_print_options_failure : Error<
     "unable to open CC_PRINT_OPTIONS file: %0">;
+def err_drv_lto_without_lld : Error<"LTO requires -fuse-ld=lld">;
 def err_drv_preamble_format : Error<
     "incorrect format for -preamble-bytes=N,END">;
 def err_drv_conflicting_deployment_targets : Error<
   "conflicting deployment targets, both '%0' and '%1' are present in environment">;
 def err_arc_unsupported_on_runtime : Error<
   "-fobjc-arc is not supported on platforms using the legacy runtime">;
 def err_arc_unsupported_on_toolchain : Error< // feel free to generalize this
   "-fobjc-arc is not supported on versions of OS X prior to 10.6">;

include/clang/Driver/Options.td

def fxray_instruction_threshold_EQ :
   HelpText<"Sets the minimum function size to instrument with XRay">;
 def fxray_instruction_threshold_ :
   JoinedOrSeparate<["-"], "fxray-instruction-threshold">,
   Group<f_Group>, Flags<[CC1Option]>;
 
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group<f_Group>;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group<f_Group>;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group<f_Group>,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group<f_Group>,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group<f_Group>,
   HelpText<"Enable LTO in 'full' mode">;
 def fno_lto : Flag<["-"], "fno-lto">, Group<f_Group>,
   HelpText<"Disable LTO mode (default)">;
 def flto_jobs_EQ : Joined<["-"], "flto-jobs=">,
   Flags<[CC1Option]>, Group<f_Group>,
   HelpText<"Controls the backend parallelism of -flto=thin (default "

lib/Driver/Driver.cpp

void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
   if (!SuppressMissingInputWarning && Inputs.empty()) {
     Diag(clang::diag::err_drv_no_input_files);
     return;
   }
 
   Arg *FinalPhaseArg;
   phases::ID FinalPhase = getFinalPhase(Args, &FinalPhaseArg);
 
-  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
+  if (FinalPhase == phases::Link) {
+    if (Args.hasArg(options::OPT_emit_llvm))
       Diag(clang::diag::err_drv_emit_llvm_link);
+    if (IsCLMode() && LTOMode != LTOK_None &&
+        !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+      Diag(clang::diag::err_drv_lto_without_lld);
   }
 
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
     Diag(clang::diag::err_drv_use_of_Z_option) << A->getAsString(Args);
 
   // Diagnose misuse of /Fo.

test/Driver/cl-options.c

 // RUN: env CL="/Gy" %clang_cl -### -- %s 2>&1 | FileCheck -check-prefix=ENV-CL %s
 // ENV-CL: "-ffunction-sections"
 
 // RUN: env CL="/Gy" _CL_="/Gy- -- %s" %clang_cl -### 2>&1 | FileCheck -check-prefix=ENV-_CL_ %s
 // ENV-_CL_-NOT: "-ffunction-sections"
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck -check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
+// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
 // RUN:     --driver-mode=cl \
 // RUN:     -fcolor-diagnostics \
 // RUN:     -fno-color-diagnostics \
 // RUN:     -fdiagnostics-color \
 // RUN:     -fno-diagnostics-color \