mirror of
https://github.com/dart-lang/sdk
synced 2024-09-16 02:27:39 +00:00
[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>
This commit is contained in:
parent
ee0952d8f5
commit
a3767f7db8
27
runtime/tests/vm/dart/regress_flutter83094_test.dart
Normal file
27
runtime/tests/vm/dart/regress_flutter83094_test.dart
Normal file
|
@ -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));
|
||||
}
|
27
runtime/tests/vm/dart_2/regress_flutter83094_test.dart
Normal file
27
runtime/tests/vm/dart_2/regress_flutter83094_test.dart
Normal file
|
@ -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));
|
||||
}
|
|
@ -1869,7 +1869,6 @@ void FlowGraph::InsertConversion(Representation from,
|
|||
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 @@ void FlowGraph::InsertConversion(Representation from,
|
|||
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 @@ void FlowGraph::InsertConversion(Representation from,
|
|||
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 @@ void FlowGraph::InsertConversion(Representation from,
|
|||
// 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) {
|
||||
|
|
|
@ -2481,9 +2481,12 @@ class PhiInstr : public Definition {
|
|||
|
||||
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 @@ class GotoInstr : public TemplateInstruction<0, NoThrow> {
|
|||
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 @@ class InstanceCallBaseInstr : public TemplateDartCall<0> {
|
|||
}
|
||||
idx--;
|
||||
}
|
||||
if (interface_target_.IsNull()) return kGuardInputs;
|
||||
return interface_target_.is_unboxed_parameter_at(idx) ? kNotSpeculative
|
||||
: kGuardInputs;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue