Reland "Suppress canonicalization of Unbox() instruction that can deoptimize."

runtime/vm/intermediate_language.cc

 // Copyright (c) 2013, 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/intermediate_language.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cpu.h"
 #include "vm/dart_entry.h"
 #include "vm/flow_graph_allocator.h"
@@ skipping 771 matching lines @@
       if (curr == this) return true;
     }
 
     return false;
   }
 
   return dom_block->Dominates(block);
 }
 
 
+bool Instruction::HasUnmatchedInputRepresentations() const {
+  for (intptr_t i = 0; i < InputCount(); i++) {
+    Definition* input = InputAt(i)->definition();
+    if (RequiredInputRepresentation(i) != input->representation()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+
 void Definition::ReplaceWith(Definition* other,
                              ForwardInstructionIterator* iterator) {
   // Record other's input uses.
   for (intptr_t i = other->InputCount() - 1; i >= 0; --i) {
     Value* input = other->InputAt(i);
     input->definition()->AddInputUse(input);
   }
   // Take other's environment from this definition.
   ASSERT(other->env() == NULL);
   other->SetEnvironment(env());
@@ skipping 396 matching lines @@
   switch (op_kind()) {
     case Token::kBIT_AND:
     case Token::kBIT_OR:
     case Token::kBIT_XOR:
       return false;
 
     case Token::kSHR:
       return false;
 
     case Token::kSHL:
-      return true;
+      return can_overflow() ||
+          !RangeUtils::IsPositive(right()->definition()->range());
 
     case Token::kMOD: {
       UNREACHABLE();
     }
 
     default:
       return can_overflow();
   }
 }
 
 
 bool BinarySmiOpInstr::CanDeoptimize() const {
   if (FLAG_throw_on_javascript_int_overflow && (Smi::kBits > 32)) {
     // If Smi's are bigger than 32-bits, then the instruction could deoptimize
     // if the result is too big.
     return true;
   }
   switch (op_kind()) {
     case Token::kBIT_AND:
     case Token::kBIT_OR:
     case Token::kBIT_XOR:
       return false;
-    case Token::kSHR: {
-      // Can't deopt if shift-count is known positive.
-      Range* right_range = this->right()->definition()->range();
-      return (right_range == NULL) || !right_range->IsPositive();
-    }
-    case Token::kSHL: {
-      Range* right_range = this->right()->definition()->range();
-      if ((right_range != NULL) && !can_overflow()) {
-        // Can deoptimize if right can be negative.
-        return !right_range->IsPositive();
-      }
-      return true;
-    }
+
+    case Token::kSHR:
+      return !RangeUtils::IsPositive(right()->definition()->range());
+
+    case Token::kSHL:
+      return can_overflow() ||
+          !RangeUtils::IsPositive(right()->definition()->range());
+
     case Token::kMOD: {
       Range* right_range = this->right()->definition()->range();
       return (right_range == NULL) || right_range->Overlaps(0, 0);
     }
     default:
       return can_overflow();
   }
 }
 
 
@@ skipping 733 matching lines @@
     }
 
     return replacement;
   }
 
   return this;
 }
 
 
 Definition* UnboxInstr::Canonicalize(FlowGraph* flow_graph) {
-  if (!HasUses()) return NULL;
+  if (!HasUses() && !CanDeoptimize()) return NULL;
 
   // Fold away Unbox<rep>(Box<rep>(v)).
   BoxInstr* box_defn = value()->definition()->AsBox();
   if ((box_defn != NULL) &&
       (box_defn->from_representation() == representation())) {
     return box_defn->value()->definition();
   }
 
   if ((representation() == kUnboxedDouble) && value()->BindsToConstant()) {
     UnboxedConstantInstr* uc = NULL;
@@ skipping 11 matching lines @@
       flow_graph->InsertBefore(this, uc, NULL, FlowGraph::kValue);
       return uc;
     }
   }
 
   return this;
 }
 
 
 Definition* UnboxIntegerInstr::Canonicalize(FlowGraph* flow_graph) {
-  if (!HasUses()) return NULL;
+  if (!HasUses() && !CanDeoptimize()) return NULL;
 
   // Fold away UnboxInteger<rep_to>(BoxInteger<rep_from>(v)).
   BoxIntegerInstr* box_defn = value()->definition()->AsBoxInteger();
   if (box_defn != NULL) {
     if (box_defn->value()->definition()->representation() == representation()) {
       return box_defn->value()->definition();
     } else {
       UnboxedIntConverterInstr* converter = new UnboxedIntConverterInstr(
           box_defn->value()->definition()->representation(),
           representation(),
           box_defn->value()->CopyWithType(),
           (representation() == kUnboxedInt32) ?
               GetDeoptId() : Isolate::kNoDeoptId);
-      if ((representation() == kUnboxedInt32) && is_truncating()) {
+      if ((representation() == kUnboxedInt32) && !CanDeoptimize()) {
         converter->mark_truncating();
       }
       flow_graph->InsertBefore(this, converter, env(), FlowGraph::kValue);
       return converter;
     }
   }
 
   return this;
 }
 
@@ skipping 10 matching lines @@
       // Check that constant fits into 32-bit integer.
       const int64_t value =
           static_cast<int64_t>(Smi::Cast(c->value()).Value());
       if (!Utils::IsInt(32, value)) {
         return this;
       }
     }
 
     UnboxedConstantInstr* uc =
         new UnboxedConstantInstr(c->value(), kUnboxedInt32);
+    if (c->range() != NULL) {
+      uc->set_range(*c->range());
+    }
     flow_graph->InsertBefore(this, uc, NULL, FlowGraph::kValue);
     return uc;
   }
 
   return this;
 }
 
 
 Definition* UnboxedIntConverterInstr::Canonicalize(FlowGraph* flow_graph) {
   if (!HasUses()) return NULL;
@@ skipping 159 matching lines @@
   Isolate* isolate = flow_graph->isolate();
   // Only handle strict-compares.
   if (comparison()->IsStrictCompare()) {
     bool negated = false;
     Definition* replacement =
         CanonicalizeStrictCompare(comparison()->AsStrictCompare(), &negated);
     if (replacement == comparison()) {
       return this;
     }
     ComparisonInstr* comp = replacement->AsComparison();
-    if ((comp == NULL) || comp->CanDeoptimize()) {
+    if ((comp == NULL) ||
+        comp->CanDeoptimize() ||
+        comp->HasUnmatchedInputRepresentations()) {
       return this;
     }
 
-    // Assert that the comparison is not serving as a pending deoptimization
-    // target for conversions.
-    for (intptr_t i = 0; i < comp->InputCount(); i++) {
-      if (comp->RequiredInputRepresentation(i) !=
-          comp->InputAt(i)->definition()->representation()) {
-        return this;
-      }
-    }
-
     // Replace the comparison if the replacement is used at this branch,
     // and has exactly one use.
     Value* use = comp->input_use_list();
     if ((use->instruction() == this) && comp->HasOnlyUse(use)) {
       if (negated) {
         comp->NegateComparison();
       }
       RemoveEnvironment();
       flow_graph->CopyDeoptTarget(this, comp);
       // Unlink environment from the comparison since it is copied to the
@@ skipping 1089 matching lines @@
     case Token::kTRUNCDIV: return 0;
     case Token::kMOD: return 1;
     default: UNIMPLEMENTED(); return -1;
   }
 }
 
 
 #undef __
 
 }  // namespace dart