Fix bad optimization of instance-of with uninstantiated types (issue 5216).

runtime/vm/flow_graph_optimizer.cc

 // Copyright (c) 2012, 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.
 
 #include "vm/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/flow_graph_builder.h"
 #include "vm/hash_map.h"
@@ skipping 1164 matching lines @@
         still_changing_ = defn->SetPropagatedCid(cid) || still_changing_;
       }
     }
     current_iterator_ = NULL;
   }
 }
 
 
 void FlowGraphTypePropagator::VisitAssertAssignable(
     AssertAssignableInstr* instr) {
+  bool is_null, is_instance;
   if (FLAG_eliminate_type_checks &&
       !instr->is_eliminated() &&
-      instr->value()->CompileTypeIsMoreSpecificThan(instr->dst_type())) {
+      ((instr->value()->CanComputeIsNull(&is_null) && is_null) ||
+       (instr->value()->CanComputeIsInstanceOf(instr->dst_type(), &is_instance)
+        && is_instance))) {
     // TODO(regis): Remove is_eliminated_ field and support.
     instr->eliminate();
 
     Value* use = instr->value();
     ASSERT(use != NULL);
     Definition* result = use->definition();
     ASSERT(result != NULL);
     // Replace uses and remove the current instruction via the iterator.
     instr->ReplaceUsesWith(result);
     ASSERT(current_iterator()->Current() == instr);
@@ skipping 10 matching lines @@
                                        instr->value(),
                                        instr->dst_type(),
                                        instr->dst_name(),
                                        instr->is_eliminated());
     }
   }
 }
 
 
 void FlowGraphTypePropagator::VisitAssertBoolean(AssertBooleanInstr* instr) {
-  // TODO(regis): Propagate NullType as well and revise the comment and code
-  // below to also eliminate the test for non-null and non-constant value.
-
-  // We can only eliminate an 'assert boolean' test when the checked value is
-  // a constant time constant. Indeed, a variable of the proper compile time
-  // type (bool) may still hold null at run time and therefore fail the test.
+  bool is_null, is_bool;
   if (FLAG_eliminate_type_checks &&
       !instr->is_eliminated() &&
-      instr->value()->BindsToConstant() &&
-      !instr->value()->BindsToConstantNull() &&
-      instr->value()->CompileTypeIsMoreSpecificThan(
-          Type::Handle(Type::BoolType()))) {
+      instr->value()->CanComputeIsNull(&is_null) &&
+      !is_null &&
+      instr->value()->CanComputeIsInstanceOf(Type::Handle(Type::BoolType()),
+                                             &is_bool) &&
+      is_bool) {
     // TODO(regis): Remove is_eliminated_ field and support.
     instr->eliminate();
-
     Value* use = instr->value();
     Definition* result = use->definition();
     ASSERT(result != NULL);
     // Replace uses and remove the current instruction via the iterator.
     instr->ReplaceUsesWith(result);
     ASSERT(current_iterator()->Current() == instr);
     current_iterator()->RemoveCurrentFromGraph();
     if (FLAG_trace_optimization) {
       OS::Print("Replacing v%"Pd" with v%"Pd"\n",
                 instr->ssa_temp_index(),
                 result->ssa_temp_index());
     }
 
     if (FLAG_trace_type_check_elimination) {
       const String& name = String::Handle(Symbols::New("boolean expression"));
       FlowGraphPrinter::PrintTypeCheck(parsed_function(),
                                        instr->token_pos(),
                                        instr->value(),
                                        Type::Handle(Type::BoolType()),
                                        name,
                                        instr->is_eliminated());
     }
   }
 }
 
 
 void FlowGraphTypePropagator::VisitInstanceOf(InstanceOfInstr* instr) {
-  // TODO(regis): Propagate NullType as well and revise the comment and code
-  // below to also eliminate the test for non-null and non-constant value.
-
-  // We can only eliminate an 'instance of' test when the checked value is
-  // a constant time constant. Indeed, a variable of the proper compile time
-  // type may still hold null at run time and therefore fail the test.
-  // We do not bother checking for Object destination type, since the graph
-  // builder did already.
+  bool is_null;
+  bool is_instance = false;
   if (FLAG_eliminate_type_checks &&
-      instr->value()->BindsToConstant() &&
-      !instr->value()->BindsToConstantNull()) {
-    const Bool& bool_result =
-        instr->value()->CompileTypeIsMoreSpecificThan(instr->type()) ?
-            Bool::ZoneHandle(Bool::True()) : Bool::ZoneHandle(Bool::False());
-    Definition* result = new ConstantInstr(bool_result);
+      instr->value()->CanComputeIsNull(&is_null) &&
+      (is_null ||
+       instr->value()->CanComputeIsInstanceOf(instr->type(), &is_instance))) {
+    Definition* result = new ConstantInstr(Bool::ZoneHandle(Bool::Get(
+        instr->negate_result() ? !is_instance : is_instance)));
     result->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
     result->InsertBefore(instr);
     // Replace uses and remove the current instruction via the iterator.
     instr->ReplaceUsesWith(result);
     ASSERT(current_iterator()->Current() == instr);
     current_iterator()->RemoveCurrentFromGraph();
     if (FLAG_trace_optimization) {
       OS::Print("Replacing v%"Pd" with v%"Pd"\n",
                 instr->ssa_temp_index(),
                 result->ssa_temp_index());
@@ skipping 494 matching lines @@
       DirectChainedHashMap<Definition*> child_map(*map);  // Copy map.
       OptimizeRecursive(child, &child_map);
     } else {
       OptimizeRecursive(child, map);  // Reuse map for the last child.
     }
   }
 }
 
 
 }  // namespace dart