Skip to content

Commit

Permalink
Version 3.6.0-273.0.dev
Browse files Browse the repository at this point in the history
Merge f80214e into dev
  • Loading branch information
Dart CI committed Sep 24, 2024
2 parents 07fd0b5 + f80214e commit 24ed9af
Show file tree
Hide file tree
Showing 11 changed files with 281 additions and 13 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/scorecards-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ jobs:

# Upload the results to GitHub's code scanning dashboard.
- name: "Upload to code-scanning"
uses: github/codeql-action/upload-sarif@4dd16135b69a43b6c8efb853346f8437d92d3c93
uses: github/codeql-action/upload-sarif@294a9d92911152fe08befb9ec03e240add280cb3
with:
sarif_file: results.sarif
6 changes: 5 additions & 1 deletion pkg/dart2wasm/bin/run_wasm.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,11 @@ if (argsSplit != -1) {
self.Response = function () { }

self.location = {}
self.location.href = 'file://' + args[wasmArg];
if (args[wasmArg].startsWith('/')) {
self.location.href = 'file://' + args[wasmArg];
} else {
self.location.href = args[wasmArg];
}

// Signals `Stopwatch._initTicker` to use `Date.now` to get ticks instead of
// `performance.now`, as it's not available in d8.
Expand Down
212 changes: 212 additions & 0 deletions runtime/tests/vm/dart/no_bound_check_il_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file// 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.

// Verifies that compiler can remove bounds checks when told by pragma.

import 'package:expect/expect.dart';
import 'package:vm/testing/il_matchers.dart';

import 'dart:typed_data';

@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int direct_uint8list_has_bounds_check(Uint8List list) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = list[i];
}
return result;
}

@pragma('vm:unsafe:no-bounds-checks')
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int direct_uint8list_no_bounds_check(Uint8List list) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = list[i];
}
return result;
}

@pragma('vm:prefer-inline')
int helper_uint8list_no_pragma(Uint8List list) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = list[i];
}
return result;
}

@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int inlined_uint8list_has_bounds_check(Uint8List list) {
if (helper_uint8list_no_pragma(list) == 42) {
return 42;
}
return 0;
}

@pragma('vm:prefer-inline')
@pragma('vm:unsafe:no-bounds-checks')
int helper_uint8list_pragma(Uint8List list) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = list[i];
}
return result;
}

// For whatever reason, this passes even without the fix, despite real-world
// example in CFE (AbstractScanner.select) as well as a test in the VM
// (runtime/vm/compiler/backend/redundancy_elimination_test.cc,
// BoundsCheckElimination_Pragma_Inline) needs the fix.
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int inlined_uint8list_no_bounds_check(Uint8List list, bool b) {
if (b) {
return helper_uint8list_pragma(list);
} else {
return helper_uint8list_pragma(list) + 42;
}
}

@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int direct_string_has_bounds_check(String s) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = s.codeUnitAt(i);
}
return result;
}

@pragma('vm:unsafe:no-bounds-checks')
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int direct_string_no_bounds_check(String s) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = s.codeUnitAt(i);
}
return result;
}

@pragma('vm:prefer-inline')
int helper_string_no_pragma(String s) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = s.codeUnitAt(i);
}
return result;
}

@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int inlined_string_has_bounds_check(String s) {
if (helper_string_no_pragma(s) == 42) {
return 42;
}
return 0;
}

@pragma('vm:prefer-inline')
@pragma('vm:unsafe:no-bounds-checks')
int helper_string_pragma(String s) {
int result = 0;
for (int i = 0; i < 10; i++) {
result = s.codeUnitAt(i);
}
return result;
}

@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int inlined_string_no_bounds_check(String s) {
if (helper_string_pragma(s) == 42) {
return 42;
}
return 0;
}

void main() {
// Uint8List access directly.
direct_uint8list_has_bounds_check(new Uint8List(20));
direct_uint8list_no_bounds_check(new Uint8List(20));

// Uint8List access via inlined function.
inlined_uint8list_has_bounds_check(new Uint8List(20));
inlined_uint8list_no_bounds_check(
new Uint8List(20), new Uint8List(20).toString().length.isEven);

// String access directly.
direct_string_has_bounds_check(new Uint8List(20).toString());
direct_string_no_bounds_check(new Uint8List(20).toString());

// String access via inlined function.
inlined_string_has_bounds_check(new Uint8List(20).toString());
inlined_string_no_bounds_check(new Uint8List(20).toString());
}

