Omit redundant bounds check for Dart2 on typed setters/getters.
Rationale:
Some native typed setters/getters in the typed_data library
are only called from clients that perform explicit bounds checks
first. Under strongly typed Dart2, there is no need to repeat
these tests. Avoids overhead, and reduces the polymorphicness
of these calls (preparing more inlining later).
https://github.com/dart-lang/sdk/issues/33205
Tests:
./tools/test.py --mode release --arch x64 -r vm
tools/test.py -m release -r vm -c dartk --strong standalone_2/typed_data_test
Change-Id: I81771a4f4b41a21e344f2d50745bbc30480b2a6c
Reviewed-on: https://dart-review.googlesource.com/57460
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
index 8ee80fe..5512de8 100644
--- a/runtime/vm/compiler/backend/inliner.cc
+++ b/runtime/vm/compiler/backend/inliner.cc
@@ -2605,51 +2605,55 @@
intptr_t view_cid,
Definition** array,
Definition* byte_index,
- Instruction** cursor) {
- LoadFieldInstr* length = new (Z) LoadFieldInstr(
- new (Z) Value(*array), CheckArrayBoundInstr::LengthOffsetFor(array_cid),
- Type::ZoneHandle(Z, Type::SmiType()), call->token_pos());
- length->set_is_immutable(true);
- length->set_result_cid(kSmiCid);
- length->set_recognized_kind(
- LoadFieldInstr::RecognizedKindFromArrayCid(array_cid));
- *cursor = flow_graph->AppendTo(*cursor, length, NULL, FlowGraph::kValue);
+ Instruction** cursor,
+ bool needs_bounds_check) {
+ if (needs_bounds_check) {
+ LoadFieldInstr* length = new (Z) LoadFieldInstr(
+ new (Z) Value(*array), CheckArrayBoundInstr::LengthOffsetFor(array_cid),
+ Type::ZoneHandle(Z, Type::SmiType()), call->token_pos());
+ length->set_is_immutable(true);
+ length->set_result_cid(kSmiCid);
+ length->set_recognized_kind(
+ LoadFieldInstr::RecognizedKindFromArrayCid(array_cid));
+ *cursor = flow_graph->AppendTo(*cursor, length, NULL, FlowGraph::kValue);
- intptr_t element_size = Instance::ElementSizeFor(array_cid);
- ConstantInstr* bytes_per_element =
- flow_graph->GetConstant(Smi::Handle(Z, Smi::New(element_size)));
- BinarySmiOpInstr* len_in_bytes = new (Z)
- BinarySmiOpInstr(Token::kMUL, new (Z) Value(length),
- new (Z) Value(bytes_per_element), call->deopt_id());
- *cursor = flow_graph->AppendTo(*cursor, len_in_bytes, call->env(),
- FlowGraph::kValue);
-
- // adjusted_length = len_in_bytes - (element_size - 1).
- Definition* adjusted_length = len_in_bytes;
- intptr_t adjustment = Instance::ElementSizeFor(view_cid) - 1;
- if (adjustment > 0) {
- ConstantInstr* length_adjustment =
- flow_graph->GetConstant(Smi::Handle(Z, Smi::New(adjustment)));
- adjusted_length = new (Z)
- BinarySmiOpInstr(Token::kSUB, new (Z) Value(len_in_bytes),
- new (Z) Value(length_adjustment), call->deopt_id());
- *cursor = flow_graph->AppendTo(*cursor, adjusted_length, call->env(),
+ intptr_t element_size = Instance::ElementSizeFor(array_cid);
+ ConstantInstr* bytes_per_element =
+ flow_graph->GetConstant(Smi::Handle(Z, Smi::New(element_size)));
+ BinarySmiOpInstr* len_in_bytes = new (Z)
+ BinarySmiOpInstr(Token::kMUL, new (Z) Value(length),
+ new (Z) Value(bytes_per_element), call->deopt_id());
+ *cursor = flow_graph->AppendTo(*cursor, len_in_bytes, call->env(),
FlowGraph::kValue);
- }
- // Check adjusted_length > 0.
- ConstantInstr* zero = flow_graph->GetConstant(Smi::Handle(Z, Smi::New(0)));
- *cursor = flow_graph->AppendTo(
- *cursor,
- new (Z) CheckArrayBoundInstr(new (Z) Value(adjusted_length),
- new (Z) Value(zero), call->deopt_id()),
- call->env(), FlowGraph::kEffect);
- // Check 0 <= byte_index < adjusted_length.
- *cursor = flow_graph->AppendTo(
- *cursor,
- new (Z) CheckArrayBoundInstr(new (Z) Value(adjusted_length),
- new (Z) Value(byte_index), call->deopt_id()),
- call->env(), FlowGraph::kEffect);
+ // adjusted_length = len_in_bytes - (element_size - 1).
+ Definition* adjusted_length = len_in_bytes;
+ intptr_t adjustment = Instance::ElementSizeFor(view_cid) - 1;
+ if (adjustment > 0) {
+ ConstantInstr* length_adjustment =
+ flow_graph->GetConstant(Smi::Handle(Z, Smi::New(adjustment)));
+ adjusted_length = new (Z)
+ BinarySmiOpInstr(Token::kSUB, new (Z) Value(len_in_bytes),
+ new (Z) Value(length_adjustment), call->deopt_id());
+ *cursor = flow_graph->AppendTo(*cursor, adjusted_length, call->env(),
+ FlowGraph::kValue);
+ }
+
+ // Check adjusted_length > 0.
+ ConstantInstr* zero = flow_graph->GetConstant(Smi::Handle(Z, Smi::New(0)));
+ *cursor = flow_graph->AppendTo(
+ *cursor,
+ new (Z) CheckArrayBoundInstr(new (Z) Value(adjusted_length),
+ new (Z) Value(zero), call->deopt_id()),
+ call->env(), FlowGraph::kEffect);
+ // Check 0 <= byte_index < adjusted_length.
+ *cursor = flow_graph->AppendTo(
+ *cursor,
+ new (Z)
+ CheckArrayBoundInstr(new (Z) Value(adjusted_length),
+ new (Z) Value(byte_index), call->deopt_id()),
+ call->env(), FlowGraph::kEffect);
+ }
if (RawObject::IsExternalTypedDataClassId(array_cid)) {
LoadUntaggedInstr* elements = new (Z) LoadUntaggedInstr(
@@ -2679,8 +2683,13 @@
(*entry)->InheritDeoptTarget(Z, call);
Instruction* cursor = *entry;
+ // All getters that go through InlineByteArrayBaseLoad() have explicit
+ // bounds checks in all their clients in the library, so we can omit
+ // yet another inlined bounds check when compiling for Dart2.
+ const bool needs_bounds_check = !FLAG_strong;
+
PrepareInlineByteArrayBaseOp(flow_graph, call, array_cid, view_cid, &array,
- index, &cursor);
+ index, &cursor, needs_bounds_check);
intptr_t deopt_id = Thread::kNoDeoptId;
if ((array_cid == kTypedDataInt32ArrayCid) ||
@@ -2726,8 +2735,13 @@
(*entry)->InheritDeoptTarget(Z, call);
Instruction* cursor = *entry;
+ // All setters that go through InlineByteArrayBaseStore() have explicit
+ // bounds checks in all their clients in the library, so we can omit
+ // yet another inlined bounds check when compiling for Dart2.
+ const bool needs_bounds_check = !FLAG_strong;
+
PrepareInlineByteArrayBaseOp(flow_graph, call, array_cid, view_cid, &array,
- index, &cursor);
+ index, &cursor, needs_bounds_check);
Cids* value_check = NULL;
switch (view_cid) {
