AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[flags] Rename v8_flags to FLAGS

Browse Source 
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}
This commit is contained in:
Clemens Backes 2022-08-24 16:21:43 +02:00 committed by V8 LUCI CQ
parent 0755c9b631
commit d84b4664fa
3 changed files with 38 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/flags/flag-definitions.h
View File

@ -36,15 +36,15 @@
#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \ #define FLAG_READONLY(ftype, ctype, nam, def, cmt) \
static constexpr FlagValue<ctype> nam{def}; static constexpr FlagValue<ctype> nam{def};
// Define a {FLAG_foo} alias per flag, pointing to {v8_flags.foo}. // Define a {FLAG_foo} alias per flag, pointing to {FLAGS.foo}.
// This allows to still use the old and deprecated syntax for accessing flag // This allows to still use the old and deprecated syntax for accessing flag
// values. This will be removed after v10.7. // values. This will be removed after v10.7.
// TODO(clemensb): Remove this after v10.7. // TODO(clemensb): Remove this after v10.7.
#elif defined(FLAG_MODE_DEFINE_GLOBAL_ALIASES) #elif defined(FLAG_MODE_DEFINE_GLOBAL_ALIASES)
#define FLAG_FULL(ftype, ctype, nam, def, cmt) \ #define FLAG_FULL(ftype, ctype, nam, def, cmt) \
inline auto& FLAG_##nam = v8_flags.nam; inline auto& FLAG_##nam = FLAGS.nam;
#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \ #define FLAG_READONLY(ftype, ctype, nam, def, cmt) \
inline auto constexpr& FLAG_##nam = v8_flags.nam; inline auto constexpr& FLAG_##nam = FLAGS.nam;
// We need to define all of our default values so that the Flag structure can // 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, // 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. // printing / etc in the flag parser code. We only do this for writable flags.
#elif defined(FLAG_MODE_META) #elif defined(FLAG_MODE_META)
#define FLAG_FULL(ftype, ctype, nam, def, cmt) \ #define FLAG_FULL(ftype, ctype, nam, def, cmt) \
{Flag::TYPE_##ftype, #nam, &v8_flags.nam, &FLAGDEFAULT_##nam, cmt, false}, {Flag::TYPE_##ftype, #nam, &FLAGS.nam, &FLAGDEFAULT_##nam, cmt, false},
#define FLAG_ALIAS(ftype, ctype, alias, nam) \ #define FLAG_ALIAS(ftype, ctype, alias, nam) \
{Flag::TYPE_##ftype, #alias, &v8_flags.nam, &FLAGDEFAULT_##nam, \ {Flag::TYPE_##ftype, #alias, &FLAGS.nam, &FLAGDEFAULT_##nam, \
"alias for --" #nam, false}, // NOLINT(whitespace/indent) "alias for --" #nam, false}, // NOLINT(whitespace/indent)
// We produce the code to set flags when it is implied by another flag. // We produce the code to set flags when it is implied by another flag.
#elif defined(FLAG_MODE_DEFINE_IMPLICATIONS) #elif defined(FLAG_MODE_DEFINE_IMPLICATIONS)
#define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value) \ #define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \ changed |= TriggerImplication(FLAGS.whenflag, #whenflag, &FLAGS.thenflag, \
&v8_flags.thenflag, value, false); value, false);
// A weak implication will be overwritten by a normal implication or by an // A weak implication will be overwritten by a normal implication or by an
// explicit flag. // explicit flag.
#define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value) \ #define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \ changed |= TriggerImplication(FLAGS.whenflag, #whenflag, &FLAGS.thenflag, \
&v8_flags.thenflag, value, true); value, true);
#define DEFINE_GENERIC_IMPLICATION(whenflag, statement) \ #define DEFINE_GENERIC_IMPLICATION(whenflag, statement) \
if (v8_flags.whenflag) statement; if (FLAGS.whenflag) statement;
#define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \ #define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(!v8_flags.whenflag, "!" #whenflag, \ changed |= TriggerImplication(!FLAGS.whenflag, "!" #whenflag, \
&v8_flags.thenflag, value, false); &FLAGS.thenflag, value, false);
// We apply a generic macro to the flags. // We apply a generic macro to the flags.
#elif defined(FLAG_MODE_APPLY) #elif defined(FLAG_MODE_APPLY)

30
src/flags/flags.cc
View File