void extractAllInstructions(dynamic data, List<String> into) {
if (data is Map) {
for (var entry in data.entries) {
if (entry.key == "o" && entry.value is String) {
into.add(entry.value);
} else {
extractAllInstructions(entry.value, into);
}
}
} else if (data is List) {
for (var entry in data) {
extractAllInstructions(entry, into);
}
} else {
if (data is int || data is String) {
// ok
} else {
print("Notice: Unhandled data: ${data.runtimeType}: $data");
}
}
}

bool hasCheckBounds(FlowGraph graph) {
List<String> ils = [];
extractAllInstructions(graph.blocks(), ils);
for (String il in ils) {
if (il == "GenericCheckBound") return true;
}
return false;
}

void matchIL$direct_uint8list_has_bounds_check(FlowGraph graph) {
Expect.isTrue(hasCheckBounds(graph), "should have bounds checks");
}

void matchIL$direct_uint8list_no_bounds_check(FlowGraph graph) {
Expect.isFalse(hasCheckBounds(graph), "should not have bounds checks");
}

void matchIL$inlined_uint8list_has_bounds_check(FlowGraph graph) {
Expect.isTrue(hasCheckBounds(graph), "should have bounds checks");
}

void matchIL$inlined_uint8list_no_bounds_check(FlowGraph graph) {
Expect.isFalse(hasCheckBounds(graph), "should not have bounds checks");
}

void matchIL$direct_string_has_bounds_check(FlowGraph graph) {
Expect.isTrue(hasCheckBounds(graph), "should have bounds checks");
}

void matchIL$direct_string_no_bounds_check(FlowGraph graph) {
Expect.isFalse(hasCheckBounds(graph), "should not have bounds checks");
}

void matchIL$inlined_string_has_bounds_check(FlowGraph graph) {
Expect.isTrue(hasCheckBounds(graph), "should have bounds checks");
}

