bazelbuild / bazelbuild/bazel

CppLink actions having access to fdo_profile_path variable

Open
#17,277 0 comments 0 reactions 0 assignees View on GitHub
P2 team-Rules-CPP type: feature request
Dominant language
Java
Stars
25.8k
Forks
4.6k
Avg merge
2d 20h
Merged PRs (30d)
72

Description

### Description of the feature request:

C++ rules explicitly pipes `//command_line_option:fdo_profile` to `fdo_profile_path` build variable for [CppCompileAction](https://github.com/bazelbuild/bazel/blob/8e41dce65b97a3d466d6b1e65005abc52a07b90b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java#L128). `fdo_profile_path` build variable is then used in several features in cc toolchain. [autofdo](https://github.com/bazelbuild/bazel/blob/master/tools/cpp/unix_cc_toolchain_config.bzl#L497) is one of the using features.

`autofdo` is [implicitly enabled](https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java#L1022) and expands some fdo-specific flags on c/c++ compile actions when `fdo_profile_path` is available. This works well in making sure we're able to expand the same set of flags in Bazel as in Soong for [CFlags](https://cs.android.com/android/platform/build/soong/+/master:cc/afdo.go;l=119).

In AOSP, Soong also sets some [fdo-specific flags](https://cs.android.com/android/platform/build/soong/+/master:cc/afdo.go;l=120;drc=7a8362c252b152f806fc303c414ff1418672b067) to `LdFlags`. AOSP has its own Bazel cc toolchain and we can modify our custom [`autofdo` feature](https://cs.android.com/android/platform/prebuilts/clang/host/linux-x86/+/master:cc_toolchain_features.bzl;l=917;drc=ac41525cbeb88c7e56c230e7bc1f0dffa8233ede) to expand the flags on the link actions as well. However, Bazel Cc++ rules doesn't not pipe the `fdo_profile_path` variable for `CppLinkAction` yet. Hence, the dynamic fdo-specific flags don't have access to `fdo_profile_path` build variable to expand the flags correctly.

**It would be better if there's some mechanism to pipe build variables for C++ toolchain to use for this specific use case.**

### What underlying problem are you trying to solve with this feature?

There is a workaround to this problem. To set the flags on link actions, we can use `user_link_flags` and `additional_linker_inputs` attributes such as

```
filegroup(
name = "libart_fdo",
srcs = select({
"...": ["libart_arm64.afdo"],
"...": ["libart_arm.afdo"],
})
)
cc_shared_library(
name = "foo",
user_link_flags = ["-fprofile-sample-use=$(location :libart_fdo)"],
additional_linker_inputs = [":libart_fdo"],
)
```

But this is counter-intuitive to the existence of `fdo_profile` rule where we use `fdo_profile` rule to set the compile flags correctly and use ^ to set the link flags correctly. Another problem is that this approach results to duplicate profiles. `fdo_profile` symlinks the profile to a separate directory which is referenced in the compile flags. However, the link flags reference a different location though they should be the same.

Contributor guide

Open the contributing guide

Research direction

Read CcCompilationHelper.java and CcCommon.java to trace how fdo_profile_path reaches CppCompileAction, then compare that path with CppLinkAction. Review unix_cc_toolchain_config.bzl and the linked AOSP toolchain feature; done means link actions can expand the same profile path without the user_link_flags workaround or duplicate profile locations.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
build-system
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.