Inline toDouble calls on mints

runtime/vm/flow_graph_optimizer.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/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/cpu.h"
 #include "vm/dart_entry.h"
@@ skipping 620 matching lines @@
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
     converted = new UnboxInt32Instr(use->CopyWithType(), deopt_id);
   } else if ((from == kUnboxedInt32) && (to == kTagged)) {
     converted = new BoxInt32Instr(use->CopyWithType());
   } else if ((from == kTagged) && (to == kUnboxedUint32)) {
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
     converted = new UnboxUint32Instr(use->CopyWithType(), deopt_id);
   } else if (from == kUnboxedMint && to == kUnboxedDouble) {
     ASSERT(CanUnboxDouble());
-    // Convert by boxing/unboxing.
-    // TODO(fschneider): Implement direct unboxed mint-to-double conversion.
-    BoxIntegerInstr* boxed =
-        new(I) BoxIntegerInstr(use->CopyWithType());
-    use->BindTo(boxed);
-    InsertBefore(insert_before, boxed, NULL, FlowGraph::kValue);
-
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
-    converted = new(I) UnboxDoubleInstr(new(I) Value(boxed), deopt_id);
-
+    converted = new MintToDoubleInstr(use->CopyWithType(), deopt_id);
   } else if ((from == kUnboxedDouble) && (to == kTagged)) {
     ASSERT(CanUnboxDouble());
     converted = new(I) BoxDoubleInstr(use->CopyWithType());
-
   } else if ((from == kTagged) && (to == kUnboxedDouble)) {
     ASSERT(CanUnboxDouble());
     ASSERT((deopt_target != NULL) ||
            (use->Type()->ToCid() == kDoubleCid));
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
     ConstantInstr* constant = use->definition()->AsConstant();
     if ((constant != NULL) && constant->value().IsSmi()) {
       const double dbl_val = Smi::Cast(constant->value()).AsDoubleValue();
       const Double& dbl_obj =
@@ skipping 2378 matching lines @@
           call->deopt_id(),
           call->token_pos());
       ReplaceCall(call, store_op);
       return true;
     }
     return false;
   }
 
   if (CanUnboxDouble() &&
       (recognized_kind == MethodRecognizer::kIntegerToDouble) &&
-      (ic_data.NumberOfChecks() == 1) &&
-      (class_ids[0] == kSmiCid)) {
-    AddReceiverCheck(call);
-    ReplaceCall(call,
-                new(I) SmiToDoubleInstr(
-                    new(I) Value(call->ArgumentAt(0)),
-                    call->token_pos()));
-    return true;
+      (ic_data.NumberOfChecks() == 1)) {
+    if (class_ids[0] == kSmiCid) {
+      AddReceiverCheck(call);
+      ReplaceCall(call,
+                  new(I) SmiToDoubleInstr(
+                      new(I) Value(call->ArgumentAt(0)),
+                      call->token_pos()));
+      return true;
+    } else if (class_ids[0] == kMintCid) {
+      AddReceiverCheck(call);
+      ReplaceCall(call,
+                  new(I) MintToDoubleInstr(new(I) Value(call->ArgumentAt(0)),
+                                           call->deopt_id()));
+      return true;
+    }
   }
 
   if (class_ids[0] == kDoubleCid) {
     if (!CanUnboxDouble()) {
       return false;
     }
     switch (recognized_kind) {
       case MethodRecognizer::kDoubleToInteger: {
         AddReceiverCheck(call);
         ASSERT(call->HasICData());
@@ skipping 1423 matching lines @@
         new(I) ZoneGrowableArray<Value*>(call->ArgumentCount());
     for (intptr_t i = 0; i < call->ArgumentCount(); i++) {
       args->Add(new(I) Value(call->ArgumentAt(i)));
     }
     InvokeMathCFunctionInstr* invoke =
         new(I) InvokeMathCFunctionInstr(args,
                                         call->deopt_id(),
                                         recognized_kind,
                                         call->token_pos());
     ReplaceCall(call, invoke);
-  } else if (recognized_kind == MethodRecognizer::kDoubleFromInteger) {
-    if (call->HasICData() && (call->ic_data()->NumberOfChecks() == 1)) {

[Cutch, 2014/09/16 16:04:11]

This code had two problems:

1) DoubleFromInteger is a factory constructor and we were checking argument 0
(which is the type and is always null) so this never activated.
2) toDouble calls the factory constructor but toDouble on smis was inlined as an
instance call. So we never even hit here.

[Florian Schneider, 2014/09/17 11:15:32]

Good catch. Double.fromInteger is called in a lot of other places (e.g.
Double._addFromInteger), so my suggestion instead of removing this code is to
fix it by making num_checked_arguments 2 for calls to Double.fromInteger and
check the second argument using ArgIsAlways(kSmiCid, ic_data, 1).

Then you can the case for mint here as well.

[Cutch, 2014/09/18 16:39:49]

Done.

-      const ICData& ic_data = *call->ic_data();
-      if (CanUnboxDouble() && ArgIsAlways(kSmiCid, ic_data, 0)) {
-        Definition* arg = call->ArgumentAt(0);
-        InsertBefore(call,
-                     new(I) CheckSmiInstr(
-                         new(I) Value(arg),
-                         call->deopt_id(),
-                         call->token_pos()),
-                     call->env(),
-                     FlowGraph::kEffect);
-        ReplaceCall(call,
-                    new(I) SmiToDoubleInstr(new(I) Value(arg),
-                                            call->token_pos()));
-      }
-    }
   } else if (call->function().IsFactory()) {
     const Class& function_class =
         Class::Handle(I, call->function().Owner());
     if ((function_class.library() == Library::CoreLibrary()) ||
         (function_class.library() == Library::TypedDataLibrary())) {
       intptr_t cid = FactoryRecognizer::ResultCid(call->function());
       switch (cid) {
         case kArrayCid: {
           Value* type = new(I) Value(call->ArgumentAt(0));
           Value* num_elements = new(I) Value(call->ArgumentAt(1));
@@ skipping 3944 matching lines @@
   const Object& value = instr->value()->definition()->constant_value();
   if (IsConstant(value) && value.IsInteger()) {
     SetValue(instr, Double::Handle(I,
         Double::New(Integer::Cast(value).AsDoubleValue(), Heap::kOld)));
   } else if (IsNonConstant(value)) {
     SetValue(instr, non_constant_);
   }
 }
 
 
+void ConstantPropagator::VisitMintToDouble(MintToDoubleInstr* instr) {
+  const Object& value = instr->value()->definition()->constant_value();
+  if (IsConstant(value) && value.IsInteger()) {
+    SetValue(instr, Double::Handle(I,
+        Double::New(Integer::Cast(value).AsDoubleValue(), Heap::kOld)));
+  } else if (IsNonConstant(value)) {
+    SetValue(instr, non_constant_);
+  }
+}
+
+
 void ConstantPropagator::VisitInt32ToDouble(Int32ToDoubleInstr* instr) {
   const Object& value = instr->value()->definition()->constant_value();
   if (IsConstant(value) && value.IsInteger()) {
     SetValue(instr, Double::Handle(I,
         Double::New(Integer::Cast(value).AsDoubleValue(), Heap::kOld)));
   } else if (IsNonConstant(value)) {
     SetValue(instr, non_constant_);
   }
 }
 
@@ skipping 1650 matching lines @@
 
   // Insert materializations at environment uses.
   for (intptr_t i = 0; i < exits_collector_.exits().length(); i++) {
     CreateMaterializationAt(
         exits_collector_.exits()[i], alloc, alloc->cls(), *slots);
   }
 }
 
 
 }  // namespace dart