Skip to content

V
V8
Commit 3e9f46e8 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ
Browse files
Options

Revert "[flags] Rename v8_flags to FLAGS" 

This reverts commit d84b4664.

Reason for revert: Fails "Mutable Constants" check on android-binary-size: https://ci.chromium.org/ui/p/chromium/builders/try/android-binary-size/1211670/overview

For details about this check, see https://chromium.googlesource.com/chromium/src/+/main/docs/speed/binary_size/android_binary_size_trybot.md#Mutable-Constants

Original change's description:
> [flags] Rename v8_flags to FLAGS
>
> Team members expressed concerns that "v8_flags" is easier to miss in the
> code than the previous "FLAG_" syntax. After a poll and discussions we
> decided to rename the struct to "FLAGS", so the new syntax for
> addressing flag values is "FLAGS.foo" instead of the previous
> "FLAG_foo".
>
> R=​cbruni@chromium.org
> CC=​jkummerow@chromium.org
>
> Bug: v8:12887
> Change-Id: I51af4aa7fd5a3b3c29310c0cb4c4ff42086ff012
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3854508
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82701}

Bug: v8:12887
Change-Id: I75516a0be9bc475afa2bbaa96a05e8a9b5be9be7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3855936
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82711}
parent 59d7cf52
Hide whitespace changes
Inline Side-by-side
Showing with 34 additions and 38 deletions
src/flags/flag-definitions.h
View file @ 3e9f46e8
......@@ -36,15 +36,15 @@
#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \
static constexpr FlagValue<ctype> nam{def};
// Define a {FLAG_foo} alias per flag, pointing to {FLAGS.foo}.
// Define a {FLAG_foo} alias per flag, pointing to {v8_flags.foo}.
// This allows to still use the old and deprecated syntax for accessing flag
// values. This will be removed after v10.7.
// TODO(clemensb): Remove this after v10.7.
#elif defined(FLAG_MODE_DEFINE_GLOBAL_ALIASES)
#define FLAG_FULL(ftype, ctype, nam, def, cmt) \
inline auto& FLAG_##nam = FLAGS.nam;
inline auto& FLAG_##nam = v8_flags.nam;
#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \
inline auto constexpr& FLAG_##nam = FLAGS.nam;
inline auto constexpr& FLAG_##nam = v8_flags.nam;
// We need to define all of our default values so that the Flag structure can
// access them by pointer. These are just used internally inside of one .cc,
......@@ -57,29 +57,29 @@
// printing / etc in the flag parser code. We only do this for writable flags.
#elif defined(FLAG_MODE_META)
#define FLAG_FULL(ftype, ctype, nam, def, cmt) \
{Flag::TYPE_##ftype, #nam, &FLAGS.nam, &FLAGDEFAULT_##nam, cmt, false},
#define FLAG_ALIAS(ftype, ctype, alias, nam) \
{Flag::TYPE_##ftype, #alias, &FLAGS.nam, &FLAGDEFAULT_##nam, \
{Flag::TYPE_##ftype, #nam, &v8_flags.nam, &FLAGDEFAULT_##nam, cmt, false},
#define FLAG_ALIAS(ftype, ctype, alias, nam) \
{Flag::TYPE_##ftype, #alias, &v8_flags.nam, &FLAGDEFAULT_##nam, \
"alias for --" #nam, false}, // NOLINT(whitespace/indent)
// We produce the code to set flags when it is implied by another flag.
#elif defined(FLAG_MODE_DEFINE_IMPLICATIONS)
#define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(FLAGS.whenflag, #whenflag, &FLAGS.thenflag, \
value, false);
#define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \
&v8_flags.thenflag, value, false);
// A weak implication will be overwritten by a normal implication or by an
// explicit flag.
#define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(FLAGS.whenflag, #whenflag, &FLAGS.thenflag, \
value, true);
#define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \
&v8_flags.thenflag, value, true);
#define DEFINE_GENERIC_IMPLICATION(whenflag, statement) \
if (FLAGS.whenflag) statement;
if (v8_flags.whenflag) statement;
#define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(!FLAGS.whenflag, "!" #whenflag, \
&FLAGS.thenflag, value, false);
#define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(!v8_flags.whenflag, "!" #whenflag, \
&v8_flags.thenflag, value, false);
// We apply a generic macro to the flags.
#elif defined(FLAG_MODE_APPLY)
......
src/flags/flags.cc
View file @ 3e9f46e8
......@@ -30,12 +30,12 @@
namespace v8::internal {
// Define {FLAGS}, declared in flags.h.
FlagValues FLAGS;
// Define {v8_flags}, declared in flags.h.
FlagValues v8_flags;
// {FLAGS} needs to be aligned to a memory page, and the size needs to be a
// {v8_flags} needs to be aligned to a memory page, and the size needs to be a
// multiple of a page size. This is required for memory-protection of the memory
// holding the {FLAGS} struct.
// holding the {v8_flags} struct.
// Both is guaranteed by the {alignas(kMinimumOSPageSize)} annotation on
// {FlagValues}.
static_assert(alignof(FlagValues) == kMinimumOSPageSize);
......@@ -205,14 +205,14 @@ struct Flag {
}
static bool ShouldCheckFlagContradictions() {
if (FLAGS.allow_overwriting_for_next_flag) {
if (v8_flags.allow_overwriting_for_next_flag) {
// Setting the flag manually to false before calling Reset() avoids this
// becoming re-entrant.
FLAGS.allow_overwriting_for_next_flag = false;
FindFlagByPointer(&FLAGS.allow_overwriting_for_next_flag)->Reset();
v8_flags.allow_overwriting_for_next_flag = false;
FindFlagByPointer(&v8_flags.allow_overwriting_for_next_flag)->Reset();
return false;
}
return FLAGS.abort_on_contradictory_flags && !FLAGS.fuzzing;
return v8_flags.abort_on_contradictory_flags && !v8_flags.fuzzing;
}
// {change_flag} indicates if we're going to change the flag value.
......@@ -267,7 +267,7 @@ struct Flag {
case SetBy::kCommandLine:
if (new_set_by == SetBy::kImplication && check_command_line_flags) {
// Exit instead of abort for certain testing situations.
if (FLAGS.exit_on_contradictory_flags) base::OS::ExitProcess(0);
if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0);
if (is_bool_flag) {
FatalError{} << "Flag " << FlagName{name()}
<< ": value implied by " << FlagName{implied_by}
......@@ -280,7 +280,7 @@ struct Flag {
} else if (new_set_by == SetBy::kCommandLine &&
check_command_line_flags) {
// Exit instead of abort for certain testing situations.
if (FLAGS.exit_on_contradictory_flags) base::OS::ExitProcess(0);
if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0);
if (is_bool_flag) {
FatalError{} << "Command-line provided flag " << FlagName{name()}
<< " specified as both true and false";
......@@ -493,9 +493,9 @@ uint32_t ComputeFlagListHash() {
if (flag.IsDefault()) continue;
// We want to be able to flip --profile-deserialization without
// causing the code cache to get invalidated by this hash.
if (flag.PointsTo(&FLAGS.profile_deserialization)) continue;
// Skip FLAGS.random_seed to allow predictable code caching.
if (flag.PointsTo(&FLAGS.random_seed)) continue;
if (flag.PointsTo(&v8_flags.profile_deserialization)) continue;
// Skip v8_flags.random_seed to allow predictable code caching.
if (flag.PointsTo(&v8_flags.random_seed)) continue;
modified_args_as_string << flag;
}
std::string args(modified_args_as_string.str());
......@@ -694,7 +694,7 @@ int FlagList::SetFlagsFromCommandLine(int* argc, char** argv, bool remove_flags,
}
}
if (FLAGS.help) {
if (v8_flags.help) {
if (help_options.HasUsage()) {
PrintF(stdout, "%s", help_options.usage());
}
......@@ -770,7 +770,7 @@ int FlagList::SetFlagsFromString(const char* str, size_t len) {
// static
void FlagList::FreezeFlags() {
flags_frozen.store(true, std::memory_order_relaxed);
base::OS::SetDataReadOnly(&FLAGS, sizeof(FLAGS));
base::OS::SetDataReadOnly(&v8_flags, sizeof(v8_flags));
}
// static
......
src/flags/flags.h
View file @ 3e9f46e8
......@@ -11,14 +11,14 @@
#if V8_ENABLE_WEBASSEMBLY
// Include the wasm-limits.h header for some default values of Wasm flags.
// This can be reverted once we can use designated initializations (C++20) for
// {FLAGS} (defined in flags.cc) instead of specifying the default values in
// {v8_flags} (defined in flags.cc) instead of specifying the default values in
// the header and using the default constructor.
#include "src/wasm/wasm-limits.h"
#endif
namespace v8::internal {
// The value of a single flag (this is the type of all FLAGS.* fields).
// The value of a single flag (this is the type of all v8_flags.* fields).
template <typename T>
class FlagValue {
public:
......@@ -52,11 +52,7 @@ struct alignas(kMinimumOSPageSize) FlagValues {
#include "src/flags/flag-definitions.h" // NOLINT(build/include)
};
// The name "FLAGS" does not follow the naming convention for global objects,
// but was preferred by the majority of engineers in a poll in August 2022.
// It's similar to the old "FLAG_foo" syntax, and the capital letters make it
// easier to spot code paths that depend on global flags.
V8_EXPORT_PRIVATE extern FlagValues FLAGS;
V8_EXPORT_PRIVATE extern FlagValues v8_flags;
// TODO(clemensb): Remove this after v10.7.
#define FLAG_MODE_DEFINE_GLOBAL_ALIASES
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment