Fixed Typefeedback for Number Truncation

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 7613 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,

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

Nit: Fix indentation of arguments.

                                               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);
     shr = HShr::cast(left);
@@ skipping 44 matching lines @@
   // tagged representation later on.
   if (expected_type->Is(Type::Oddball())) {
     // TODO(olivf) The BinaryOpStub only records undefined. It might pay off to
     // also record booleans and convert them to 0/1 here.
     IfBuilder if_nan(this);
     if_nan.If<HCompareObjectEqAndBranch>(value,
         graph()->GetConstantUndefined());
     if_nan.Then();
     if_nan.ElseDeopt();
     if_nan.End();
+    *expected = handle(Type::Double(), isolate());
     return Add<HConstant>(OS::nan_value());
   }
 
   return value;
 }
 
 
 HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
     BinaryOperation* expr,
     HValue* left,
     HValue* right) {
   HValue* context = environment()->context();
   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();
+
+  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);
     right = TruncateToNumber(right, &right_type);
   }
 
   if (left_type->Is(Type::None())) {
     Add<HDeoptimize>(Deoptimizer::SOFT);
     // 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())) {
     Add<HDeoptimize>(Deoptimizer::SOFT);
     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);
         AddInstruction(HCheckInstanceType::NewIsString(left, zone()));
         BuildCheckHeapObject(right);
         AddInstruction(HCheckInstanceType::NewIsString(right, zone()));
         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);
       break;
     case Token::MOD:
       instr = HMod::New(zone(), context, left, right, fixed_right_arg);
       break;
     case Token::DIV:
       instr = HDiv::New(zone(), context, left, right);
       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;
       if (left_type->Is(Type::Signed32()) &&
           right_type->Is(Type::Signed32()) &&
           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;
     }
     case Token::SAR:
       instr = HSar::New(zone(), context, left, right);
       break;
     case Token::SHR:
       instr = HShr::New(zone(), context, left, right);
       if (FLAG_opt_safe_uint32_operations && instr->IsShr() &&
           CanBeZero(right)) {
@@ skipping 2014 matching lines @@
   if (ShouldProduceTraceOutput()) {
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 } }  // namespace v8::internal