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

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 ↗

(On Diff #89577)

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 ↗

(On Diff #89577)

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 ↗

(On Diff #89590)

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/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>,
+  Flags<[CoreOption, CC1Option]>, Group<f_Group>,
   HelpText<"Controls the backend parallelism of -flto=thin (default "
            "of 0 means the number of threads will be derived from "
            "the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group<f_Group>,
+  Flags<[CoreOption, CC1Option]>, Group<f_Group>,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
                                 Group<f_Group>, Flags<[DriverOption, CoreOption]>;
 def fmerge_all_constants : Flag<["-"], "fmerge-all-constants">, Group<f_Group>;
 def fmessage_length_EQ : Joined<["-"], "fmessage-length=">, Group<f_Group>;
 def fms_extensions : Flag<["-"], "fms-extensions">, Group<f_Group>, Flags<[CC1Option, CoreOption]>,
   HelpText<"Accept some non-standard constructs supported by the Microsoft compiler">;
 def fms_compatibility : Flag<["-"], "fms-compatibility">, Group<f_Group>, Flags<[CC1Option, CoreOption]>,

test/Driver/cl-lto.c

+// -flto causes a switch to llvm-bc object files.
+// RUN: %clang_cl -ccc-print-phases -c -flto -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-COMPILE-ACTIONS < %t %s
+//
+// CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
+// CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc
+
+// RUN: %clang_cl -ccc-print-phases -flto -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
+//
+// CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}lto.c", c
+// CHECK-COMPILELINK-ACTIONS: 1: preprocessor, {0}, cpp-output
+// CHECK-COMPILELINK-ACTIONS: 2: compiler, {1}, ir
+// CHECK-COMPILELINK-ACTIONS: 3: backend, {2}, lto-bc
+// CHECK-COMPILELINK-ACTIONS: 4: linker, {3}, image
+
+// llvm-bc and llvm-ll outputs need to match regular suffixes
+// (unfortunately).
+// RUN: %clang_cl -### -flto -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-COMPILELINK-SUFFIXES < %t %s
+//
+// CHECK-COMPILELINK-SUFFIXES: "-emit-llvm-bc"
+// CHECK-COMPILELINK-SUFFIXES-SAME: "-o" "{{.*}}cl-lto{{[^ ]*}}.obj"
+// CHECK-COMPILELINK-SUFFIXES-SAME: "-x" "c" "{{.*}}cl-lto.c"
+// CHECK-COMPILELINK-SUFFIXES: "{{.*}}cl-lto.{{(exe)}}" {{.*}}"{{.*}}cl-lto{{[^ ]*}}.obj"

test/Driver/cl-thinlto.c

+// -flto=thin causes a switch to llvm-bc object files.
+// RUN: %clang_cl -ccc-print-phases -c -flto=thin -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-COMPILE-ACTIONS < %t %s
+//
+// CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
+// CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc
+
+// RUN: %clang_cl -ccc-print-phases -flto=thin -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
+//
+// CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}thinlto.c", c
+// CHECK-COMPILELINK-ACTIONS: 1: preprocessor, {0}, cpp-output
+// CHECK-COMPILELINK-ACTIONS: 2: compiler, {1}, ir
+// CHECK-COMPILELINK-ACTIONS: 3: backend, {2}, lto-bc
+// CHECK-COMPILELINK-ACTIONS: 4: linker, {3}, image
+
+// -flto=thin should take precedence over earlier -fno-lto and -flto=full.
+// RUN: %clang_cl -### -fuse-ld=lld -flto=full -fno-lto -flto=thin -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-LINK-THIN-ACTION < %t %s
+//
+// CHECK-LINK-THIN-ACTION: "-cc1"{{.*}}"-flto=thin"
+
+// Check that subsequent -flto=full takes precedence
+// RUN: %clang_cl -### -flto=thin -flto=full -- %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-LINK-FULL-ACTION < %t %s
+//
+// CHECK-LINK-FULL-ACTION: "-cc1"{{.*}}"-flto=full"
+
+// Check that subsequent -fno-lto takes precedence
+// RUN: %clang_cl -### %s -flto=thin -fno-lto 2> %t
+// RUN: FileCheck -check-prefix=CHECK-LINK-NOLTO-ACTION < %t %s
+//
+// CHECK-LINK-NOLTO-ACTION-NOT: "-cc1"{{.*}}"-flto"