From 568bb7b70f32d52e6626c313fc1d3dc18a0757af Mon Sep 17 00:00:00 2001 From: David Havas Date: Fri, 3 Jan 2025 00:30:33 +0100 Subject: [PATCH] Make lto toolchain options public and don't link proc-macros with LTO (#3147) I would like to make lto setting public in toolchain so that when we are providing custom toolchains it would be easier to make configurations for LTO with `select` statements. The logic which determines the flags based on cargo outputs. Proc-macro crates can not be linked with lto and we should not emit bitcode either. This fixes #3143 --------- Co-authored-by: Daniel Wagner-Hall --- docs/src/rust_toolchains.md | 6 ++++-- rust/private/lto.bzl | 10 ++++++---- rust/toolchain.bzl | 13 +++++++------ test/unit/lto/lto_test_suite.bzl | 22 +++++++++++++++++++++- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/docs/src/rust_toolchains.md b/docs/src/rust_toolchains.md index 368541ba73..0c40d2967d 100644 --- a/docs/src/rust_toolchains.md +++ b/docs/src/rust_toolchains.md @@ -46,8 +46,9 @@ rust_toolchain(name, default_edition, dylib_ext, env, exec_triple, experimental_link_std_dylib, experimental_use_cc_common_link, extra_exec_rustc_flags, extra_rustc_flags, extra_rustc_flags_for_crate_types, global_allocator_library, llvm_cov, llvm_profdata, - llvm_tools, opt_level, per_crate_rustc_flags, rust_doc, rust_std, rustc, rustc_lib, - rustfmt, staticlib_ext, stdlib_linkflags, strip_level, target_json, target_triple) + llvm_tools, lto, opt_level, per_crate_rustc_flags, rust_doc, rust_std, rustc, + rustc_lib, rustfmt, staticlib_ext, stdlib_linkflags, strip_level, target_json, + target_triple) Declares a Rust toolchain for use. @@ -118,6 +119,7 @@ See `@rules_rust//rust:repositories.bzl` for examples of defining the `@rust_cpu | llvm_cov | The location of the `llvm-cov` binary. Can be a direct source or a filegroup containing one item. If None, rust code is not instrumented for coverage. | Label | optional | `None` | | llvm_profdata | The location of the `llvm-profdata` binary. Can be a direct source or a filegroup containing one item. If `llvm_cov` is None, this can be None as well and rust code is not instrumented for coverage. | Label | optional | `None` | | llvm_tools | LLVM tools that are shipped with the Rust toolchain. | Label | optional | `None` | +| lto | Label to an LTO setting whether which can enable custom LTO settings | Label | optional | `"@rules_rust//rust/settings:lto"` | | opt_level | Rustc optimization levels. | Dictionary: String -> String | optional | `{"dbg": "0", "fastbuild": "0", "opt": "3"}` | | per_crate_rustc_flags | Extra flags to pass to rustc in non-exec configuration | List of strings | optional | `[]` | | rust_doc | The location of the `rustdoc` binary. Can be a direct source or a filegroup containing one item. | Label | required | | diff --git a/rust/private/lto.bzl b/rust/private/lto.bzl index 04c271750d..18fe307515 100644 --- a/rust/private/lto.bzl +++ b/rust/private/lto.bzl @@ -60,7 +60,7 @@ def _determine_lto_object_format(ctx, toolchain, crate_info): if is_exec_configuration(ctx): return "only_object" - mode = toolchain._lto.mode + mode = toolchain.lto.mode if mode in ["off", "unspecified"]: return "only_object" @@ -75,9 +75,10 @@ def _determine_lto_object_format(ctx, toolchain, crate_info): # If we're building an 'rlib' and LTO is enabled, then we can skip # generating object files entirely. return "only_bitcode" - elif crate_info.type == "dylib": + elif crate_info.type in ["dylib", "proc-macro"]: # If we're a dylib and we're running LTO, then only emit object code # because 'rustc' doesn't currently support LTO with dylibs. + # proc-macros do not benefit from LTO, and cannot be dynamically linked with LTO. return "only_object" else: return "object_and_bitcode" @@ -93,7 +94,7 @@ def construct_lto_arguments(ctx, toolchain, crate_info): Returns: list: A list of strings that are valid flags for 'rustc'. """ - mode = toolchain._lto.mode + mode = toolchain.lto.mode # The user is handling LTO on their own, don't add any arguments. if mode == "manual": @@ -102,7 +103,8 @@ def construct_lto_arguments(ctx, toolchain, crate_info): format = _determine_lto_object_format(ctx, toolchain, crate_info) args = [] - if mode in ["thin", "fat", "off"] and not is_exec_configuration(ctx): + # proc-macros do not benefit from LTO, and cannot be dynamically linked with LTO. + if mode in ["thin", "fat", "off"] and not is_exec_configuration(ctx) and crate_info.type != "proc-macro": args.append("lto={}".format(mode)) if format == "object_and_bitcode": diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index 7bd0dbad5d..97d9e07f4c 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -521,7 +521,7 @@ def _rust_toolchain_impl(ctx): third_party_dir = ctx.attr._third_party_dir[BuildSettingInfo].value pipelined_compilation = ctx.attr._pipelined_compilation[BuildSettingInfo].value no_std = ctx.attr._no_std[BuildSettingInfo].value - lto = ctx.attr._lto[RustLtoInfo] + lto = ctx.attr.lto[RustLtoInfo] experimental_use_global_allocator = ctx.attr._experimental_use_global_allocator[BuildSettingInfo].value if _experimental_use_cc_common_link(ctx): @@ -672,6 +672,7 @@ def _rust_toolchain_impl(ctx): nostd_and_global_allocator_cc_info = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc"), llvm_cov = ctx.file.llvm_cov, llvm_profdata = ctx.file.llvm_profdata, + lto = lto, make_variables = make_variable_info, rust_doc = sysroot.rustdoc, rust_std = sysroot.rust_std, @@ -705,7 +706,6 @@ def _rust_toolchain_impl(ctx): _toolchain_generated_sysroot = ctx.attr._toolchain_generated_sysroot[BuildSettingInfo].value, _incompatible_do_not_include_data_in_compile_data = ctx.attr._incompatible_do_not_include_data_in_compile_data[IncompatibleFlagInfo].enabled, _no_std = no_std, - _lto = lto, _codegen_units = ctx.attr._codegen_units[BuildSettingInfo].value, ) return [ @@ -808,6 +808,11 @@ rust_toolchain = rule( doc = "LLVM tools that are shipped with the Rust toolchain.", allow_files = True, ), + "lto": attr.label( + providers = [RustLtoInfo], + default = Label("//rust/settings:lto"), + doc = "Label to an LTO setting whether which can enable custom LTO settings", + ), "opt_level": attr.string_dict( doc = "Rustc optimization levels.", default = { @@ -897,10 +902,6 @@ rust_toolchain = rule( default = Label("//rust/settings:incompatible_do_not_include_data_in_compile_data"), doc = "Label to a boolean build setting that controls whether to include data files in compile_data.", ), - "_lto": attr.label( - providers = [RustLtoInfo], - default = Label("//rust/settings:lto"), - ), "_no_std": attr.label( default = Label("//rust/settings:no_std"), ), diff --git a/test/unit/lto/lto_test_suite.bzl b/test/unit/lto/lto_test_suite.bzl index 46c0b3172f..8ccf47da18 100644 --- a/test/unit/lto/lto_test_suite.bzl +++ b/test/unit/lto/lto_test_suite.bzl @@ -2,7 +2,7 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest") load("@bazel_skylib//rules:write_file.bzl", "write_file") -load("//rust:defs.bzl", "rust_library") +load("//rust:defs.bzl", "rust_library", "rust_proc_macro") load( "//test/unit:common.bzl", "assert_action_mnemonic", @@ -78,6 +78,14 @@ _lto_level_fat_test = analysistest.make( config_settings = {str(Label("//rust/settings:lto")): "fat"}, ) +def _lto_proc_macro(ctx): + return _lto_test_impl(ctx, None, "no", False) + +_lto_proc_macro_test = analysistest.make( + _lto_proc_macro, + config_settings = {str(Label("//rust/settings:lto")): "thin"}, +) + def lto_test_suite(name): """Entry-point macro called from the BUILD file. @@ -100,6 +108,12 @@ def lto_test_suite(name): edition = "2021", ) + rust_proc_macro( + name = "proc_macro", + srcs = [":lib.rs"], + edition = "2021", + ) + _lto_level_default_test( name = "lto_level_default_test", target_under_test = ":lib", @@ -125,6 +139,11 @@ def lto_test_suite(name): target_under_test = ":lib", ) + _lto_proc_macro_test( + name = "lto_proc_macro_test", + target_under_test = ":proc_macro", + ) + native.test_suite( name = name, tests = [ @@ -133,5 +152,6 @@ def lto_test_suite(name): ":lto_level_off_test", ":lto_level_thin_test", ":lto_level_fat_test", + ":lto_proc_macro_test", ], )