Add Clang -ftime-trace support to rules_cc - #772
Conversation
|
i think the failed case is not related with this PR. @fmeum could you take a look? |
|
I don't know why this happens. @trybka |
Declare trace JSON files as compile action outputs and expose them via the trace_files output group, enabled with features = ["trace"] on Clang toolchains.
4d3c124 to
9b9465d
Compare
|
Slowki in bazelbuild/bazel#9047 mentions using an aspect to collect this (as does an internal project), this makes me unsure whether it needs to be in the rules (though it should be @trybka or @pzembrod 's call) |
@lilygorsheneva Thanks for flagging this 1. An aspect would be fragile for C++20 Modules. The current implementation in _create_cc_compile_actions_with_cpp20_module is not a flat "one source → one object" flow. 2. It also pushes a compatibility burden onto users. Bazel 9 moves C++ compilation onto rules_cc's own Starlark internals, so an aspect written for the pre-9 native C++ rules may need additional adaptation for Bazel 9. Implementing this inside rules_cc keeps the compatibility story simpler: as long as the user's rules_cc version is compatible with their Bazel version, trace collection should work normally. With an external aspect, users may need to handle version-specific differences themselves, so it is less likely to work out of the box across Bazel / rules_cc versions. For example, to make bazelbuild/bazel#9047 (comment) work for bazel 9, the patch needed. diff --git a/MODULE.bazel b/MODULE.bazel
new file mode 100644
index 0000000..133dd0f
--- /dev/null
+++ b/MODULE.bazel
@@ -0,0 +1,4 @@
+module(name = "clang_aspects")
+
+bazel_dep(name = "rules_cc", version = "0.2.17")
+bazel_dep(name = "bazel_skylib", version = "1.9.0")
diff --git a/clang_time_trace.bzl b/clang_time_trace.bzl
index b88be59..a6db9e8 100644
--- a/clang_time_trace.bzl
+++ b/clang_time_trace.bzl
@@ -1,6 +1,8 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@rules_cc//cc:action_names.bzl", "ACTION_NAMES")
load("@rules_cc//cc:find_cc_toolchain.bzl", "find_cc_toolchain", "use_cc_toolchain")
+load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
+load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
def _clang_time_trace_aspect_impl(target, ctx):
# Get the compile commandsGiven that |
trybka
left a comment
There was a problem hiding this comment.
I don't understand the Android failures but they certainly seem unrelated. @keith any thoughts?
Re: aspect vs. not
We currently use an aspect for this internally ourselves.
I don't know that we want to go down the path of adding bespoke logic for every compiler/linker feature that emits another file. (One could imagine adding link support for --cref -Map, for instance).
I wonder if there's a better way to define a feature with a generic additional output file that gets fed in with a build variable, then we don't need the rules to handle every permutation, just a magic feature with a specific build variable.
That might require that they have unique names though.
|
|
||
| _VALID_CPP_SOURCE_TYPES = set([CPP_SOURCE_TYPE_SOURCE, CPP_SOURCE_TYPE_HEADER, CPP_SOURCE_TYPE_CLIF_INPUT_PROTO]) | ||
|
|
||
| def _clang_trace_enabled(feature_configuration): |
There was a problem hiding this comment.
is ftime-trace clang specific?
There was a problem hiding this comment.
AFAIK yes, there's a GCC plugin that gives some advanced metrics but I believe it's kinda defunct.
| "lto_compilation_context": {}, | ||
| "gcno_files": [], | ||
| "pic_gcno_files": [], | ||
| "trace_files": [], |
There was a problem hiding this comment.
do we need to track the .pic files separately, or could we bundle them together?
|
android stuff is fixed now so we can ignore |
|
IMO we should support someway for toolchains to add files like this in a generic way. we have had quite a few cases of this overtime and I think it would be nice to not have to add them 1 by 1. this is another case |
Could we have the toolchain provide semi-arbitrary (we'd need to define what context objects we can pass) starlark functions to the rules? |
Declare trace JSON files as compile action outputs and expose them via the trace_files output group, enabled with features = ["trace"] on Clang toolchains.
Fix bazelbuild/bazel#9047