VM: Fix bug in type propagation at conditionals.
Propagating type/cid at conditional branches is not possible because
it may cause invalid code motion.
For this to be safe we need to explicitly represent the dependency
between checks eliminated in a branch and the condition that constrains the type/cid.
BUG=
R=vegorov@google.com
Review URL: https://codereview.chromium.org/1491373005 .
diff --git a/runtime/vm/flow_graph_type_propagator.cc b/runtime/vm/flow_graph_type_propagator.cc
index 3448fd0..29ec735 100644
--- a/runtime/vm/flow_graph_type_propagator.cc
+++ b/runtime/vm/flow_graph_type_propagator.cc
@@ -149,8 +149,6 @@
}
}
- HandleBranchOnStrictCompare(block);
-
for (intptr_t i = 0; i < block->dominated_blocks().length(); ++i) {
PropagateRecursive(block->dominated_blocks()[i]);
}
@@ -159,61 +157,6 @@
}
-void FlowGraphTypePropagator::HandleBranchOnStrictCompare(
- BlockEntryInstr* block) {
- BranchInstr* branch = block->last_instruction()->AsBranch();
- if (branch == NULL) {
- return;
- }
-
- StrictCompareInstr* compare = branch->comparison()->AsStrictCompare();
- if ((compare == NULL) || !compare->right()->BindsToConstant()) {
- return;
- }
-
- const intptr_t rollback_point = rollback_.length();
-
- Definition* defn = compare->left()->definition();
- const Object& right = compare->right()->BoundConstant();
- intptr_t cid = right.GetClassId();
-
- if (defn->IsLoadClassId() && right.IsSmi()) {
- defn = defn->AsLoadClassId()->object()->definition();
- cid = Smi::Cast(right).Value();
- }
-
- if (!CheckClassInstr::IsImmutableClassId(cid)) {
- if ((cid == kOneByteStringCid) || (cid == kTwoByteStringCid)) {
- SetTypeOf(defn, ZoneCompileType::Wrap(CompileType::String()));
- PropagateRecursive((compare->kind() == Token::kEQ_STRICT) ?
- branch->true_successor() : branch->false_successor());
- RollbackTo(rollback_point);
- }
- return;
- }
-
- if (compare->kind() == Token::kEQ_STRICT) {
- if (cid == kNullCid) {
- branch->set_constrained_type(MarkNonNullable(defn));
- PropagateRecursive(branch->false_successor());
- }
-
- SetCid(defn, cid);
- PropagateRecursive(branch->true_successor());
- } else if (compare->kind() == Token::kNE_STRICT) {
- if (cid == kNullCid) {
- branch->set_constrained_type(MarkNonNullable(defn));
- PropagateRecursive(branch->true_successor());
- }
-
- SetCid(defn, cid);
- PropagateRecursive(branch->false_successor());
- }
-
- RollbackTo(rollback_point);
-}
-
-
void FlowGraphTypePropagator::RollbackTo(intptr_t rollback_point) {
for (intptr_t i = rollback_.length() - 1; i >= rollback_point; i--) {
types_[rollback_[i].index()] = rollback_[i].type();
diff --git a/runtime/vm/flow_graph_type_propagator.h b/runtime/vm/flow_graph_type_propagator.h
index 4b1baf4..3c10917 100644
--- a/runtime/vm/flow_graph_type_propagator.h
+++ b/runtime/vm/flow_graph_type_propagator.h
@@ -20,7 +20,6 @@
void Propagate();
void PropagateRecursive(BlockEntryInstr* block);
- void HandleBranchOnStrictCompare(BlockEntryInstr* block);
void RollbackTo(intptr_t rollback_point);
diff --git a/tests/language/vm/type_propagation_test.dart b/tests/language/vm/type_propagation_test.dart
new file mode 100644
index 0000000..935c096
--- /dev/null
+++ b/tests/language/vm/type_propagation_test.dart
@@ -0,0 +1,56 @@
+// Copyright (c) 2015, 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.
+// VMOptions=--optimization-counter-threshold=1000 --max-polymorphic-checks=1
+
+// Test correct loop invariant code motion and type propagation from is-checks
+// and null-comparisons.
+
+class B {
+ var b;
+ B(this.b);
+}
+
+class C {
+ final f0;
+
+ final a;
+ C() : a = new B(0);
+}
+
+foo(x) {
+ for (var i = 0; i < 10; i++) {
+ i + i;
+ i + i;
+ if (x is C) {
+ x.a.b < 0;
+ }
+ }
+}
+
+class Y { var f = null; }
+
+bar(y) {
+ var x = y.f;
+ for (var i = 0; i < 10; i++) {
+ if (x != null) {
+ x.a.b < 0;
+ }
+ }
+}
+
+
+main () {
+ var o = new Y();
+ o.f = new C();
+ bar(o);
+ o.f = null;
+ bar(o);
+
+ for (var i = 0; i < 1000; i++) bar(o);
+
+ foo(new C());
+ foo(0);
+
+ for (var i = 0; i < 1000; i++) foo(0);
+}
