Commit b8f91666 authored by Tobias Tebbi's avatar Tobias Tebbi Committed by Commit Bot

[flags] warn about contradictory flags

Design Doc: https://docs.google.com/document/d/1lkvu8crkK7Ei39qjkPCFijpNyxWXsOktG9GB-7K34jM/

Bug: v8:10577
Change-Id: Ib9cfdffa401c48c895bf31caed5ee03545beddab
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2154792Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarMichael Achenbach <machenbach@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Reviewed-by: 's avatarTamer Tas <tmrts@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68168}
parent b6133551
......@@ -3172,6 +3172,7 @@ bool Shell::SetOptions(int argc, char* argv[]) {
}
}
i::FLAG_abort_on_contradictory_flags = true;
v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
options.mock_arraybuffer_allocator = i::FLAG_mock_arraybuffer_allocator;
options.mock_arraybuffer_allocator_limit =
......@@ -3618,12 +3619,11 @@ class D8Testing {
"--max-inlined-bytecode-size=999999 "
"--max-inlined-bytecode-size-cumulative=999999 "
"--noalways-opt";
static const char* kForcedOptimizations = "--always-opt";
if (run == GetStressRuns() - 1) {
V8::SetFlagsFromString(kForcedOptimizations);
} else {
if (run < GetStressRuns() - 1) {
V8::SetFlagsFromString(kLazyOptimizations);
} else {
i::FLAG_always_opt = true;
}
}
......
......@@ -15,6 +15,11 @@
#define DEFINE_IMPLICATION(whenflag, thenflag) \
DEFINE_VALUE_IMPLICATION(whenflag, thenflag, true)
// A weak implication will be overwritten by a normal implication or by an
// explicit flag.
#define DEFINE_WEAK_IMPLICATION(whenflag, thenflag) \
DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, true)
#define DEFINE_NEG_IMPLICATION(whenflag, thenflag) \
DEFINE_VALUE_IMPLICATION(whenflag, thenflag, false)
......@@ -59,14 +64,22 @@
// 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) \
if (FLAG_##whenflag) FLAG_##thenflag = value;
#define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(FLAG_##whenflag, #whenflag, &FLAG_##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(FLAG_##whenflag, #whenflag, &FLAG_##thenflag, \
value, true);
#define DEFINE_GENERIC_IMPLICATION(whenflag, statement) \
if (FLAG_##whenflag) statement;
#define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \
if (!FLAG_##whenflag) FLAG_##thenflag = value;
#define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \
changed |= TriggerImplication(!FLAG_##whenflag, #whenflag, &FLAG_##thenflag, \
value, false);
// We apply a generic macro to the flags.
#elif defined(FLAG_MODE_APPLY)
......@@ -94,6 +107,10 @@
#define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value)
#endif
#ifndef DEFINE_WEAK_VALUE_IMPLICATION
#define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value)
#endif
#ifndef DEFINE_GENERIC_IMPLICATION
#define DEFINE_GENERIC_IMPLICATION(whenflag, statement)
#endif
......@@ -203,6 +220,14 @@ struct MaybeBoolFlag {
//
#define FLAG FLAG_FULL
// ATTENTION: This is set to true by default in d8. But For API compatibility,
// it generally defaults to false.
DEFINE_BOOL(abort_on_contradictory_flags, false,
"Disallow flags or implications overriding each other.")
// This implication is also hard-coded into the flags processing to make sure it
// becomes active before we even process subsequent flags.
DEFINE_NEG_IMPLICATION(fuzzing, abort_on_contradictory_flags)
// Flags for language modes and experimental language features.
DEFINE_BOOL(use_strict, false, "enforce strict mode")
......@@ -373,8 +398,23 @@ DEFINE_BOOL(future, FUTURE_BOOL,
"Implies all staged features that we want to ship in the "
"not-too-far future")
DEFINE_IMPLICATION(future, write_protect_code_memory)
DEFINE_IMPLICATION(future, finalize_streaming_on_background)
DEFINE_WEAK_IMPLICATION(future, write_protect_code_memory)
DEFINE_WEAK_IMPLICATION(future, finalize_streaming_on_background)
// Flags for jitless
DEFINE_BOOL(jitless, V8_LITE_BOOL,
"Disable runtime allocation of executable memory.")
// Jitless V8 has a few implications:
DEFINE_NEG_IMPLICATION(jitless, opt)
// Field representation tracking is only used by TurboFan.
DEFINE_NEG_IMPLICATION(jitless, track_field_types)
DEFINE_NEG_IMPLICATION(jitless, track_heap_object_fields)
// Regexps are interpreted.
DEFINE_IMPLICATION(jitless, regexp_interpret_all)
// asm.js validation is disabled since it triggers wasm code generation.
DEFINE_NEG_IMPLICATION(jitless, validate_asm)
// --jitless also implies --no-expose-wasm, see InitializeOncePerProcessImpl.
DEFINE_BOOL(assert_types, false,
"generate runtime type assertions to test the typer")
......@@ -427,21 +467,6 @@ DEFINE_BOOL_READONLY(string_slices, true, "use string slices")
DEFINE_INT(interrupt_budget, 144 * KB,
"interrupt budget which should be used for the profiler counter")
// Flags for jitless
DEFINE_BOOL(jitless, V8_LITE_BOOL,
"Disable runtime allocation of executable memory.")
// Jitless V8 has a few implications:
DEFINE_NEG_IMPLICATION(jitless, opt)
// Field representation tracking is only used by TurboFan.
DEFINE_NEG_IMPLICATION(jitless, track_field_types)
DEFINE_NEG_IMPLICATION(jitless, track_heap_object_fields)
// Regexps are interpreted.
DEFINE_IMPLICATION(jitless, regexp_interpret_all)
// asm.js validation is disabled since it triggers wasm code generation.
DEFINE_NEG_IMPLICATION(jitless, validate_asm)
// --jitless also implies --no-expose-wasm, see InitializeOncePerProcessImpl.
// Flags for inline caching and feedback vectors.
DEFINE_BOOL(use_ic, true, "use inline caching")
DEFINE_INT(budget_for_feedback_vector_allocation, 1 * KB,
......@@ -514,7 +539,7 @@ DEFINE_BOOL(concurrent_inlining, false,
"run optimizing compiler's inlining phase on a separate thread")
DEFINE_INT(max_serializer_nesting, 25,
"maximum levels for nesting child serializers")
DEFINE_IMPLICATION(future, concurrent_inlining)
DEFINE_WEAK_IMPLICATION(future, concurrent_inlining)
DEFINE_BOOL(trace_heap_broker_verbose, false,
"trace the heap broker verbosely (all reports)")
DEFINE_BOOL(trace_heap_broker_memory, false,
......@@ -726,7 +751,7 @@ DEFINE_BOOL(liftoff, true,
#else
DEFINE_BOOL(liftoff, false,
"enable Liftoff, the baseline compiler for WebAssembly")
DEFINE_IMPLICATION(future, liftoff)
DEFINE_WEAK_IMPLICATION(future, liftoff)
#endif
// We can't tier up (from Liftoff to TurboFan) in single-threaded mode, hence
// disable Liftoff in that configuration for now. The alternative is disabling
......@@ -1813,9 +1838,11 @@ DEFINE_IMPLICATION(unbox_double_fields, track_double_fields)
#undef DEFINE_STRING
#undef DEFINE_FLOAT
#undef DEFINE_IMPLICATION
#undef DEFINE_WEAK_IMPLICATION
#undef DEFINE_NEG_IMPLICATION
#undef DEFINE_NEG_VALUE_IMPLICATION
#undef DEFINE_VALUE_IMPLICATION
#undef DEFINE_WEAK_VALUE_IMPLICATION
#undef DEFINE_GENERIC_IMPLICATION
#undef DEFINE_ALIAS_BOOL
#undef DEFINE_ALIAS_INT
......
This diff is collapsed.
......@@ -1056,6 +1056,7 @@ int main(int argc, char* argv[]) {
v8::V8::InitializeICUDefaultLocation(argv[0]);
std::unique_ptr<v8::Platform> platform(v8::platform::NewDefaultPlatform());
v8::V8::InitializePlatform(platform.get());
v8::internal::FLAG_abort_on_contradictory_flags = true;
v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
v8::V8::InitializeExternalStartupData(argv[0]);
v8::V8::Initialize();
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --no-stress-opt --trace-wasm-memory --liftoff --no-future
// Flags: --no-stress-opt --trace-wasm-memory --liftoff
// Flags: --no-wasm-tier-up --experimental-wasm-simd
// Flags: --enable-sse3 --enable-sse4-1
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --no-stress-opt --trace-wasm-memory --no-liftoff --no-future
// Flags: --no-stress-opt --trace-wasm-memory --no-liftoff
// Flags: --experimental-wasm-simd
load("test/mjsunit/wasm/wasm-module-builder.js");
......
......@@ -2,8 +2,15 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --noverify-heap --noenable-slow-asserts
// Flags: --opt --no-always-opt
// The flags are processed left to right. --no-abort-on-contradictory-flags
// disables the checking for conflicts, then we process --noverify-heap and
// --noenable-slow-asserts, which the test runner already set to true before.
// This causes the flags to be overwritten while silencing the error. Then we
// re-enable --abort-on-contradictory-flags to make sure that the processing of
// other flags and flag implications, which happens later, still produces
// errors.
// Flags: --no-abort-on-contradictory-flags --noverify-heap --noenable-slow-asserts --abort-on-contradictory-flags
// Flags: --allow-natives-syntax --opt --no-always-opt
// --noverify-heap and --noenable-slow-asserts are set because the test is too
// slow with it on.
......
......@@ -367,17 +367,6 @@
'regexp-tier-up-multiple': [SKIP],
'regress/regress-996234': [SKIP],
# Tests that depend on optimization (beyond doing assertOptimized).
'compiler/is-being-interpreted-*': [SKIP],
'compiler/serializer-accessors': [SKIP],
'compiler/serializer-apply': [SKIP],
'compiler/serializer-call': [SKIP],
'compiler/serializer-dead-after-jump': [SKIP],
'compiler/serializer-dead-after-return': [SKIP],
'compiler/serializer-transition-propagation': [SKIP],
'regress/regress-1049982-1': [SKIP],
'regress/regress-1049982-2': [SKIP],
# These tests check that we can trace the compiler.
'tools/compiler-trace-flags': [SKIP],
'tools/compiler-trace-flags-wasm': [SKIP],
......@@ -1153,10 +1142,6 @@
'regress/regress-1049982-2': [SKIP],
'es6/iterator-eager-deopt': [SKIP],
# interrupt_budget overrides don't work with TurboProp.
'interrupt-budget-override': [SKIP],
'never-optimize': [SKIP],
# In turboprop we reuse the optimized code on soft deopt. The following tests
# test for a soft deopt and they won't work in TurboProp.
'deopt-recursive-soft-once': [SKIP],
......
......@@ -2,7 +2,15 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --random-seed=20 --nostress-opt --noalways-opt --predictable
// Overwrite the random seed provided by the test runner to make this test less
// flaky.
// The flags are processed left to right. --no-abort-on-contradictory-flags
// disables the checking for conflicts, then we process --random-seed=20 to
// overwrite the value the test runner already set before. Then we re-enable
// --abort-on-contradictory-flags to make sure that the processing of other
// flags and flag implications, which happens later, still produces errors.
// Flags: --no-abort-on-contradictory-flags --random-seed=20 --abort-on-contradictory-flags
// Flags: --nostress-opt --noalways-opt --predictable
(function() {
var kHistory = 2;
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --noconcurrent-recompilation --expose-gc --allow-natives-syntax
// Flags: --expose-gc --allow-natives-syntax
// Flags: --concurrent-recompilation --block-concurrent-recompilation
gc();
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --gc-interval=439 --random-seed=-423594851
// Flags: --allow-natives-syntax --gc-interval=439
var __v_3;
function __f_2() {
......
......@@ -4,7 +4,7 @@
// Flags: --allow-natives-syntax --stress-compaction
// To reliably reproduce the crash use --verify-heap --random-seed=-133185440
// To reliably reproduce the crash use --verify-heap
function __f_2(o) {
return o.field.b.x;
......
......@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --random-seed=1
for (let i = 0; i < 54; ++i) Math.random();
let sum = 0;
for (let i = 0; i < 10; ++i)
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --no-liftoff --no-future --debug-code
// Flags: --no-liftoff --debug-code
load('test/mjsunit/wasm/wasm-module-builder.js');
......
......@@ -2,6 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --random-seed=-1595876594 --disable-in-process-stack-traces --no-lazy
// Flags: --disable-in-process-stack-traces --no-lazy
var __v_47 = ({[__v_46]: __f_52}) => { var __v_46 = 'b'; return __f_52; };
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --expose-gc --predictable --random-seed=-1109634722
// Flags: --expose-gc --predictable
gc();
gc();
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Flags: --random-seed=891196975 --expose-gc --allow-natives-syntax
// Flags: --expose-gc --allow-natives-syntax
// Flags: --gc-interval=207 --stress-compaction --validate-asm
// Flags: --opt --no-always-opt
//
......
......@@ -2,9 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Flags: --random-seed=891196975 --expose-gc --allow-natives-syntax
// Flags: --gc-interval=207 --stress-compaction --validate-asm
// Flags: --opt --no-always-opt
// Flags: --expose-gc --allow-natives-syntax --gc-interval=207
// Flags: --stress-compaction --validate-asm --opt --no-always-opt
//
// /v8/test/mjsunit/wasm/grow-memory.js
// /v8/test/mjsunit/regress/regress-540.js
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --random-seed=1557792826 --expose-gc --invoke-weak-callbacks --omit-quit --gc-interval=469 --validate-asm
// Flags: --expose-gc --invoke-weak-callbacks --omit-quit --gc-interval=469 --validate-asm
function nop() {}
var __v_42 = {};
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Flags: --random-seed=-1101427159 --enable-slow-asserts --expose-wasm
// Flags: --enable-slow-asserts --expose-wasm
(function __f_7() {
assertThrows(() => new WebAssembly.Memory({initial: 59199}), RangeError);
......
......@@ -25,8 +25,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --random-seed=17 --allow-natives-syntax
// Flags: --expose-externalize-string
// Flags: --allow-natives-syntax --expose-externalize-string
assertEquals("ΚΟΣΜΟΣ ΚΟΣΜΟΣ".toLowerCase(), "κοσμος κοσμος");
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --liftoff --no-future --no-wasm-tier-up
// Flags: --allow-natives-syntax --liftoff --no-wasm-tier-up
// Compile functions 0 and 2 with Turbofan, the rest with Liftoff:
// Flags: --wasm-tier-mask-for-testing=5
......
......@@ -24,8 +24,7 @@ ALL_VARIANT_FLAGS = {
# independent of JS optimizations, so we can combine those configs.
"nooptimization": [["--no-opt", "--liftoff", "--no-wasm-tier-up"]],
"slow_path": [["--force-slow-path"]],
"stress": [["--stress-opt", "--always-opt", "--no-liftoff",
"--stress-lazy-source-positions"]],
"stress": [["--stress-opt", "--no-liftoff", "--stress-lazy-source-positions"]],
"stress_js_bg_compile_wasm_code_gc": [["--stress-background-compile",
"--stress-wasm-code-gc"]],
"stress_incremental_marking": [["--stress-incremental-marking"]],
......@@ -40,6 +39,21 @@ ALL_VARIANT_FLAGS = {
"top_level_await": [["--harmony-top-level-await"]],
}
# Flags that lead to a contradiction with the flags provided by the respective
# variant. This depends on the flags specified in ALL_VARIANT_FLAGS and on the
# implications defined in flag-definitions.h
INCOMPATIBLE_FLAGS_PER_VARIANT = {
"assert_types": ["--no-assert-types"],
"jitless": ["--opt", "--liftoff", "--track-field-types", "--validate-asm"],
"no_wasm_traps": ["--wasm-trap-handler"],
"nooptimization": ["--opt", "--no-liftoff", "--predictable", "--wasm-tier-up"],
"slow_path": ["--no-force-slow-path"],
"stress_incremental_marking": ["--no-stress-incremental-marking"],
"stress_js_bg_compile_wasm_code_gc": ["--no-stress-background-compile"],
"stress": ["--always-opt", "--liftoff"],
"turboprop": ["--turbo-inlining", "--interrupt-budget=*", "--no-turboprop"],
}
SLOW_VARIANTS = set([
'stress',
'stress_snapshot',
......
......@@ -34,6 +34,7 @@ from ..outproc import base as outproc
from ..local import command
from ..local import statusfile
from ..local import utils
from ..local.variants import INCOMPATIBLE_FLAGS_PER_VARIANT
FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
......@@ -84,7 +85,8 @@ class TestCase(object):
# Outcomes
self._statusfile_outcomes = None
self.expected_outcomes = None
self._expected_outcomes = None
self._checked_flag_contradictions = False
self._statusfile_flags = None
self._prepare_outcomes()
......@@ -116,7 +118,7 @@ class TestCase(object):
outcomes = self.suite.statusfile.get_outcomes(self.name, self.variant)
self._statusfile_outcomes = filter(not_flag, outcomes)
self._statusfile_flags = filter(is_flag, outcomes)
self.expected_outcomes = (
self._expected_outcomes = (
self._parse_status_file_outcomes(self._statusfile_outcomes))
def _parse_status_file_outcomes(self, outcomes):
......@@ -141,6 +143,30 @@ class TestCase(object):
return outproc.OUTCOMES_FAIL
return expected_outcomes or outproc.OUTCOMES_PASS
@property
def expected_outcomes(self):
if not self._checked_flag_contradictions:
self._checked_flag_contradictions = True
def normalize_flag(flag):
return flag.replace("_", "-").replace("--no-", "--no")
all_flags = [normalize_flag(flag) for flag in self._get_cmd_params()]
def has_flag(conflicting_flag):
conflicting_flag = normalize_flag(conflicting_flag)
if conflicting_flag in all_flags:
return True
if conflicting_flag.endswith("*"):
return any(flag.startswith(conflicting_flag[:-1]) for flag in all_flags)
return False
if self.variant in INCOMPATIBLE_FLAGS_PER_VARIANT:
for conflicting_flag in INCOMPATIBLE_FLAGS_PER_VARIANT[self.variant]:
if has_flag(conflicting_flag):
self._expected_outcomes = outproc.OUTCOMES_FAIL
return self._expected_outcomes
@property
def do_skip(self):
return (statusfile.SKIP in self._statusfile_outcomes and
......
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