build: revert builtins PGO logging file changes (#38235)

* build: revert builtins PGO error for release builds

* build: warn instead of aborting on bad profile
This commit is contained in:
Keeley Hammond
2023-05-09 14:50:41 -07:00
committed by GitHub
parent e30b25269d
commit a0e6ca8dab
2 changed files with 72 additions and 0 deletions

View File

@@ -8,3 +8,4 @@ fix_build_deprecated_attribute_for_older_msvc_versions.patch
fix_disable_implies_dcheck_for_node_stream_array_buffers.patch
force_cppheapcreateparams_to_be_noncopyable.patch
chore_allow_customizing_microtask_policy_per_context.patch
build_revert_builtins_pgo.patch

View File

@@ -0,0 +1,71 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Keeley Hammond <vertedinde@electronjs.org>
Date: Tue, 9 May 2023 11:44:16 -0700
Subject: build: revert "Reland "[builtins-pgo] Enable builtins PGO for Windows
and gcc builds""
This reverts commit be04b76419b55d47a696d2c9417baa0ab7d6462b.
This commit causes build failures on release builds across platforms. This patch
can be removed when 1) the issue is resolved with Chromium upstream or
2) when Electron disables builtins PGO.
diff --git a/BUILD.gn b/BUILD.gn
index 2dc5737260d544460e00b037fe62d3ecf846d46d..7434814cc3ec451a33106a8aec9648427f7d3bbb 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -578,30 +578,20 @@ if (v8_builtins_profiling_log_file == "default") {
# are accessed,
# * v8_enable_webassembly because it changes the set of opcodes which affects
# graphs hashes,
+ # * !is_clang because it might affect argument evaluation order, which
+ # makes node IDs not predictable for subgraphs like Op1(Op2(), Op3()) and
+ # as a result different graph hash.
if (v8_enable_builtins_optimization && !v8_enable_builtins_profiling &&
- !is_debug && !dcheck_always_on && v8_enable_webassembly) {
- # This is about function arguments evaluation order, which makes node IDs
- # not predictable for subgraphs like Op1(Op2(), Op3()) and as a result
- # different graph hashes.
- # Clang uses left-to-right order everywhere except Windows, otherwise the
- # order is right-to-left.
- # TODO(crbug.com/v8/13647): Remove once this issue is fixed in CSA.
- if (!is_clang || is_win) {
- pgo_profile_suffix = "-rl"
- } else {
- pgo_profile_suffix = ""
- }
+ is_clang && !is_debug && !dcheck_always_on && v8_enable_webassembly) {
if ((v8_current_cpu == "x64" || v8_current_cpu == "arm64") &&
v8_enable_pointer_compression && v8_enable_external_code_space &&
v8_enable_sandbox) {
# Note, currently x64 profile can be applied to arm64 but not the other
# way round.
- v8_builtins_profiling_log_file =
- "tools/builtins-pgo/profiles/x64" + pgo_profile_suffix + ".profile"
+ v8_builtins_profiling_log_file = "tools/builtins-pgo/profiles/x64.profile"
} else if (v8_current_cpu == "x86" || v8_current_cpu == "arm") {
# Note, x86 profile can be applied to arm but not the other way round.
- v8_builtins_profiling_log_file =
- "tools/builtins-pgo/profiles/x86" + pgo_profile_suffix + ".profile"
+ v8_builtins_profiling_log_file = "tools/builtins-pgo/profiles/x86.profile"
}
}
}
@@ -2266,11 +2256,13 @@ template("run_mksnapshot") {
args += [
"--turbo-profiling-input",
rebase_path(v8_builtins_profiling_log_file, root_build_dir),
-
- # Replace this with --warn-about-builtin-profile-data to see the full
- # list of builtins with incompatible profiles.
- "--abort-on-bad-builtin-profile-data",
]
+
+ # Replace this with --warn-about-builtin-profile-data to see the full
+ # list of builtins with incompatible profiles.
+ # TODO(crbug.com/v8/13647): Do not fail for invalid profiles
+ # args += [ "--abort-on-bad-builtin-profile-data" ]
+ args += [ "--warn-about-builtin-profile-data" ]
}
# This is needed to distinguish between generating code for the simulator