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.

Sign up for GitHub

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

Jump to bottom

Track unix_cc_toolchain_config from bazel's tools/cpp #23

Closed
mattgodbolt opened this issue Oct 1, 2019 · 4 comments · Fixed by #75
Closed

Track unix_cc_toolchain_config from bazel's tools/cpp #23

mattgodbolt opened this issue Oct 1, 2019 · 4 comments · Fixed by #75

Comments

@mattgodbolt
Copy link

I may be missing something but other toolchains seem to support thin_lto or other forms of LTO (https://github.com/tensorflow/tensorflow/blob/1843f39c11e42751837c62acfb74e1fe697353ef/third_party/toolchains/clang6/CROSSTOOL.tpl#L402 for example) -- it would be lovely to support that here. Or perhaps I'm mistaken: can someone suggest how I might enable LTO/pgo in general?
@siddharthab
Copy link
Contributor

In general, this project is now lagging behind https://source.bazel.build/bazel/+/master:tools/cpp/unix_cc_toolchain_config.bzl.

Maybe I should also try to just use that rule directly without copy pasting code from there.

I am going to repurpose this issue towards that.

@siddharthab siddharthab changed the title thin_lto feature support Track unix_cc_toolchain_config from bazel's tools/cpp Oct 3, 2019
@mattgodbolt
Copy link
Author

Thanks @siddharthab !

@siddharthab
Copy link
Contributor

ThinLTO has now made it to @bazel_tools//tools/cpp:unix_cc_toolchain_config.bzl in bazel 1.0. So we can now refactor our code to depend on the rule there for this as well as other features.

@Monnoroch
Copy link

The standard @bazel_tools//tools/cpp:unix_cc_toolchain_config.bzl also supports more platforms, such as ARM (Android). There's an LLVM distribution for armv7a, yet this toolchain doesn't support it purely because of the lack of config, which already exists in Bazel. Would be great to reuse the main config and get all that!

rrbutani added a commit to rrbutani/bazel-toolchain that referenced this issue Feb 9, 2021
@rrbutani
llvm_toolchain: first pass at using cc_toolchain_config from @bazel…
8b4f72e 
…_tools

(See bazel-contrib#23).

Since `@bazel_tools//tools/cpp:unix_cc_toolchain_config.bzl:cc_toolchain_config` is a rule, our `cc_toolchain_config`
needs to become a macro instead of a rule. Luckily we actually can do this without introducing any breaking changes.

Other than the discrepancies listed within (the things marked with `## NOTE:` — make vars and framework paths)
everything seemed to just fall into place. So far it seems to _just work_ 🤞.
@rrbutani rrbutani mentioned this issue Feb 9, 2021
Merged
rrbutani added a commit to rrbutani/bazel-toolchain that referenced this issue Aug 15, 2021
@rrbutani
llvm_toolchain: first pass at using cc_toolchain_config from @bazel…
050af4b 
…_tools

(See bazel-contrib#23).

Since `@bazel_tools//tools/cpp:unix_cc_toolchain_config.bzl:cc_toolchain_config` is a rule, our `cc_toolchain_config`
needs to become a macro instead of a rule. Luckily we actually can do this without introducing any breaking changes.

Other than the discrepancies listed within (the things marked with `## NOTE:` — make vars and framework paths)
everything seemed to just fall into place. So far it seems to _just work_ 🤞.
siddharthab pushed a commit that referenced this issue Sep 21, 2021
@rrbutani
Use unix_cc_toolchain_config from @bazel_tools (#75)
b20fe21 
Closes #23.

Notes:
1. `cc_toolchain_config` is now a macro.
2. [Make variables](https://github.com/grailbio/bazel-toolchain/blob/f2d1ba2c9d713b2aa6e7063f6d11dd3d64aa402a/toolchain/cc_toolchain_config.bzl.tpl#L527-L537) and [Framework paths](https://github.com/grailbio/bazel-toolchain/blob/f2d1ba2c9d713b2aa6e7063f6d11dd3d64aa402a/toolchain/cc_toolchain_config.bzl.tpl#L347-L365) ([only used on macOS](https://github.com/grailbio/bazel-toolchain/blob/f2d1ba2c9d713b2aa6e7063f6d11dd3d64aa402a/toolchain/cc_toolchain_config.bzl.tpl#L498-L499)) are not yet supported; see in-code comments (beginning ## NOTE:).
3. `llvm-strip` is now used for llvm versions ≥7 as the strip binary.
4. Contains fixes for using/running the toolchain from other locations in the execroot. Most notably, `rules_foreign_cc` will use a different PWD than the execroot root for cmake targets which breaks the wrapper script. This commit has the wrapper script search alongside itself for `clang` when it isn't in the usual places.
5. Modern versions of ld.lld support start and end groups but we don't use ld.lld on macOS (yet), so don't use this feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use unix_cc_toolchain_config.bzl:cc_toolchain_config from @bazel_tools. rrbutani/bazel-toolchain
3 participants
@mattgodbolt @Monnoroch @siddharthab