[vm/compiler] Generate non-speculative Unbox instructions for Phis
TEST=runtime/tests/vm/dart/regress_flutter83094_test.dart
Issue https://github.com/flutter/flutter/issues/83094
Change-Id: Ib4eebc993e06f6925f11bd18e5f29f22ba3c6322
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201363
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/tests/vm/dart/regress_flutter83094_test.dart b/runtime/tests/vm/dart/regress_flutter83094_test.dart
new file mode 100644
index 0000000..2572019
--- /dev/null
+++ b/runtime/tests/vm/dart/regress_flutter83094_test.dart
@@ -0,0 +1,27 @@
+// 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.
+
+// Verifies that compiler doesn't crash due to incompatible types
+// when unboxing input of a Phi.
+// Regression test for https://github.com/flutter/flutter/issues/83094.
+
+import 'package:expect/expect.dart';
+
+class A {
+ @pragma('vm:never-inline')
+ double getMaxIntrinsicWidth() => 1.toDouble();
+}
+
+A _leading = A();
+
+@pragma('vm:never-inline')
+double computeMaxIntrinsicWidth(double height, double horizontalPadding) {
+ final leadingWidth =
+ _leading == null ? 0 : _leading.getMaxIntrinsicWidth() as int;
+ return horizontalPadding + leadingWidth;
+}
+
+main() {
+ Expect.throws(() => computeMaxIntrinsicWidth(1, 2));
+}
diff --git a/runtime/tests/vm/dart_2/regress_flutter83094_test.dart b/runtime/tests/vm/dart_2/regress_flutter83094_test.dart
new file mode 100644
index 0000000..2572019
--- /dev/null
+++ b/runtime/tests/vm/dart_2/regress_flutter83094_test.dart
@@ -0,0 +1,27 @@
+// 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.
+
+// Verifies that compiler doesn't crash due to incompatible types
+// when unboxing input of a Phi.
+// Regression test for https://github.com/flutter/flutter/issues/83094.
+
+import 'package:expect/expect.dart';
+
+class A {
+ @pragma('vm:never-inline')
+ double getMaxIntrinsicWidth() => 1.toDouble();
+}
+
+A _leading = A();
+
+@pragma('vm:never-inline')
+double computeMaxIntrinsicWidth(double height, double horizontalPadding) {
+ final leadingWidth =
+ _leading == null ? 0 : _leading.getMaxIntrinsicWidth() as int;
+ return horizontalPadding + leadingWidth;
+}
+
+main() {
+ Expect.throws(() => computeMaxIntrinsicWidth(1, 2));
+}
diff --git a/runtime/vm/compiler/backend/flow_graph.cc b/runtime/vm/compiler/backend/flow_graph.cc
index a24af44..52c25a2 100644
--- a/runtime/vm/compiler/backend/flow_graph.cc
+++ b/runtime/vm/compiler/backend/flow_graph.cc
@@ -1869,7 +1869,6 @@
bool is_environment_use) {
ASSERT(from != to);
Instruction* insert_before;
- Instruction* deopt_target;
PhiInstr* phi = use->instruction()->AsPhi();
if (phi != NULL) {
ASSERT(phi->is_alive());
@@ -1877,14 +1876,19 @@
auto predecessor = phi->block()->PredecessorAt(use->use_index());
insert_before = predecessor->last_instruction();
ASSERT(insert_before->GetBlock() == predecessor);
- deopt_target = NULL;
} else {
- deopt_target = insert_before = use->instruction();
+ insert_before = use->instruction();
+ }
+ const Instruction::SpeculativeMode speculative_mode =
+ use->instruction()->SpeculativeModeOfInput(use->use_index());
+ Instruction* deopt_target = nullptr;
+ if (speculative_mode == Instruction::kGuardInputs || to == kUnboxedInt32) {
+ deopt_target = insert_before;
}
Definition* converted = NULL;
if (IsUnboxedInteger(from) && IsUnboxedInteger(to)) {
- const intptr_t deopt_id = (to == kUnboxedInt32) && (deopt_target != NULL)
+ const intptr_t deopt_id = (to == kUnboxedInt32) && (deopt_target != nullptr)
? deopt_target->DeoptimizationTarget()
: DeoptId::kNone;
converted =
@@ -1893,18 +1897,17 @@
converted = new Int32ToDoubleInstr(use->CopyWithType());
} else if ((from == kUnboxedInt64) && (to == kUnboxedDouble) &&
CanConvertInt64ToDouble()) {
- const intptr_t deopt_id = (deopt_target != NULL)
+ const intptr_t deopt_id = (deopt_target != nullptr)
? deopt_target->DeoptimizationTarget()
: DeoptId::kNone;
ASSERT(CanUnboxDouble());
converted = new Int64ToDoubleInstr(use->CopyWithType(), deopt_id);
} else if ((from == kTagged) && Boxing::Supports(to)) {
- const intptr_t deopt_id = (deopt_target != NULL)
+ const intptr_t deopt_id = (deopt_target != nullptr)
? deopt_target->DeoptimizationTarget()
: DeoptId::kNone;
- converted = UnboxInstr::Create(
- to, use->CopyWithType(), deopt_id,
- use->instruction()->SpeculativeModeOfInput(use->use_index()));
+ converted =
+ UnboxInstr::Create(to, use->CopyWithType(), deopt_id, speculative_mode);
} else if ((to == kTagged) && Boxing::Supports(from)) {
converted = BoxInstr::Create(from, use->CopyWithType());
} else {
@@ -1912,30 +1915,32 @@
// Insert two "dummy" conversion instructions with the correct
// "from" and "to" representation. The inserted instructions will
// trigger a deoptimization if executed. See #12417 for a discussion.
- const intptr_t deopt_id = (deopt_target != NULL)
+ // If the use is not speculative, then this code should be unreachable.
+ // Insert Stop for a graceful error and aid unreachable code elimination.
+ if (speculative_mode == Instruction::kNotSpeculative) {
+ StopInstr* stop = new (Z) StopInstr("Incompatible conversion.");
+ InsertBefore(insert_before, stop, nullptr, FlowGraph::kEffect);
+ }
+ const intptr_t deopt_id = (deopt_target != nullptr)
? deopt_target->DeoptimizationTarget()
: DeoptId::kNone;
ASSERT(Boxing::Supports(from));
ASSERT(Boxing::Supports(to));
Definition* boxed = BoxInstr::Create(from, use->CopyWithType());
use->BindTo(boxed);
- InsertBefore(insert_before, boxed, NULL, FlowGraph::kValue);
- converted = UnboxInstr::Create(to, new (Z) Value(boxed), deopt_id);
+ InsertBefore(insert_before, boxed, nullptr, FlowGraph::kValue);
+ converted = UnboxInstr::Create(to, new (Z) Value(boxed), deopt_id,
+ speculative_mode);
}
- ASSERT(converted != NULL);
- InsertBefore(insert_before, converted, use->instruction()->env(),
+ ASSERT(converted != nullptr);
+ InsertBefore(insert_before, converted,
+ (deopt_target != nullptr) ? deopt_target->env() : nullptr,
FlowGraph::kValue);
if (is_environment_use) {
use->BindToEnvironment(converted);
} else {
use->BindTo(converted);
}
-
- if ((to == kUnboxedInt32) && (phi != NULL)) {
- // Int32 phis are unboxed optimistically. Ensure that unboxing
- // has deoptimization target attached from the goto instruction.
- CopyDeoptTarget(converted, insert_before);
- }
}
void FlowGraph::InsertConversionsFor(Definition* def) {
diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h
index a53501e..823d9da 100644
--- a/runtime/vm/compiler/backend/il.h
+++ b/runtime/vm/compiler/backend/il.h
@@ -2481,9 +2481,12 @@
virtual void set_representation(Representation r) { representation_ = r; }
- // In AOT mode Phi instructions do not check types of inputs when unboxing.
+ // Only Int32 phis in JIT mode are unboxed optimistically.
virtual SpeculativeMode SpeculativeModeOfInput(intptr_t index) const {
- return CompilerState::Current().is_aot() ? kNotSpeculative : kGuardInputs;
+ return (CompilerState::Current().is_aot() ||
+ (representation_ != kUnboxedInt32))
+ ? kNotSpeculative
+ : kGuardInputs;
}
virtual uword Hash() const {
@@ -3140,6 +3143,9 @@
return true;
}
+ // May require a deoptimization target for int32 Phi input conversions.
+ virtual intptr_t DeoptimizationTarget() const { return GetDeoptId(); }
+
virtual bool ComputeCanDeoptimize() const { return false; }
virtual bool HasUnknownSideEffects() const { return false; }
@@ -4097,6 +4103,7 @@
}
idx--;
}
+ if (interface_target_.IsNull()) return kGuardInputs;
return interface_target_.is_unboxed_parameter_at(idx) ? kNotSpeculative
: kGuardInputs;
}
