Hydrogen binop improvements

- Truncate oddball if possible.
- Support for StringAdd with only one String argument.
- Use constructor macro for HMul.
- Add ForceNumberType for HydrogenStubs to enforce input representations.

BUG=
R=svenpanne@chromium.org

Review URL: https://codereview.chromium.org/23503058

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16849 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/hydrogen-instructions.h

@@ -1207,6 +1207,12 @@ class HControlInstruction : public HInstruction {
     return SuccessorCount() > 1 ? SuccessorAt(1) : NULL;
   }
 
+  void Not() {
+    HBasicBlock* swap = SuccessorAt(0);
+    SetSuccessorAt(0, SuccessorAt(1));
+    SetSuccessorAt(1, swap);
+  }
+
   DECLARE_ABSTRACT_INSTRUCTION(ControlInstruction)
 };
 
@@ -3907,13 +3913,13 @@ class HBitwiseBinaryOperation : public HBinaryOperation {
   }
 
   virtual void RepresentationChanged(Representation to) V8_OVERRIDE {
-    if (!to.IsTagged()) {
+    if (to.IsTagged()) {
+      SetAllSideEffects();
+      ClearFlag(kUseGVN);
+    } else {
       ASSERT(to.IsSmiOrInteger32());
       ClearAllSideEffects();
       SetFlag(kUseGVN);
-    } else {
-      SetAllSideEffects();
-      ClearFlag(kUseGVN);
     }
   }
 
@@ -4591,10 +4597,12 @@ class HMul V8_FINAL : public HArithmeticBinaryOperation {
                            HValue* right);
 
   static HInstruction* NewImul(Zone* zone,
-                               HValue* context,
-                               HValue* left,
-                               HValue* right) {
-    HMul* mul = new(zone) HMul(context, left, right);
+                         HValue* context,
+                         HValue* left,
+                         HValue* right) {
+    HInstruction* instr = HMul::New(zone, context, left, right);
+    if (!instr->IsMul()) return instr;
+    HMul* mul = HMul::cast(instr);
     // TODO(mstarzinger): Prevent bailout on minus zero for imul.
     mul->AssumeRepresentation(Representation::Integer32());
     mul->ClearFlag(HValue::kCanOverflow);
@@ -6572,14 +6580,21 @@ class HStringAdd V8_FINAL : public HBinaryOperation {
   HStringAdd(HValue* context, HValue* left, HValue* right, StringAddFlags flags)
       : HBinaryOperation(context, left, right, HType::String()), flags_(flags) {
     set_representation(Representation::Tagged());
-    SetFlag(kUseGVN);
-    SetGVNFlag(kDependsOnMaps);
-    SetGVNFlag(kChangesNewSpacePromotion);
+    if (flags_ == STRING_ADD_CHECK_NONE) {
+      SetFlag(kUseGVN);
+      SetGVNFlag(kDependsOnMaps);
+      SetGVNFlag(kChangesNewSpacePromotion);
+    } else {
+      SetAllSideEffects();
+    }
   }
 
-  // No side-effects except possible allocation.
-  // NOTE: this instruction _does not_ call ToString() on its inputs.
-  virtual bool IsDeletable() const V8_OVERRIDE { return true; }
+  // No side-effects except possible allocation:
+  // NOTE: this instruction does not call ToString() on its inputs, when flags_
+  // is set to STRING_ADD_CHECK_NONE.
+  virtual bool IsDeletable() const V8_OVERRIDE {
+    return flags_ == STRING_ADD_CHECK_NONE;
+  }
 
   const StringAddFlags flags_;
 };

src/hydrogen.cc

@@ -737,7 +737,8 @@ HGraphBuilder::IfBuilder::IfBuilder(
 }
 
 
-void HGraphBuilder::IfBuilder::AddCompare(HControlInstruction* compare) {
+HControlInstruction* HGraphBuilder::IfBuilder::AddCompare(
+    HControlInstruction* compare) {
   if (split_edge_merge_block_ != NULL) {
     HEnvironment* env = first_false_block_->last_environment();
     HBasicBlock* split_edge =
@@ -756,6 +757,7 @@ void HGraphBuilder::IfBuilder::AddCompare(HControlInstruction* compare) {
   }
   builder_->current_block()->Finish(compare);
   needs_compare_ = false;
+  return compare;
 }
 
 
@@ -7581,10 +7583,10 @@ static bool ShiftAmountsAllowReplaceByRotate(HValue* sa,
 // 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,
-                                              HValue* right,
-                                              HValue** operand,
-                                              HValue** shift_amount) {
+bool HGraphBuilder::MatchRotateRight(HValue* left,
+                                     HValue* right,
+                                     HValue** operand,
+                                     HValue** shift_amount) {
   HShl* shl;
   HShr* shr;
   if (left->IsShl() && right->IsShr()) {
@@ -7620,6 +7622,18 @@ bool CanBeZero(HValue* right) {
 }
 
 
+HValue* HGraphBuilder::EnforceNumberType(HValue* number,
+                                         Handle<Type> expected) {
+  if (expected->Is(Type::Smi())) {
+    return Add<HForceRepresentation>(number, Representation::Smi());
+  }
+  if (expected->Is(Type::Signed32())) {
+    return Add<HForceRepresentation>(number, Representation::Integer32());
+  }
+  return number;
+}
+
+
 HValue* HGraphBuilder::TruncateToNumber(HValue* value, Handle<Type>* expected) {
   if (value->IsConstant()) {
     HConstant* constant = HConstant::cast(value);
@@ -7630,6 +7644,63 @@ HValue* HGraphBuilder::TruncateToNumber(HValue* value, Handle<Type>* expected) {
     }
   }
 
+  Handle<Type> expected_type = *expected;
+
+  // Separate the number type from the rest.
+  Handle<Type> expected_obj = handle(Type::Intersect(
+      expected_type, handle(Type::NonNumber(), isolate())), isolate());
+  Handle<Type> expected_number = handle(Type::Intersect(
+      expected_type, handle(Type::Number(), isolate())), isolate());
+
+  // We expect to get a number.
+  // (We need to check first, since Type::None->Is(Type::Any()) == true.
+  if (expected_obj->Is(Type::None())) {
+    ASSERT(!expected_number->Is(Type::None()));
+    return value;
+  }
+
+  if (expected_obj->Is(Type::Undefined())) {
+    // This is already done by HChange.
+    *expected = handle(Type::Union(
+          expected_number, handle(Type::Double(), isolate())), isolate());
+    return value;
+  }
+
+  if (expected_obj->Is(Type::Null())) {
+    *expected = handle(Type::Union(
+          expected_number, handle(Type::Smi(), isolate())), isolate());
+    IfBuilder if_null(this);
+    if_null.If<HCompareObjectEqAndBranch>(value,
+                                          graph()->GetConstantNull());
+    if_null.Then();
+    Push(graph()->GetConstant0());
+    if_null.Else();
+    Push(value);
+    if_null.End();
+    return Pop();
+  }
+
+  if (expected_obj->Is(Type::Boolean())) {
+    *expected = handle(Type::Union(
+          expected_number, handle(Type::Smi(), isolate())), isolate());
+    IfBuilder if_true(this);
+    if_true.If<HCompareObjectEqAndBranch>(value,
+                                          graph()->GetConstantTrue());
+    if_true.Then();
+    Push(graph()->GetConstant1());
+    if_true.Else();
+    IfBuilder if_false(this);
+    if_false.If<HCompareObjectEqAndBranch>(value,
+                                           graph()->GetConstantFalse());
+    if_false.Then();
+    Push(graph()->GetConstant0());
+    if_false.Else();
+    Push(value);
+    if_false.End();
+    if_true.End();
+    return Pop();
+  }
+
   return value;
 }
 
@@ -7643,38 +7714,72 @@ HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
   Handle<Type> right_type = expr->right()->bounds().lower;
   Handle<Type> result_type = expr->bounds().lower;
   Maybe<int> fixed_right_arg = expr->fixed_right_arg();
+
+  return HGraphBuilder::BuildBinaryOperation(expr->op(), left, right,
+      left_type, right_type, result_type, fixed_right_arg, context);
+}
+
+
+HInstruction* HGraphBuilder::BuildBinaryOperation(
+    Token::Value op,
+    HValue* left,
+    HValue* right,
+    Handle<Type> left_type,
+    Handle<Type> right_type,
+    Handle<Type> result_type,
+    Maybe<int> fixed_right_arg,
+    HValue* context) {
+
   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().IsNonString() && right->type().IsNonString())) {
-    // For addition we can only truncate the arguments to number if we can
-    // prove that we will not end up in string concatenation mode.
-    left = TruncateToNumber(left, &left_type);
-    right = TruncateToNumber(right, &right_type);
-  }
+  bool maybe_string_add = op == Token::ADD &&
+                          (left_type->Maybe(Type::String()) ||
+                           right_type->Maybe(Type::String()));
 
   if (left_type->Is(Type::None())) {
     Add<HDeoptimize>("Insufficient type feedback for LHS of binary operation",
                      Deoptimizer::SOFT);
-    // TODO(rossberg): we should be able to get rid of non-continuous defaults.
+    // TODO(rossberg): we should be able to get rid of non-continuous
+    // defaults.
     left_type = handle(Type::Any(), isolate());
+  } else {
+    if (!maybe_string_add) left = TruncateToNumber(left, &left_type);
+    left_rep = Representation::FromType(left_type);
   }
+
   if (right_type->Is(Type::None())) {
     Add<HDeoptimize>("Insufficient type feedback for RHS of binary operation",
                      Deoptimizer::SOFT);
     right_type = handle(Type::Any(), isolate());
+  } else {
+    if (!maybe_string_add) right = TruncateToNumber(right, &right_type);
+    right_rep = Representation::FromType(right_type);
   }
+
+  Representation result_rep = Representation::FromType(result_type);
+
+  bool is_string_add = op == Token::ADD &&
+                       (left_type->Is(Type::String()) ||
+                        right_type->Is(Type::String()));
+
   HInstruction* instr = NULL;
-  switch (expr->op()) {
+  switch (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()));
-        instr = HStringAdd::New(zone(), context, left, right);
+      if (is_string_add) {
+        StringAddFlags flags = STRING_ADD_CHECK_BOTH;
+        if (left_type->Is(Type::String())) {
+          BuildCheckHeapObject(left);
+          AddInstruction(HCheckInstanceType::NewIsString(left, zone()));
+          flags = STRING_ADD_CHECK_RIGHT;
+        }
+        if (right_type->Is(Type::String())) {
+          BuildCheckHeapObject(right);
+          AddInstruction(HCheckInstanceType::NewIsString(right, zone()));
+          flags = (flags == STRING_ADD_CHECK_BOTH)
+              ? STRING_ADD_CHECK_LEFT : STRING_ADD_CHECK_NONE;
+        }
+        instr = HStringAdd::New(zone(), context, left, right, flags);
       } else {
         instr = HAdd::New(zone(), context, left, right);
       }
@@ -7693,7 +7798,7 @@ HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
       break;
     case Token::BIT_XOR:
     case Token::BIT_AND:
-      instr = NewUncasted<HBitwise>(expr->op(), left, right);
+      instr = NewUncasted<HBitwise>(op, left, right);
       break;
     case Token::BIT_OR: {
       HValue* operand, *shift_amount;
@@ -7702,7 +7807,7 @@ HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
           MatchRotateRight(left, right, &operand, &shift_amount)) {
         instr = new(zone()) HRor(context, operand, shift_amount);
       } else {
-        instr = NewUncasted<HBitwise>(expr->op(), left, right);
+        instr = NewUncasted<HBitwise>(op, left, right);
       }
       break;
     }

src/hydrogen.h

@@ -1260,10 +1260,26 @@ class HGraphBuilder {
   HInstruction* BuildLoadStringLength(HValue* object, HValue* checked_value);
   HStoreNamedField* AddStoreMapConstant(HValue* object, Handle<Map>);
   HLoadNamedField* AddLoadElements(HValue* object);
+
+  bool MatchRotateRight(HValue* left,
+                        HValue* right,
+                        HValue** operand,
+                        HValue** shift_amount);
+
+  HInstruction* BuildBinaryOperation(Token::Value op,
+                                     HValue* left,
+                                     HValue* right,
+                                     Handle<Type> left_type,
+                                     Handle<Type> right_type,
+                                     Handle<Type> result_type,
+                                     Maybe<int> fixed_right_arg,
+                                     HValue* context);
+
   HLoadNamedField* AddLoadFixedArrayLength(HValue *object);
 
   HValue* AddLoadJSBuiltin(Builtins::JavaScript builtin);
 
+  HValue* EnforceNumberType(HValue* number, Handle<Type> expected);
   HValue* TruncateToNumber(HValue* value, Handle<Type>* expected);
 
   void PushAndAdd(HInstruction* instr);
@@ -1308,30 +1324,21 @@ class HGraphBuilder {
     template<class Condition>
     Condition* IfNot(HValue* p) {
       Condition* compare = If<Condition>(p);
-      HBasicBlock* block0 = compare->SuccessorAt(0);
-      HBasicBlock* block1 = compare->SuccessorAt(1);
-      compare->SetSuccessorAt(0, block1);
-      compare->SetSuccessorAt(1, block0);
+      compare->Not();
       return compare;
     }
 
     template<class Condition, class P2>
     Condition* IfNot(HValue* p1, P2 p2) {
       Condition* compare = If<Condition>(p1, p2);
-      HBasicBlock* block0 = compare->SuccessorAt(0);
-      HBasicBlock* block1 = compare->SuccessorAt(1);
-      compare->SetSuccessorAt(0, block1);
-      compare->SetSuccessorAt(1, block0);
+      compare->Not();
       return compare;
     }
 
     template<class Condition, class P2, class P3>
     Condition* IfNot(HValue* p1, P2 p2, P3 p3) {
       Condition* compare = If<Condition>(p1, p2, p3);
-      HBasicBlock* block0 = compare->SuccessorAt(0);
-      HBasicBlock* block1 = compare->SuccessorAt(1);
-      compare->SetSuccessorAt(0, block1);
-      compare->SetSuccessorAt(1, block0);
+      compare->Not();
       return compare;
     }
 
@@ -1389,7 +1396,7 @@ class HGraphBuilder {
     void Return(HValue* value);
 
    private:
-    void AddCompare(HControlInstruction* compare);
+    HControlInstruction* AddCompare(HControlInstruction* compare);
 
     HGraphBuilder* builder() const { return builder_; }
 
@@ -2189,11 +2196,6 @@ class HOptimizedGraphBuilder V8_FINAL
                                 HValue* receiver,
                                 Handle<Map> receiver_map);
 
-  bool MatchRotateRight(HValue* left,
-                        HValue* right,
-                        HValue** operand,
-                        HValue** shift_amount);
-
   // The translation state of the currently-being-translated function.
   FunctionState* function_state_;
 

src/types.h

@@ -128,6 +128,7 @@ namespace internal {
   V(Receiver,        kObject | kProxy)                              \
   V(Allocated,       kDouble | kName | kReceiver)                   \
   V(Any,             kOddball | kNumber | kAllocated | kInternal)   \
+  V(NonNumber,       kAny - kNumber)                                \
   V(Detectable,      kAllocated - kUndetectable)
 
 #define TYPE_LIST(V)     \