Commit a25aa43e authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

[cleanup] Remove --stress-opt

--stress-opt never did what we wanted it to; it ran its runs in
different contexts (therefore not able to share feedback across runs),
and even if it didn't, each run would create new closures for any
defined closures, so we'd still more than likely end up poly- or
mega-morphic.

Fuzzers cover this use case better than --stress-opt ever did, so now
it's just using precious bot time. We can get rid of it.

Bug: v8:10386
Change-Id: Ibbb9207d887b4b1dc4ec9093858d477c0f95eb37
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3803228
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82722}
parent 8809950e
......@@ -4659,7 +4659,6 @@ void PreProcessUnicodeFilenameArg(char* argv[], int i) {
bool Shell::SetOptions(int argc, char* argv[]) {
bool logfile_per_isolate = false;
bool no_always_turbofan = false;
options.d8_path = argv[0];
for (int i = 0; i < argc; i++) {
if (strcmp(argv[i], "--") == 0) {
......@@ -4675,16 +4674,6 @@ bool Shell::SetOptions(int argc, char* argv[]) {
} else if (strcmp(argv[i], "--simulate-errors") == 0) {
options.simulate_errors = true;
argv[i] = nullptr;
} else if (strcmp(argv[i], "--stress-opt") == 0) {
options.stress_opt = true;
argv[i] = nullptr;
} else if (strcmp(argv[i], "--nostress-opt") == 0 ||
strcmp(argv[i], "--no-stress-opt") == 0) {
options.stress_opt = false;
argv[i] = nullptr;
} else if (strcmp(argv[i], "--noalways-turbofan") == 0 ||
strcmp(argv[i], "--no-always-turbofan") == 0) {
no_always_turbofan = true;
} else if (strcmp(argv[i], "--fuzzing") == 0 ||
strcmp(argv[i], "--no-abort-on-contradictory-flags") == 0 ||
strcmp(argv[i], "--noabort-on-contradictory-flags") == 0) {
......@@ -4904,11 +4893,6 @@ bool Shell::SetOptions(int argc, char* argv[]) {
}
}
if (options.stress_opt && no_always_turbofan &&
check_d8_flag_contradictions) {
FATAL("Flag --no-always-turbofan is incompatible with --stress-opt.");
}
if (options.throw_on_failed_access_check &&
options.noop_on_failed_access_check && check_d8_flag_contradictions) {
FATAL(
......@@ -5455,24 +5439,6 @@ class D8Testing {
#endif
}
/**
* Indicate the number of the run which is about to start. The value of run
* should be between 0 and one less than the result from GetStressRuns()
*/
static void PrepareStressRun(int run) {
static const char* kLazyOptimizations =
"--prepare-always-turbofan "
"--max-inlined-bytecode-size=999999 "
"--max-inlined-bytecode-size-cumulative=999999 "
"--noalways-turbofan";
if (run == 0) {
V8::SetFlagsFromString(kLazyOptimizations);
} else if (run == GetStressRuns() - 1) {
i::FLAG_always_turbofan = true;
}
}
/**
* Force deoptimization of all functions.
*/
......@@ -5660,9 +5626,7 @@ int Shell::Main(int argc, char* argv[]) {
// Disable flag freezing if we are producing a code cache, because for that we
// modify FLAG_hash_seed (below).
// Also --stress-opt modifies flags between runs.
if (options.code_cache_options != ShellOptions::kNoProduceCache ||
options.stress_opt) {
if (options.code_cache_options != ShellOptions::kNoProduceCache) {
i::FLAG_freeze_flags_after_init = false;
}
......@@ -5793,18 +5757,7 @@ int Shell::Main(int argc, char* argv[]) {
CpuProfilingOptions{});
}
if (options.stress_opt) {
options.stress_runs = D8Testing::GetStressRuns();
for (int i = 0; i < options.stress_runs && result == 0; i++) {
printf("============ Stress %d/%d ============\n", i + 1,
options.stress_runs.get());
D8Testing::PrepareStressRun(i);
bool last_run = i == options.stress_runs - 1;
result = RunMain(isolate, last_run);
}
printf("======== Full Deoptimization =======\n");
D8Testing::DeoptimizeAll(isolate);
} else if (i::FLAG_stress_runs > 0) {
if (i::FLAG_stress_runs > 0) {
options.stress_runs = i::FLAG_stress_runs;
for (int i = 0; i < options.stress_runs && result == 0; i++) {
printf("============ Run %d/%d ============\n", i + 1,
......
......@@ -415,7 +415,6 @@ class ShellOptions {
DisallowReassignment<bool> wait_for_background_tasks = {
"wait-for-background-tasks", true};
DisallowReassignment<bool> simulate_errors = {"simulate-errors", false};
DisallowReassignment<bool> stress_opt = {"stress-opt", false};
DisallowReassignment<int> stress_runs = {"stress-runs", 1};
DisallowReassignment<bool> interactive_shell = {"shell", false};
bool test_shell = false;
......
......@@ -1108,10 +1108,9 @@
################################################################################
['variant == stress', {
# The 'stress' variants sets the '--stress-opt' d8 flag, which executes 2 runs
# in debug mode and 5 runs in release mode. Hence the module will be cached
# between runs, and the correct caching behavior cannot be observed anymore in
# the later runs.
# The 'stress' variants sets the '--no-liftoff' flag, which means that Wasm
# compiles directly to TurboFan. This makes the cache no longer incremental,
# and these tests don't make sense.
'test-streaming-compilation/AsyncTestIncrementalCaching': [SKIP],
'test-streaming-compilation/SingleThreadedTestIncrementalCaching': [SKIP],
}],
......
......@@ -36,10 +36,6 @@
##############################################################################
['variant == stress', {
# TODO(turbofan): Functions with eval or debugger now get optimized
# with Turbofan, which has issues with the debugger issues.
'debug/debug-evaluate-locals': [FAIL],
# Very slow in stress mode.
'regress/regress-2318': [SKIP],
......
......@@ -53,8 +53,7 @@ class TestCase(testcase.D8TestCase):
self._base_path = os.path.join(self.suite.root, self.path)
source = self.get_source()
self._source_files = self._parse_source_files(source)
# Do not stress-opt message tests, since that changes the output.
self._source_flags = ['--no-stress-opt'] + self._parse_source_flags(source)
self._source_flags = self._parse_source_flags(source)
def _parse_source_files(self, source):
files = []
......
......@@ -1212,16 +1212,15 @@
}], # 'arch == s390x'
##############################################################################
# TODO(v8:10386): The stress variant has been found to have limited value, and
# we've already removed its main flag (--stress-opt). We may want to remove it
# entirely.
['variant == stress', {
# Slow tests.
'array-natives-elements': [SKIP],
'ignition/regress-599001-verifyheap': [SKIP],
'unicode-test': [SKIP],
# The RegExp code cache means running this test multiple times is invalid.
'regexp-tier-up': [SKIP],
'regexp-tier-up-multiple': [SKIP],
# Flaky crash on Odroid devices: https://crbug.com/v8/7678
'regress/regress-336820': [PASS, ['arch == arm and not simulator_run', SKIP]],
......
......@@ -31,7 +31,7 @@ ALL_VARIANT_FLAGS = {
"nooptimization": [["--no-turbofan", "--liftoff", "--no-wasm-tier-up"]],
"slow_path": [["--force-slow-path"]],
"stress": [[
"--stress-opt", "--no-liftoff", "--stress-lazy-source-positions",
"--no-liftoff", "--stress-lazy-source-positions",
"--no-wasm-generic-wrapper", "--no-wasm-lazy-compilation"
]],
"stress_concurrent_allocation": [["--stress-concurrent-allocation"]],
......@@ -75,9 +75,6 @@ INCOMPATIBLE_FLAGS_PER_VARIANT = {
# stress_snapshot.
"stress_snapshot": ["--expose-fast-api"],
"stress": [
"--always-turbofan", "--no-always-turbofan",
"--max-inlined-bytecode-size=*",
"--max-inlined-bytecode-size-cumulative=*", "--stress-inline",
"--liftoff-only", "--wasm-speculative-inlining",
"--wasm-dynamic-tiering"
],
......
......@@ -373,8 +373,6 @@ class TestCase(object):
def _get_timeout(self, params):
timeout = self._test_config.timeout
if "--stress-opt" in params:
timeout *= 4
if "--jitless" in params:
timeout *= 2
if "--no-turbofan" in params:
......
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