Skip to content

Commit

Permalink
[vm/aot] Prefer EqualityCompare to avoid redundant boxing
Browse files Browse the repository at this point in the history
When specializing x == y comparisons where one of the operands is known
to be a Smi but the other one is *not* only emit StrictCompare
if either x or y are nullable. For non-nullable equality comparisons
prefer EqualityCompare which allows to avoid redundant boxing as
non-nullable integer values are highly likely to be unboxed.

TEST=vm/dart{,_2}/aot_prefer_equality_comparison_il_test

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm_x64-try
Change-Id: I1153f111dcbb518d25d753dc4357c72522389951
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214962
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
  • Loading branch information
mraleph authored and commit-bot@chromium.org committed Sep 30, 2021
1 parent 8740a4f commit 62a903f
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 6 deletions.
18 changes: 18 additions & 0 deletions runtime/tests/vm/dart/aot_prefer_equality_comparison_il_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2021, 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.

// Test that we emit EqualityCompare rather than StrictCompare+BoxInt64
// when comparing non-nullable integer to a Smi.

// MatchIL[AOT]=factorial
// __ GraphEntry
// __ FunctionEntry
// __ CheckStackOverflow
// __ Branch(EqualityCompare)
@pragma('vm:never-inline')
int factorial(int value) => value == 1 ? value : value * factorial(value - 1);

void main() {
print(factorial(4));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2021, 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.

// Test that we emit EqualityCompare rather than StrictCompare+BoxInt64
// when comparing non-nullable integer to a Smi.

// MatchIL[AOT]=factorial
// __ GraphEntry
// __ FunctionEntry
// __ CheckStackOverflow
// __ Branch(EqualityCompare)
@pragma('vm:never-inline')
int factorial(int value) => value == 1 ? value : value * factorial(value - 1);

void main() {
print(factorial(4));
}
24 changes: 18 additions & 6 deletions runtime/vm/compiler/aot/aot_call_specializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -465,23 +465,35 @@ bool AotCallSpecializer::TryOptimizeIntegerOperation(TemplateDartCall<0>* instr,

switch (op_kind) {
case Token::kEQ:
case Token::kNE:
if (left_type->IsNull() || left_type->IsNullableSmi() ||
right_type->IsNull() || right_type->IsNullableSmi()) {
case Token::kNE: {
// If both arguments are nullable Smi or one of the arguments is
// a null or Smi and the other argument is nullable then emit
// StrictCompare (all arguments are going to be boxed anyway).
// Otherwise prefer EqualityCompare to avoid redundant boxing.
const bool left_is_null_or_smi =
left_type->IsNull() || left_type->IsNullableSmi();
const bool right_is_null_or_smi =
right_type->IsNull() || right_type->IsNullableSmi();
const bool both_are_nullable_smis =
left_type->IsNullableSmi() && right_type->IsNullableSmi();
const bool either_can_be_null =
left_type->is_nullable() || right_type->is_nullable();
if (both_are_nullable_smis ||
((left_is_null_or_smi || right_is_null_or_smi) &&
either_can_be_null)) {
replacement = new (Z) StrictCompareInstr(
instr->source(),
(op_kind == Token::kEQ) ? Token::kEQ_STRICT : Token::kNE_STRICT,
left_value->CopyWithType(Z), right_value->CopyWithType(Z),
/*needs_number_check=*/false, DeoptId::kNone);
} else {
const bool null_aware =
left_type->is_nullable() || right_type->is_nullable();
replacement = new (Z) EqualityCompareInstr(
instr->source(), op_kind, left_value->CopyWithType(Z),
right_value->CopyWithType(Z), kMintCid, DeoptId::kNone,
null_aware, Instruction::kNotSpeculative);
/*null_aware=*/either_can_be_null, Instruction::kNotSpeculative);
}
break;
}
case Token::kLT:
case Token::kLTE:
case Token::kGT:
Expand Down

0 comments on commit 62a903f

Please sign in to comment.