From b2652f5985a6151fee2a96215c2db92ada5b0ab4 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Tue, 4 Jan 2022 16:51:15 +0000 Subject: [PATCH] [vm/aot] Remove support for disabling table dispatch --use_table_dispatch option is enabled everywhere for a while (since https://dart-review.googlesource.com/c/sdk/+/131833). An ability to disable dispatch table is not tested well and adds maintenance burden. TEST=ci Change-Id: I7cec18f12a4b5619d510dfd6dc9f918825918e2a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/225360 Reviewed-by: Aske Simon Christensen Reviewed-by: Slava Egorov Commit-Queue: Alexander Markov --- .../vm/dart/deferred_loading_call_modes_test.dart | 3 --- .../vm/dart/v8_snapshot_profile_writer_test.dart | 6 ------ .../vm/dart_2/deferred_loading_call_modes_test.dart | 3 --- .../vm/dart_2/v8_snapshot_profile_writer_test.dart | 6 ------ runtime/vm/compiler/aot/precompiler.cc | 13 ++----------- runtime/vm/compiler/aot/precompiler.h | 1 - runtime/vm/compiler/aot/precompiler_tracer.cc | 4 +--- runtime/vm/compiler/assembler/assembler_arm.cc | 6 ++---- runtime/vm/compiler/assembler/assembler_arm64.cc | 6 ++---- runtime/vm/compiler/compiler_pass.cc | 4 +--- .../compiler/frontend/kernel_translation_helper.cc | 10 ++-------- runtime/vm/flag_list.h | 1 - runtime/vm/object.cc | 5 +++-- 13 files changed, 13 insertions(+), 55 deletions(-) diff --git a/runtime/tests/vm/dart/deferred_loading_call_modes_test.dart b/runtime/tests/vm/dart/deferred_loading_call_modes_test.dart index 954a3d985bc..0f155d29f9a 100644 --- a/runtime/tests/vm/dart/deferred_loading_call_modes_test.dart +++ b/runtime/tests/vm/dart/deferred_loading_call_modes_test.dart @@ -2,9 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -// VMOptions=--use_table_dispatch=false -// VMOptions=--use_table_dispatch=true - import "splay_test.dart" deferred as splay; // Some non-trivial code. main() async { diff --git a/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart b/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart index 26cc7c7edee..1af4fa393bc 100644 --- a/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart +++ b/runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart @@ -108,7 +108,6 @@ Future testJIT(String dillPath, String snapshotKind) async { Future testAOT(String dillPath, {bool useAsm = false, bool forceDrops = false, - bool useDispatch = true, bool stripUtil = false, // Note: forced true if useAsm. bool stripFlag = false, bool disassemble = false}) async { @@ -129,9 +128,6 @@ Future testAOT(String dillPath, if (forceDrops) { descriptionBuilder.write('-dropped'); } - if (!useDispatch) { - descriptionBuilder.write('-nodispatch'); - } if (stripFlag) { descriptionBuilder.write('-intstrip'); } @@ -158,7 +154,6 @@ Future testAOT(String dillPath, '--no-retain-function-objects', '--no-retain-code-objects' ], - useDispatch ? '--use-table-dispatch' : '--no-use-table-dispatch', if (disassemble) '--disassemble', // Not defined in PRODUCT mode. dillPath, ]; @@ -434,7 +429,6 @@ main() async { // Test unstripped ELF generation directly. await testAOT(aotDillPath); await testAOT(aotDillPath, forceDrops: true); - await testAOT(aotDillPath, forceDrops: true, useDispatch: false); // Test flag-stripped ELF generation. await testAOT(aotDillPath, stripFlag: true); diff --git a/runtime/tests/vm/dart_2/deferred_loading_call_modes_test.dart b/runtime/tests/vm/dart_2/deferred_loading_call_modes_test.dart index 5efc2bdebf5..940c992a3a6 100644 --- a/runtime/tests/vm/dart_2/deferred_loading_call_modes_test.dart +++ b/runtime/tests/vm/dart_2/deferred_loading_call_modes_test.dart @@ -4,9 +4,6 @@ // @dart = 2.9 -// VMOptions=--use_table_dispatch=false -// VMOptions=--use_table_dispatch=true - import "splay_test.dart" deferred as splay; // Some non-trivial code. main() async { diff --git a/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart b/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart index cf3e5811c61..21db861391b 100644 --- a/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart +++ b/runtime/tests/vm/dart_2/v8_snapshot_profile_writer_test.dart @@ -110,7 +110,6 @@ Future testJIT(String dillPath, String snapshotKind) async { Future testAOT(String dillPath, {bool useAsm = false, bool forceDrops = false, - bool useDispatch = true, bool stripUtil = false, // Note: forced true if useAsm. bool stripFlag = false, bool disassemble = false}) async { @@ -131,9 +130,6 @@ Future testAOT(String dillPath, if (forceDrops) { descriptionBuilder.write('-dropped'); } - if (!useDispatch) { - descriptionBuilder.write('-nodispatch'); - } if (stripFlag) { descriptionBuilder.write('-intstrip'); } @@ -160,7 +156,6 @@ Future testAOT(String dillPath, '--no-retain-function-objects', '--no-retain-code-objects' ], - useDispatch ? '--use-table-dispatch' : '--no-use-table-dispatch', if (disassemble) '--disassemble', // Not defined in PRODUCT mode. dillPath, ]; @@ -428,7 +423,6 @@ main() async { // Test unstripped ELF generation directly. await testAOT(aotDillPath); await testAOT(aotDillPath, forceDrops: true); - await testAOT(aotDillPath, forceDrops: true, useDispatch: false); // Test flag-stripped ELF generation. await testAOT(aotDillPath, stripFlag: true); diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index 930e8ad5ed2..a893335a8bd 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -487,10 +487,8 @@ void Precompiler::DoCompileAll() { // as well as other type checks. HierarchyInfo hierarchy_info(T); - if (FLAG_use_table_dispatch) { - dispatch_table_generator_ = new compiler::DispatchTableGenerator(Z); - dispatch_table_generator_->Initialize(IG->class_table()); - } + dispatch_table_generator_ = new compiler::DispatchTableGenerator(Z); + dispatch_table_generator_->Initialize(IG->class_table()); // Precompile constructors to compute information such as // optimized instruction count (used in inlining heuristics). @@ -1446,8 +1444,6 @@ void Precompiler::AddSelector(const String& selector) { } void Precompiler::AddTableSelector(const compiler::TableSelector* selector) { - ASSERT(FLAG_use_table_dispatch); - if (is_tracing()) { tracer_->WriteTableSelectorRef(selector->id); } @@ -1459,10 +1455,6 @@ void Precompiler::AddTableSelector(const compiler::TableSelector* selector) { } bool Precompiler::IsHitByTableSelector(const Function& function) { - if (!FLAG_use_table_dispatch) { - return false; - } - const int32_t selector_id = selector_map()->SelectorId(function); if (selector_id == compiler::SelectorMap::kInvalidSelectorId) return false; return seen_table_selectors_.HasKey(selector_id); @@ -2011,7 +2003,6 @@ void Precompiler::TraceForRetainedFunctions() { void Precompiler::FinalizeDispatchTable() { PRECOMPILER_TIMER_SCOPE(this, FinalizeDispatchTable); - if (!FLAG_use_table_dispatch) return; HANDLESCOPE(T); // Build the entries used to serialize the dispatch table before // dropping functions, as we may clear references to Code objects. diff --git a/runtime/vm/compiler/aot/precompiler.h b/runtime/vm/compiler/aot/precompiler.h index 678905c4dfe..2b748399558 100644 --- a/runtime/vm/compiler/aot/precompiler.h +++ b/runtime/vm/compiler/aot/precompiler.h @@ -247,7 +247,6 @@ class Precompiler : public ValueObject { } compiler::SelectorMap* selector_map() { - ASSERT(FLAG_use_table_dispatch); return dispatch_table_generator_->selector_map(); } diff --git a/runtime/vm/compiler/aot/precompiler_tracer.cc b/runtime/vm/compiler/aot/precompiler_tracer.cc index d0a2e5f8887..9588400658a 100644 --- a/runtime/vm/compiler/aot/precompiler_tracer.cc +++ b/runtime/vm/compiler/aot/precompiler_tracer.cc @@ -90,9 +90,7 @@ void PrecompilerTracer::WriteEntityTable() { const auto& fun = Function::Cast(obj); cls_ = fun.Owner(); const intptr_t selector_id = - FLAG_use_table_dispatch - ? precompiler_->selector_map()->SelectorId(fun) - : -1; + precompiler_->selector_map()->SelectorId(fun); Write("\"%c\",%" Pd ",%" Pd ",%" Pd "", fun.IsDynamicFunction() ? 'F' : 'S', InternEntity(cls_), InternString(NameForTrace(fun)), selector_id); diff --git a/runtime/vm/compiler/assembler/assembler_arm.cc b/runtime/vm/compiler/assembler/assembler_arm.cc index a9a71ee10d9..8d265deadec 100644 --- a/runtime/vm/compiler/assembler/assembler_arm.cc +++ b/runtime/vm/compiler/assembler/assembler_arm.cc @@ -1604,10 +1604,8 @@ void Assembler::LoadPoolPointer(Register reg) { void Assembler::SetupGlobalPoolAndDispatchTable() { ASSERT(FLAG_precompiled_mode); ldr(PP, Address(THR, target::Thread::global_object_pool_offset())); - if (FLAG_use_table_dispatch) { - ldr(DISPATCH_TABLE_REG, - Address(THR, target::Thread::dispatch_table_array_offset())); - } + ldr(DISPATCH_TABLE_REG, + Address(THR, target::Thread::dispatch_table_array_offset())); } void Assembler::LoadIsolate(Register rd) { diff --git a/runtime/vm/compiler/assembler/assembler_arm64.cc b/runtime/vm/compiler/assembler/assembler_arm64.cc index b4dcf19d2b3..07bcb1375ce 100644 --- a/runtime/vm/compiler/assembler/assembler_arm64.cc +++ b/runtime/vm/compiler/assembler/assembler_arm64.cc @@ -1552,10 +1552,8 @@ void Assembler::SetupGlobalPoolAndDispatchTable() { ASSERT(FLAG_precompiled_mode); ldr(PP, Address(THR, target::Thread::global_object_pool_offset())); sub(PP, PP, Operand(kHeapObjectTag)); // Pool in PP is untagged! - if (FLAG_use_table_dispatch) { - ldr(DISPATCH_TABLE_REG, - Address(THR, target::Thread::dispatch_table_array_offset())); - } + ldr(DISPATCH_TABLE_REG, + Address(THR, target::Thread::dispatch_table_array_offset())); } void Assembler::CheckCodePointer() { diff --git a/runtime/vm/compiler/compiler_pass.cc b/runtime/vm/compiler/compiler_pass.cc index 0f9e5ea0cda..1faa8d39b8b 100644 --- a/runtime/vm/compiler/compiler_pass.cc +++ b/runtime/vm/compiler/compiler_pass.cc @@ -470,9 +470,7 @@ COMPILER_PASS(SelectRepresentations, { }); COMPILER_PASS(UseTableDispatch, { - if (FLAG_use_table_dispatch) { - state->call_specializer->ReplaceInstanceCallsWithDispatchTableCalls(); - } + state->call_specializer->ReplaceInstanceCallsWithDispatchTableCalls(); }); COMPILER_PASS_REPEAT(CSE, { return DominatorBasedCSE::Optimize(flow_graph); }); diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.cc b/runtime/vm/compiler/frontend/kernel_translation_helper.cc index 86b49ff8df6..bf9a9ad06b6 100644 --- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc +++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc @@ -3636,10 +3636,7 @@ void TypeTranslator::SetupUnboxingInfoMetadata(const Function& function, const auto unboxing_info = unboxing_info_metadata_helper_.GetUnboxingInfoMetadata(kernel_offset); - // TODO(dartbug.com/32292): accept unboxed parameters and return value - // when FLAG_use_table_dispatch == false. - if (FLAG_precompiled_mode && unboxing_info != nullptr && - FLAG_use_table_dispatch) { + if (FLAG_precompiled_mode && unboxing_info != nullptr) { for (intptr_t i = 0; i < unboxing_info->unboxed_args_info.length(); i++) { SetupUnboxingInfoOfParameter(function, i, unboxing_info); } @@ -3655,10 +3652,7 @@ void TypeTranslator::SetupUnboxingInfoMetadataForFieldAccessors( const auto unboxing_info = unboxing_info_metadata_helper_.GetUnboxingInfoMetadata(kernel_offset); - // TODO(dartbug.com/32292): accept unboxed parameters and return value - // when FLAG_use_table_dispatch == false. - if (FLAG_precompiled_mode && unboxing_info != nullptr && - FLAG_use_table_dispatch) { + if (FLAG_precompiled_mode && unboxing_info != nullptr) { if (field_accessor.IsImplicitSetterFunction()) { for (intptr_t i = 0; i < unboxing_info->unboxed_args_info.length(); i++) { SetupUnboxingInfoOfParameter(field_accessor, i, unboxing_info); diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h index cccfdfb40b8..6e885be3c17 100644 --- a/runtime/vm/flag_list.h +++ b/runtime/vm/flag_list.h @@ -184,7 +184,6 @@ constexpr bool FLAG_support_il_printer = false; P(reorder_basic_blocks, bool, true, "Reorder basic blocks") \ C(stress_async_stacks, false, false, bool, false, \ "Stress test async stack traces") \ - P(use_table_dispatch, bool, true, "Enable dispatch table based calls.") \ P(retain_function_objects, bool, true, \ "Serialize function objects for all code objects even if not otherwise " \ "needed in the precompiled runtime.") \ diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 9bb4bb7a25d..32593176d04 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -10161,8 +10161,9 @@ bool Function::NeedsMonomorphicCheckedEntry(Zone* zone) const { return true; } - // If table dispatch is disabled, all instance calls use switchable calls. - if (!(FLAG_precompiled_mode && FLAG_use_table_dispatch)) { + // AOT mode uses table dispatch. + // In JIT mode all instance calls use switchable calls. + if (!FLAG_precompiled_mode) { return true; }