Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The thin_lto feature is not enabled for MacOS #16269

Open
mrkkrp opened this issue Sep 13, 2022 · 5 comments
Open

The thin_lto feature is not enabled for MacOS #16269

mrkkrp opened this issue Sep 13, 2022 · 5 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple team-Rules-CPP Issues for C++ rules type: feature request

Comments

@mrkkrp
Copy link
Contributor

mrkkrp commented Sep 13, 2022

Description of the bug:

In unix_cc_toolchain_config.bzl the thin_lto feature is in the config for Linux but not for MacOS. The result is that thin LTO cannot be used on MacOS with Bazel. AFAIU thin lto is supported on MacOS as well. Is this an oversight or there is a reason for exclusion of this feature on MacOS?

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

No response

Which operating system are you running Bazel on?

MacOS Monterey 12.5.1

What is the output of bazel info release?

release 5.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@keith
Copy link
Member

keith commented Sep 13, 2022

I think this is just an oversight, FWIW as a short term workaround passing --copt=-flto=thin (and similar flags if you are building other languages) should "just work" (maybe you want --linkopt=-flto=thin as well)

@mrkkrp
Copy link
Contributor Author

mrkkrp commented Sep 13, 2022

This was my first attempt. --copt=-flto=thin seems to add -flto=thin to the invocations of compile actions, however --linkopt=-flto=thin has no effect on the linking actions. Is this normal? Are these two flags really enough (there is quite a bit going on is that feature definition)?

@keith
Copy link
Member

keith commented Sep 13, 2022

What failures do you see with that? Technically the linkopt might not matter since if the linker gets lto'd inputs it probably handles them regardless of the flags. I think if we were to add it to the crosstool we would have something that looked a bit more like

thinlto_feature = feature(
name = "thin_lto",
flag_sets = [
flag_set(
actions = [
ACTION_NAMES.c_compile,
ACTION_NAMES.cpp_compile,
] + all_link_actions + lto_index_actions,
flag_groups = [
flag_group(flags = ["-flto=thin"]),
flag_group(
expand_if_available = "lto_indexing_bitcode_file",
flags = [
"-Xclang",
"-fthin-link-bitcode=%{lto_indexing_bitcode_file}",
],
),
],
),
flag_set(
actions = [ACTION_NAMES.linkstamp_compile],
flag_groups = [flag_group(flags = ["-DBUILD_LTO_TYPE=thin"])],
),
flag_set(
actions = lto_index_actions,
flag_groups = [
flag_group(flags = [
"-flto=thin",
"-Wl,-plugin-opt,thinlto-index-only%{thinlto_optional_params_file}",
"-Wl,-plugin-opt,thinlto-emit-imports-files",
"-Wl,-plugin-opt,thinlto-prefix-replace=%{thinlto_prefix_replace}",
]),
flag_group(
expand_if_available = "thinlto_object_suffix_replace",
flags = [
"-Wl,-plugin-opt,thinlto-object-suffix-replace=%{thinlto_object_suffix_replace}",
],
),
flag_group(
expand_if_available = "thinlto_merged_object_file",
flags = [
"-Wl,-plugin-opt,obj-path=%{thinlto_merged_object_file}",
],
),
],
),
flag_set(
actions = [ACTION_NAMES.lto_backend],
flag_groups = [
flag_group(flags = [
"-c",
"-fthinlto-index=%{thinlto_index}",
"-o",
"%{thinlto_output_object_file}",
"-x",
"ir",
"%{thinlto_input_bitcode_file}",
]),
],
),
],
)
, but at least some flags like the linker plugin options are not available with ld64. I don't know the story behind the other flags, but I wouldn't be surprised if they were mostly nice, but not required, optimizations

@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple and removed untriaged labels Oct 26, 2022
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Dec 31, 2023
@fhanau
Copy link

fhanau commented Jan 6, 2024

@bazelbuild/triage

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

5 participants