Speed up Binops

src/hydrogen.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 3104 matching lines @@
           instruction->DeleteAndReplaceWith(instruction->RedefinedOperand());
         } else {
           instruction->ReplaceAllUsesWith(instruction->ActualValue());
         }
       }
     }
   }
 }
 
 
-void HOptimizedGraphBuilder::PushAndAdd(HInstruction* instr) {
+void HGraphBuilder::PushAndAdd(HInstruction* instr) {
   Push(instr);
   AddInstruction(instr);
 }
 
 
 template <class Instruction>
 HInstruction* HOptimizedGraphBuilder::PreProcessCall(Instruction* call) {
   int count = call->argument_count();
   ZoneList<HValue*> arguments(count, zone());
   for (int i = 0; i < count; ++i) {
@@ skipping 4709 matching lines @@
     return false;
   }
   return (sub->right() == sa);
 }
 
 
 // Checks if the left and the right are shift instructions with the oposite
 // directions that can be replaced by one rotate right instruction or not.
 // Returns the operand and the shift amount for the rotate instruction in the
 // former case.
-bool HOptimizedGraphBuilder::MatchRotateRight(HValue* left,
+bool HGraphBuilder::MatchRotateRight(HValue* left,
                                               HValue* right,
                                               HValue** operand,
                                               HValue** shift_amount) {
   HShl* shl;
   HShr* shr;
   if (left->IsShl() && right->IsShr()) {
     shl = HShl::cast(left);
     shr = HShr::cast(right);
   } else if (left->IsShr() && right->IsShl()) {
     shl = HShl::cast(right);
@@ skipping 18 matching lines @@
     HConstant* right_const = HConstant::cast(right);
     if (right_const->HasInteger32Value() &&
        (right_const->Integer32Value() & 0x1f) != 0) {
       return false;
     }
   }
   return true;
 }
 
 
-HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
+HValue* HGraphBuilder::ConstantToNumber(HValue* value) {

[Toon Verwaest, 2013/07/18 13:57:12]

Perhaps you can also handle strings here.

+  if (value->IsConstant()) {

[Toon Verwaest, 2013/07/18 13:57:12]

Move this code together with HConstant::CopyToTruncatedInt32 as "CopyToDouble",
and perhaps extend CopyToTruncatedInt32 to also directly handle the special
values handled below.

+    HConstant* constant = HConstant::cast(value);
+    if (constant->handle()->IsBoolean()) {
+      return constant->handle()->BooleanValue()
+        ? graph()->GetConstant1() : graph()->GetConstant0();

[Jakob Kummerow, 2013/07/18 11:56:15]

nit: indentation

+    } else if (constant->handle()->IsUndefined() ||
+               constant->handle()->IsTheHole()) {

[Toon Verwaest, 2013/07/18 13:57:12]

Remove TheHole case.

+      return Add<HConstant>(OS::nan_value(), Representation::Double());
+    } else if (constant->handle()->IsNull()) {
+      return graph()->GetConstant0();
+    }
+  }
+  return value;
+}
+
+
+HInstruction* HGraphBuilder::BuildBinaryOperation(
     BinaryOperation* expr,
     HValue* left,
     HValue* right) {
   HValue* context = environment()->LookupContext();
   Handle<Type> left_type = expr->left()->bounds().lower;
   Handle<Type> right_type = expr->right()->bounds().lower;
   Handle<Type> result_type = expr->bounds().lower;
   Maybe<int> fixed_right_arg = expr->fixed_right_arg();
-  Representation left_rep = Representation::FromType(left_type);
-  Representation right_rep = Representation::FromType(right_type);
-  Representation result_rep = Representation::FromType(result_type);
+
+  if (expr->op() != Token::ADD ||
+      (left->type().IsNoString() && right->type().IsNoString())) {
+    left = ConstantToNumber(left);
+    right = ConstantToNumber(right);
+  }
+
+  left_type = left->type().intersect(left_type, isolate());
+  right_type = right->type().intersect(right_type, isolate());
 
   if (left_type->Is(Type::None())) {
     AddSoftDeoptimize();
     // TODO(rossberg): we should be able to get rid of non-continuous defaults.
     left_type = handle(Type::Any(), isolate());
   }
   if (right_type->Is(Type::None())) {
     AddSoftDeoptimize();
     right_type = handle(Type::Any(), isolate());
   }
+
+  Representation left_rep = left->IsConstant() ?
+    left->KnownOptimalRepresentation() : Representation::FromType(left_type);

[Jakob Kummerow, 2013/07/18 11:56:15]

nit: indentation. Also, prefer the '?' on the next line. Maybe:
  Representation left_rep = left->IsConstant()
      ? left->KnownOptimalRepresentation()
      : Representation::FromType(left_type);

[Toon Verwaest, 2013/07/18 13:57:12]

Could we calculate this in "type" and only at the end convert to Representation?
Now it's pretty schizophrenic mixing HType, Type and Representation.

+  Representation right_rep = right->IsConstant() ?
+    right->KnownOptimalRepresentation() : Representation::FromType(right_type);
+  Representation result_rep = Representation::FromType(result_type);
+
   HInstruction* instr = NULL;
   switch (expr->op()) {
     case Token::ADD:
       if (left_type->Is(Type::String()) && right_type->Is(Type::String())) {
-        BuildCheckHeapObject(left);
-        AddInstruction(HCheckInstanceType::NewIsString(left, zone()));
-        BuildCheckHeapObject(right);
-        AddInstruction(HCheckInstanceType::NewIsString(right, zone()));
+        if (!left->type().IsString() || !right->type().IsString()) {

[Jakob Kummerow, 2013/07/18 11:56:15]

Why is this one atomic block rather than "if (!left is string) { add check for
left; }" and then the same for |right|?

+          BuildCheckHeapObject(left);
+          AddInstruction(HCheckInstanceType::NewIsString(left, zone()));
+          BuildCheckHeapObject(right);
+          AddInstruction(HCheckInstanceType::NewIsString(right, zone()));

[Toon Verwaest, 2013/07/18 13:57:12]

I'd use canonicalization to eliminate CheckInstanceType(string) of values that
are known to be type().IsString(); rather than conditionally emit the
instructions.

+        }
         instr = HStringAdd::New(zone(), context, left, right);
       } else {
         instr = HAdd::New(zone(), context, left, right);
       }
       break;
     case Token::SUB:
       instr = HSub::New(zone(), context, left, right);
       break;
     case Token::MUL:
       instr = HMul::New(zone(), context, left, right);
@@ skipping 2056 matching lines @@
   if (ShouldProduceTraceOutput()) {
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 } }  // namespace v8::internal