void matchIL$inlined_string_no_bounds_check(FlowGraph graph) {
Expect.isFalse(hasCheckBounds(graph), "should not have bounds checks");
}
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/inliner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1312,7 +1312,7 @@ class CallSiteInliner : public ValueObject {
exit_collector,
/*optimized=*/true, Compiler::kNoOSRDeoptId,
caller_graph_->max_block_id() + 1,
entry_kind == Code::EntryKind::kUnchecked);
entry_kind == Code::EntryKind::kUnchecked, &call_data->caller);
{
COMPILER_TIMINGS_TIMER_SCOPE(thread(), BuildGraph);
callee_graph = builder.BuildGraph();
Expand Down
43 changes: 38 additions & 5 deletions runtime/vm/compiler/frontend/base_flow_graph_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "vm/compiler/backend/range_analysis.h" // For Range.
#include "vm/compiler/frontend/flow_graph_builder.h" // For InlineExitCollector.
#include "vm/compiler/frontend/kernel_to_il.h" // For FlowGraphBuilder.
#include "vm/compiler/frontend/kernel_translation_helper.h"
#include "vm/compiler/jit/compiler.h" // For Compiler::IsBackgroundCompilation().
#include "vm/compiler/runtime_api.h"
Expand Down Expand Up @@ -223,6 +224,29 @@ Fragment BaseFlowGraphBuilder::Return(TokenPosition position) {
return instructions.closed();
}

bool BaseFlowGraphBuilder::ShouldOmitCheckBoundsIn(const Function& function,
const Function* caller) {
auto& state = CompilerState::Current();
if (!state.is_aot()) {
return false;
}

// The function itself is annotated with pragma.
if (state.PragmasOf(function).unsafe_no_bounds_checks) {
return true;
}

// We are inlining recognized method and the caller
// is annotated with pragma.
if (caller != nullptr &&
FlowGraphBuilder::IsRecognizedMethodForFlowGraph(function) &&
state.PragmasOf(*caller).unsafe_no_bounds_checks) {
return true;
}

return false;
}

bool BaseFlowGraphBuilder::ShouldOmitStackOverflowChecks(
bool optimizing,
const Function& function) {
Expand Down Expand Up @@ -420,11 +444,20 @@ Fragment BaseFlowGraphBuilder::LoadIndexed(classid_t class_id,
}

Fragment BaseFlowGraphBuilder::GenericCheckBound() {
Value* index = Pop();
Value* length = Pop();
auto* instr = new (Z) GenericCheckBoundInstr(length, index, GetNextDeoptId());
Push(instr);
return Fragment(instr);
// Consume deopt_id even if not inserting the instruction to avoid
// problems with JIT (even though should_omit_check_bounds() will be false
// in JIT).
const intptr_t deopt_id = GetNextDeoptId();
if (should_omit_check_bounds()) {
// Drop length but preserve index.
return DropTempsPreserveTop(/*num_temps_to_drop=*/1);
} else {
Value* index = Pop();
Value* length = Pop();
auto* instr = new (Z) GenericCheckBoundInstr(length, index, deopt_id);
Push(instr);
return Fragment(instr);
}
}

Fragment BaseFlowGraphBuilder::LoadUntagged(intptr_t offset) {
Expand Down
10 changes: 9 additions & 1 deletion runtime/vm/compiler/frontend/base_flow_graph_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,16 @@ class BaseFlowGraphBuilder {
intptr_t osr_id = DeoptId::kNone,
ZoneGrowableArray<intptr_t>* context_level_array = nullptr,
InlineExitCollector* exit_collector = nullptr,
bool inlining_unchecked_entry = false)
bool inlining_unchecked_entry = false,
const Function* caller = nullptr)
: parsed_function_(parsed_function),
function_(parsed_function_->function()),
optimizing_(optimizing),
should_omit_stack_overflow_checks_(
ShouldOmitStackOverflowChecks(optimizing,
parsed_function->function())),
should_omit_check_bounds_(
ShouldOmitCheckBoundsIn(parsed_function->function(), caller)),
thread_(Thread::Current()),
zone_(thread_->zone()),
osr_id_(osr_id),
Expand Down Expand Up @@ -516,17 +519,22 @@ class BaseFlowGraphBuilder {
Fragment RecordCoverageImpl(TokenPosition position, bool is_branch_coverage);
intptr_t GetCoverageIndexFor(intptr_t encoded_position);

static bool ShouldOmitCheckBoundsIn(const Function& function,
const Function* caller);

static bool ShouldOmitStackOverflowChecks(bool optimizing,
const Function& function);

bool should_omit_stack_overflow_checks() const {
return should_omit_stack_overflow_checks_;
}
bool should_omit_check_bounds() const { return should_omit_check_bounds_; }

const ParsedFunction* parsed_function_;
const Function& function_;
const bool optimizing_;
const bool should_omit_stack_overflow_checks_;
const bool should_omit_check_bounds_;

Thread* thread_;
Zone* zone_;
Expand Down
6 changes: 4 additions & 2 deletions runtime/vm/compiler/frontend/kernel_to_il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,16 @@ FlowGraphBuilder::FlowGraphBuilder(
bool optimizing,
intptr_t osr_id,
intptr_t first_block_id,
bool inlining_unchecked_entry)
bool inlining_unchecked_entry,
const Function* caller)
: BaseFlowGraphBuilder(parsed_function,
optimizing,
first_block_id - 1,
osr_id,
context_level_array,
exit_collector,
inlining_unchecked_entry),
inlining_unchecked_entry,
caller),
translation_helper_(Thread::Current()),
thread_(translation_helper_.thread()),
zone_(translation_helper_.zone()),
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/compiler/frontend/kernel_to_il.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
bool optimizing,
intptr_t osr_id,
intptr_t first_block_id = 1,
bool inlining_unchecked_entry = false);
bool inlining_unchecked_entry = false,
const Function* caller = nullptr);
virtual ~FlowGraphBuilder();

FlowGraph* BuildGraph();
Expand Down
7 changes: 7 additions & 0 deletions tests/web/wasm/uri_base_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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.

void main() {
print(Uri.base.toFilePath()); // should not crash
}
1 change: 1 addition & 0 deletions tests/web/web.status
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ code_motion_exception_test: Skip # Required V8 specific format of JavaScript err
[ $compiler == dart2wasm && $runtime != d8 ]
wasm/source_map_simple_test: SkipByDesign # Reads source map file using d8's readbuffer
wasm/source_map_simple_optimized_test: SkipByDesign # Reads source map file using d8's readbuffer
wasm/uri_base_test: SkipByDesign # Converts Uri.base to file path

[ $compiler == dartk && $runtime == vm ]
new_from_env_test: SkipByDesign # dart2js only test
Expand Down
2 changes: 1 addition & 1 deletion tools/VERSION
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ CHANNEL dev
MAJOR 3
MINOR 6
PATCH 0
PRERELEASE 272
PRERELEASE 273
PRERELEASE_PATCH 0

0 comments on commit 24ed9af

Please sign in to comment.