From b0535bddd688609c014c2d65d56f67eb618e72fe Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Thu, 25 Apr 2024 14:26:55 +0000 Subject: [PATCH] [vm] Add pragma vm:unsafe:no-bounds-checks This pragma instructs compiler to remove all bounds checks from the annotated function. This can be helpful when tuning performance of hot tight loops where compiler is unable to eliminate bounds check itself. For very tight loops I have measured 25-50% overhead from bounds checks which I think comes from some combination of general code quality issues due to fixed input registers and increased branch density. In future it could be possible to teach our range analysis to eliminate bounds checks when loop bound is itself bounded by array length, but for now we can resort to this pragma for extremely hot library code. Issue https://github.com/dart-lang/sdk/issues/55522 TEST=vm/cc/BoundsCheckElimination_Pragma R=alexmarkov@google.com Change-Id: Ia7b1e88a16a2b45fa8593a227a4985568892b29c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364500 Reviewed-by: Alexander Markov Commit-Queue: Slava Egorov --- runtime/docs/pragmas.md | 1 + runtime/vm/compiler/backend/flow_graph.cc | 13 +++++++++- runtime/vm/compiler/backend/flow_graph.h | 5 ++++ runtime/vm/compiler/backend/il.cc | 4 ++- .../backend/redundancy_elimination_test.cc | 26 +++++++++++++++++++ runtime/vm/symbols.h | 3 ++- 6 files changed, 49 insertions(+), 3 deletions(-) diff --git a/runtime/docs/pragmas.md b/runtime/docs/pragmas.md index db2dcf40f53..22945d1243f 100644 --- a/runtime/docs/pragmas.md +++ b/runtime/docs/pragmas.md @@ -31,6 +31,7 @@ understands potential repercussions. | Pragma | Meaning | | --- | --- | | `vm:unsafe:no-interrupts` | Removes all `CheckStackOverflow` instructions from the optimized version of the marked function, which disables stack overflow checking and interruption within that function. This pragma exists mainly for performance evaluation and should not be used in a general-purpose code, because VM relies on these checks for OOB message delivery and GC scheduling. | +| `vm:unsafe:no-bounds-checks` | Removes all array bounds checks from the optimized version of the marked function in AOT mode. This pragma exists for optimizing throughput of extremely tight loops. | ## Pragmas for internal use diff --git a/runtime/vm/compiler/backend/flow_graph.cc b/runtime/vm/compiler/backend/flow_graph.cc index 6fcdce7e509..53735a57673 100644 --- a/runtime/vm/compiler/backend/flow_graph.cc +++ b/runtime/vm/compiler/backend/flow_graph.cc @@ -40,6 +40,14 @@ static bool ShouldReorderBlocks(const Function& function, FLAG_reorder_basic_blocks && !function.IsFfiCallbackTrampoline(); } +static bool IsMarkedWithNoBoundsChecks(const Function& function) { + Object& options = Object::Handle(); + return Library::FindPragma(dart::Thread::Current(), + /*only_core=*/false, function, + Symbols::vm_unsafe_no_bounds_checks(), + /*multiple=*/false, &options); +} + FlowGraph::FlowGraph(const ParsedFunction& parsed_function, GraphEntryInstr* graph_entry, intptr_t max_block_id, @@ -70,7 +78,10 @@ FlowGraph::FlowGraph(const ParsedFunction& parsed_function, loop_invariant_loads_(nullptr), captured_parameters_(new(zone()) BitVector(zone(), variable_count())), inlining_id_(-1), - should_print_(false) { + should_print_(false), + should_remove_all_bounds_checks_( + CompilerState::Current().is_aot() && + IsMarkedWithNoBoundsChecks(parsed_function.function())) { should_print_ = FlowGraphPrinter::ShouldPrint(parsed_function.function(), &compiler_pass_filters_); ComputeLocationsOfFixedParameters( diff --git a/runtime/vm/compiler/backend/flow_graph.h b/runtime/vm/compiler/backend/flow_graph.h index 9a8cfedecb6..9ae45118046 100644 --- a/runtime/vm/compiler/backend/flow_graph.h +++ b/runtime/vm/compiler/backend/flow_graph.h @@ -509,6 +509,10 @@ class FlowGraph : public ZoneAllocated { bool should_reorder_blocks() const { return should_reorder_blocks_; } + bool should_remove_all_bounds_checks() const { + return should_remove_all_bounds_checks_; + } + // // High-level utilities. // @@ -735,6 +739,7 @@ class FlowGraph : public ZoneAllocated { intptr_t inlining_id_; bool should_print_; + const bool should_remove_all_bounds_checks_; uint8_t* compiler_pass_filters_ = nullptr; intptr_t max_argument_slot_count_ = -1; diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index e8f561340c7..88b8960a7d6 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -6771,7 +6771,9 @@ bool CheckArrayBoundInstr::IsFixedLengthArrayType(intptr_t cid) { } Definition* CheckBoundBaseInstr::Canonicalize(FlowGraph* flow_graph) { - return IsRedundant() ? index()->definition() : this; + return (flow_graph->should_remove_all_bounds_checks() || IsRedundant()) + ? index()->definition() + : this; } intptr_t CheckArrayBoundInstr::LengthOffsetFor(intptr_t class_id) { diff --git a/runtime/vm/compiler/backend/redundancy_elimination_test.cc b/runtime/vm/compiler/backend/redundancy_elimination_test.cc index fb2879f1a12..1dc4b8bb43d 100644 --- a/runtime/vm/compiler/backend/redundancy_elimination_test.cc +++ b/runtime/vm/compiler/backend/redundancy_elimination_test.cc @@ -1603,6 +1603,32 @@ ISOLATE_UNIT_TEST_CASE(CheckStackOverflowElimination_NoInterruptsPragma) { } } +ISOLATE_UNIT_TEST_CASE(BoundsCheckElimination_Pragma) { + const char* kScript = R"( + import 'dart:typed_data'; + + @pragma('vm:unsafe:no-bounds-checks') + int test(Uint8List list) { + int result = 0; + for (int i = 0; i < 10; i++) { + result = list[i]; + } + return result; + } + )"; + + const auto& root_library = Library::Handle(LoadTestScript(kScript)); + const auto& function = Function::Handle(GetFunction(root_library, "test")); + + TestPipeline pipeline(function, CompilerPass::kAOT); + auto flow_graph = pipeline.RunPasses({}); + for (auto block : flow_graph->postorder()) { + for (auto instr : block->instructions()) { + EXPECT_PROPERTY(instr, !it.IsCheckBoundBase()); + } + } +} + // This test checks that CSE unwraps redefinitions when comparing all // instructions except loads, which are handled specially. ISOLATE_UNIT_TEST_CASE(CSE_Redefinitions) { diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h index 1f68845f1c8..bce66d2a677 100644 --- a/runtime/vm/symbols.h +++ b/runtime/vm/symbols.h @@ -565,7 +565,8 @@ class ObjectPointerVisitor; V(vm_testing_print_flow_graph, "vm:testing:print-flow-graph") \ V(vm_trace_entrypoints, "vm:testing.unsafe.trace-entrypoints-fn") \ V(vm_unsafe_no_interrupts, "vm:unsafe:no-interrupts") \ - V(vm_align_loops, "vm:align-loops") + V(vm_align_loops, "vm:align-loops") \ + V(vm_unsafe_no_bounds_checks, "vm:unsafe:no-bounds-checks") // Contains a list of frequently used strings in a canonicalized form. This // list is kept in the vm_isolate in order to share the copy across isolates