Skip to content

Commit a0c4ac1

Browse files
aartbikcommit-bot@chromium.org
authored andcommitted
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). #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 <[email protected]> Reviewed-by: Vyacheslav Egorov <[email protected]>
1 parent 2ad29cb commit a0c4ac1

File tree

1 file changed

+59
-45
lines changed

1 file changed

+59
-45
lines changed

runtime/vm/compiler/backend/inliner.cc

Lines changed: 59 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2605,51 +2605,55 @@ static void PrepareInlineByteArrayBaseOp(FlowGraph* flow_graph,
26052605
intptr_t view_cid,
26062606
Definition** array,
26072607
Definition* byte_index,
2608-
Instruction** cursor) {
2609-
LoadFieldInstr* length = new (Z) LoadFieldInstr(
2610-
new (Z) Value(*array), CheckArrayBoundInstr::LengthOffsetFor(array_cid),
2611-
Type::ZoneHandle(Z, Type::SmiType()), call->token_pos());
2612-
length->set_is_immutable(true);
2613-
length->set_result_cid(kSmiCid);
2614-
length->set_recognized_kind(
2615-
LoadFieldInstr::RecognizedKindFromArrayCid(array_cid));
2616-
*cursor = flow_graph->AppendTo(*cursor, length, NULL, FlowGraph::kValue);
2617-
2618-
intptr_t element_size = Instance::ElementSizeFor(array_cid);
2619-
ConstantInstr* bytes_per_element =
2620-
flow_graph->GetConstant(Smi::Handle(Z, Smi::New(element_size)));
2621-
BinarySmiOpInstr* len_in_bytes = new (Z)
2622-
BinarySmiOpInstr(Token::kMUL, new (Z) Value(length),
2623-
new (Z) Value(bytes_per_element), call->deopt_id());
2624-
*cursor = flow_graph->AppendTo(*cursor, len_in_bytes, call->env(),
2625-
FlowGraph::kValue);
2626-
2627-
// adjusted_length = len_in_bytes - (element_size - 1).
2628-
Definition* adjusted_length = len_in_bytes;
2629-
intptr_t adjustment = Instance::ElementSizeFor(view_cid) - 1;
2630-
if (adjustment > 0) {
2631-
ConstantInstr* length_adjustment =
2632-
flow_graph->GetConstant(Smi::Handle(Z, Smi::New(adjustment)));
2633-
adjusted_length = new (Z)
2634-
BinarySmiOpInstr(Token::kSUB, new (Z) Value(len_in_bytes),
2635-
new (Z) Value(length_adjustment), call->deopt_id());
2636-
*cursor = flow_graph->AppendTo(*cursor, adjusted_length, call->env(),
2608+
Instruction** cursor,
2609+
bool needs_bounds_check) {
2610+
if (needs_bounds_check) {
2611+
LoadFieldInstr* length = new (Z) LoadFieldInstr(
2612+
new (Z) Value(*array), CheckArrayBoundInstr::LengthOffsetFor(array_cid),
2613+
Type::ZoneHandle(Z, Type::SmiType()), call->token_pos());
2614+
length->set_is_immutable(true);
2615+
length->set_result_cid(kSmiCid);
2616+
length->set_recognized_kind(
2617+
LoadFieldInstr::RecognizedKindFromArrayCid(array_cid));
2618+
*cursor = flow_graph->AppendTo(*cursor, length, NULL, FlowGraph::kValue);
2619+
2620+
intptr_t element_size = Instance::ElementSizeFor(array_cid);
2621+
ConstantInstr* bytes_per_element =
2622+
flow_graph->GetConstant(Smi::Handle(Z, Smi::New(element_size)));
2623+
BinarySmiOpInstr* len_in_bytes = new (Z)
2624+
BinarySmiOpInstr(Token::kMUL, new (Z) Value(length),
2625+
new (Z) Value(bytes_per_element), call->deopt_id());
2626+
*cursor = flow_graph->AppendTo(*cursor, len_in_bytes, call->env(),
26372627
FlowGraph::kValue);
2638-
}
26392628

2640-
// Check adjusted_length > 0.
2641-
ConstantInstr* zero = flow_graph->GetConstant(Smi::Handle(Z, Smi::New(0)));
2642-
*cursor = flow_graph->AppendTo(
2643-
*cursor,
2644-
new (Z) CheckArrayBoundInstr(new (Z) Value(adjusted_length),
2645-
new (Z) Value(zero), call->deopt_id()),
2646-
call->env(), FlowGraph::kEffect);
2647-
// Check 0 <= byte_index < adjusted_length.
2648-
*cursor = flow_graph->AppendTo(
2649-
*cursor,
2650-
new (Z) CheckArrayBoundInstr(new (Z) Value(adjusted_length),
2651-
new (Z) Value(byte_index), call->deopt_id()),
2652-
call->env(), FlowGraph::kEffect);
2629+
// adjusted_length = len_in_bytes - (element_size - 1).
2630+
Definition* adjusted_length = len_in_bytes;
2631+
intptr_t adjustment = Instance::ElementSizeFor(view_cid) - 1;
2632+
if (adjustment > 0) {
2633+
ConstantInstr* length_adjustment =
2634+
flow_graph->GetConstant(Smi::Handle(Z, Smi::New(adjustment)));
2635+
adjusted_length = new (Z)
2636+
BinarySmiOpInstr(Token::kSUB, new (Z) Value(len_in_bytes),
2637+
new (Z) Value(length_adjustment), call->deopt_id());
2638+
*cursor = flow_graph->AppendTo(*cursor, adjusted_length, call->env(),
2639+
FlowGraph::kValue);
2640+
}
2641+
2642+
// Check adjusted_length > 0.
2643+
ConstantInstr* zero = flow_graph->GetConstant(Smi::Handle(Z, Smi::New(0)));
2644+
*cursor = flow_graph->AppendTo(
2645+
*cursor,
2646+
new (Z) CheckArrayBoundInstr(new (Z) Value(adjusted_length),
2647+
new (Z) Value(zero), call->deopt_id()),
2648+
call->env(), FlowGraph::kEffect);
2649+
// Check 0 <= byte_index < adjusted_length.
2650+
*cursor = flow_graph->AppendTo(
2651+
*cursor,
2652+
new (Z)
2653+
CheckArrayBoundInstr(new (Z) Value(adjusted_length),
2654+
new (Z) Value(byte_index), call->deopt_id()),
2655+
call->env(), FlowGraph::kEffect);
2656+
}
26532657

26542658
if (RawObject::IsExternalTypedDataClassId(array_cid)) {
26552659
LoadUntaggedInstr* elements = new (Z) LoadUntaggedInstr(
@@ -2679,8 +2683,13 @@ static bool InlineByteArrayBaseLoad(FlowGraph* flow_graph,
26792683
(*entry)->InheritDeoptTarget(Z, call);
26802684
Instruction* cursor = *entry;
26812685

2686+
// All getters that go through InlineByteArrayBaseLoad() have explicit
2687+
// bounds checks in all their clients in the library, so we can omit
2688+
// yet another inlined bounds check when compiling for Dart2.
2689+
const bool needs_bounds_check = !FLAG_strong;
2690+
26822691
PrepareInlineByteArrayBaseOp(flow_graph, call, array_cid, view_cid, &array,
2683-
index, &cursor);
2692+
index, &cursor, needs_bounds_check);
26842693

26852694
intptr_t deopt_id = Thread::kNoDeoptId;
26862695
if ((array_cid == kTypedDataInt32ArrayCid) ||
@@ -2726,8 +2735,13 @@ static bool InlineByteArrayBaseStore(FlowGraph* flow_graph,
27262735
(*entry)->InheritDeoptTarget(Z, call);
27272736
Instruction* cursor = *entry;
27282737

2738+
// All setters that go through InlineByteArrayBaseStore() have explicit
2739+
// bounds checks in all their clients in the library, so we can omit
2740+
// yet another inlined bounds check when compiling for Dart2.
2741+
const bool needs_bounds_check = !FLAG_strong;
2742+
27292743
PrepareInlineByteArrayBaseOp(flow_graph, call, array_cid, view_cid, &array,
2730-
index, &cursor);
2744+
index, &cursor, needs_bounds_check);
27312745

27322746
Cids* value_check = NULL;
27332747
switch (view_cid) {

0 commit comments

Comments
 (0)