Fixed Typefeedback for Number Truncation

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 34ecfa34b58b342399052df6ef589f33fe17eb6c..a905673802ef25a21cbdf6c75bda12fbb2a90b54 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -7631,7 +7631,7 @@ 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,
+bool HGraphBuilder::MatchRotateRight(HValue* left,
                                               HValue* right,

[Sven Panne, 2013/08/01 09:21:09]

Nit: Fix indentation of arguments.

                                               HValue** operand,
                                               HValue** shift_amount) {
@@ -7697,6 +7697,7 @@ HValue* HGraphBuilder::TruncateToNumber(HValue* value, Handle<Type>* expected) {
     if_nan.Then();
     if_nan.ElseDeopt();
     if_nan.End();
+    *expected = handle(Type::Double(), isolate());
     return Add<HConstant>(OS::nan_value());
   }
 
@@ -7713,12 +7714,28 @@ 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) {

[Sven Panne, 2013/08/01 09:21:09]

Hmmm, having 8 arguments is not very nice, but let's see how it will be used
elsewhere and perhaps refactor that. A TODO would be nice.

[oliv, 2013/08/01 09:53:43]

well i know that the stub will have to pass exactly those args, since in the
case of stybs the type-feedback comes directly from the extra-ic-state and not
the Type system....

   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())) {
+  if (op != Token::ADD ||

[Sven Panne, 2013/08/01 09:21:09]

Brain explosion caused by heavy use of negation... o_O Pushing the negation
outwards, things are much clearer:

  if (!(op == Token::ADD &&
        (left_type->Maybe(Type::String()) ||
         right_type->Maybe(Type::String())))) {

Naming the expression would be even better:

  bool maybe_string_addition = ...
  if (!maybe_string_addition) {

This would even make the comment in the if body obsolete.

"Comments are a sign of our inability to express things in code." :-)

[oliv, 2013/08/01 09:53:43]

yup, mechanical refactoring issue...

+      (!left_type->Maybe(Type::String()) &&
+       !right_type->Maybe(Type::String()))) {
     // 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);
@@ -7735,7 +7752,7 @@ HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
     right_type = handle(Type::Any(), isolate());
   }
   HInstruction* instr = NULL;
-  switch (expr->op()) {
+  switch (op) {
     case Token::ADD:
       if (left_type->Is(Type::String()) && right_type->Is(Type::String())) {
         BuildCheckHeapObject(left);
@@ -7761,7 +7778,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;
@@ -7770,7 +7787,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;
     }