@ -30,12 +30,12 @@
namespace v8::internal { namespace v8::internal {
// Define {v8_flags}, declared in flags.h. // Define {FLAGS}, declared in flags.h.
FlagValues v8_flags; FlagValues FLAGS;
// {v8_flags} needs to be aligned to a memory page, and the size needs to be a // {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 // multiple of a page size. This is required for memory-protection of the memory
// holding the {v8_flags} struct. // holding the {FLAGS} struct.
// Both is guaranteed by the {alignas(kMinimumOSPageSize)} annotation on // Both is guaranteed by the {alignas(kMinimumOSPageSize)} annotation on
// {FlagValues}. // {FlagValues}.
static_assert(alignof(FlagValues) == kMinimumOSPageSize); static_assert(alignof(FlagValues) == kMinimumOSPageSize);
@ -205,14 +205,14 @@ struct Flag {
} }
static bool ShouldCheckFlagContradictions() { static bool ShouldCheckFlagContradictions() {
if (v8_flags.allow_overwriting_for_next_flag) { if (FLAGS.allow_overwriting_for_next_flag) {
// Setting the flag manually to false before calling Reset() avoids this // Setting the flag manually to false before calling Reset() avoids this
// becoming re-entrant. // becoming re-entrant.
v8_flags.allow_overwriting_for_next_flag = false; FLAGS.allow_overwriting_for_next_flag = false;
FindFlagByPointer(&v8_flags.allow_overwriting_for_next_flag)->Reset(); FindFlagByPointer(&FLAGS.allow_overwriting_for_next_flag)->Reset();
return false; return false;
} }
return v8_flags.abort_on_contradictory_flags && !v8_flags.fuzzing; return FLAGS.abort_on_contradictory_flags && !FLAGS.fuzzing;
} }
// {change_flag} indicates if we're going to change the flag value. // {change_flag} indicates if we're going to change the flag value.
@ -267,7 +267,7 @@ struct Flag {
case SetBy::kCommandLine: case SetBy::kCommandLine:
if (new_set_by == SetBy::kImplication && check_command_line_flags) { if (new_set_by == SetBy::kImplication && check_command_line_flags) {
// Exit instead of abort for certain testing situations. // Exit instead of abort for certain testing situations.
if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0); if (FLAGS.exit_on_contradictory_flags) base::OS::ExitProcess(0);
if (is_bool_flag) { if (is_bool_flag) {
FatalError{} << "Flag " << FlagName{name()} FatalError{} << "Flag " << FlagName{name()}
<< ": value implied by " << FlagName{implied_by} << ": value implied by " << FlagName{implied_by}
@ -280,7 +280,7 @@ struct Flag {
} else if (new_set_by == SetBy::kCommandLine && } else if (new_set_by == SetBy::kCommandLine &&
check_command_line_flags) { check_command_line_flags) {
// Exit instead of abort for certain testing situations. // Exit instead of abort for certain testing situations.
if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0); if (FLAGS.exit_on_contradictory_flags) base::OS::ExitProcess(0);
if (is_bool_flag) { if (is_bool_flag) {
FatalError{} << "Command-line provided flag " << FlagName{name()} FatalError{} << "Command-line provided flag " << FlagName{name()}
<< " specified as both true and false"; << " specified as both true and false";
@ -493,9 +493,9 @@ uint32_t ComputeFlagListHash() {
if (flag.IsDefault()) continue; if (flag.IsDefault()) continue;
// We want to be able to flip --profile-deserialization without // We want to be able to flip --profile-deserialization without
// causing the code cache to get invalidated by this hash. // causing the code cache to get invalidated by this hash.
if (flag.PointsTo(&v8_flags.profile_deserialization)) continue; if (flag.PointsTo(&FLAGS.profile_deserialization)) continue;
// Skip v8_flags.random_seed to allow predictable code caching. // Skip FLAGS.random_seed to allow predictable code caching.
if (flag.PointsTo(&v8_flags.random_seed)) continue; if (flag.PointsTo(&FLAGS.random_seed)) continue;
modified_args_as_string << flag; modified_args_as_string << flag;
} }
std::string args(modified_args_as_string.str()); std::string args(modified_args_as_string.str());
@ -694,7 +694,7 @@ int FlagList::SetFlagsFromCommandLine(int* argc, char** argv, bool remove_flags,
} }
} }
if (v8_flags.help) { if (FLAGS.help) {
if (help_options.HasUsage()) { if (help_options.HasUsage()) {
PrintF(stdout, "%s", help_options.usage()); PrintF(stdout, "%s", help_options.usage());
} }
@ -770,7 +770,7 @@ int FlagList::SetFlagsFromString(const char* str, size_t len) {
// static // static
void FlagList::FreezeFlags() { void FlagList::FreezeFlags() {
flags_frozen.store(true, std::memory_order_relaxed); flags_frozen.store(true, std::memory_order_relaxed);
base::OS::SetDataReadOnly(&v8_flags, sizeof(v8_flags)); base::OS::SetDataReadOnly(&FLAGS, sizeof(FLAGS));
} }
// static // static

10
src/flags/flags.h
View File

@ -11,14 +11,14 @@
#if V8_ENABLE_WEBASSEMBLY #if V8_ENABLE_WEBASSEMBLY
// Include the wasm-limits.h header for some default values of Wasm flags. // 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 // This can be reverted once we can use designated initializations (C++20) for
// {v8_flags} (defined in flags.cc) instead of specifying the default values in // {FLAGS} (defined in flags.cc) instead of specifying the default values in
// the header and using the default constructor. // the header and using the default constructor.
#include "src/wasm/wasm-limits.h" #include "src/wasm/wasm-limits.h"
#endif #endif
namespace v8::internal { namespace v8::internal {
// The value of a single flag (this is the type of all v8_flags.* fields). // The value of a single flag (this is the type of all FLAGS.* fields).
template <typename T> template <typename T>
class FlagValue { class FlagValue {
public: public:
@ -52,7 +52,11 @@ struct alignas(kMinimumOSPageSize) FlagValues {
#include "src/flags/flag-definitions.h" // NOLINT(build/include) #include "src/flags/flag-definitions.h" // NOLINT(build/include)
}; };
V8_EXPORT_PRIVATE extern FlagValues v8_flags; // 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;
// TODO(clemensb): Remove this after v10.7. // TODO(clemensb): Remove this after v10.7.
#define FLAG_MODE_DEFINE_GLOBAL_ALIASES #define FLAG_MODE_DEFINE_GLOBAL_ALIASES