Pass the complete number type information into the GenericBinaryOpStub.

Currently we only pass a boolean parameter indicating whether
the input operands to the GenericBinaryOpStub are guaranteed
to be numbers or not. 

Instead we pass the complete number type as a parameters. This 
allows to use more precise type information for code generation
in the stub.

Also make the computation of the result type more precise and correct on both ia32 and x64.

Review URL: http://codereview.chromium.org/593110

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

src/codegen.h

@@ -31,6 +31,7 @@
 #include "ast.h"
 #include "code-stubs.h"
 #include "runtime.h"
+#include "number-info.h"
 
 // Include the declaration of the architecture defined class CodeGenerator.
 // The contract  to the shared code is that the the CodeGenerator is a subclass

src/frame-element.cc

@@ -32,21 +32,6 @@
 namespace v8 {
 namespace internal {
 
-// -------------------------------------------------------------------------
-// FrameElement implementation.
-
-NumberInfo::Type FrameElement::number_info() {
-  // Copied elements do not have number info. Instead
-  // we have to inspect their backing element in the frame.
-  ASSERT(!is_copy());
-  if (!is_constant()) return NumberInfoField::decode(value_);
-  Handle<Object> value = handle();
-  if (value->IsSmi()) return NumberInfo::kSmi;
-  if (value->IsHeapNumber()) return NumberInfo::kHeapNumber;
-  return NumberInfo::kUnknown;
-}
-
-
 FrameElement::ZoneObjectList* FrameElement::ConstantList() {
   static ZoneObjectList list(10);
   return &list;

src/frame-element.h

@@ -53,8 +53,18 @@ class FrameElement BASE_EMBEDDED {
     SYNCED
   };
 
-  NumberInfo::Type number_info();
-  void set_number_info(NumberInfo::Type info) {
+  inline NumberInfo::Type number_info() {
+    // Copied elements do not have number info. Instead
+    // we have to inspect their backing element in the frame.
+    ASSERT(!is_copy());
+    if (!is_constant()) return NumberInfoField::decode(value_);
+    Handle<Object> value = handle();
+    if (value->IsSmi()) return NumberInfo::kSmi;
+    if (value->IsHeapNumber()) return NumberInfo::kHeapNumber;
+    return NumberInfo::kUnknown;
+  }
+
+  inline void set_number_info(NumberInfo::Type info) {
     value_ = value_ & ~NumberInfoField::mask();
     value_ = value_ | NumberInfoField::encode(info);
   }

src/ia32/codegen-ia32.cc

@@ -840,13 +840,13 @@ const char* GenericBinaryOpStub::GetName() {
   }
 
   OS::SNPrintF(Vector<char>(name_, kMaxNameLength),
-               "GenericBinaryOpStub_%s_%s%s_%s%s%s",
+               "GenericBinaryOpStub_%s_%s%s_%s%s_%s",
                op_name,
                overwrite_name,
                (flags_ & NO_SMI_CODE_IN_STUB) ? "_NoSmiInStub" : "",
                args_in_registers_ ? "RegArgs" : "StackArgs",
                args_reversed_ ? "_R" : "",
-               only_numbers_in_stub_ ? "_OnlyNumbers" : "");
+               NumberInfo::ToString(operands_type_));
   return name_;
 }
 
@@ -1010,8 +1010,8 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
   }
 
   // Get number type of left and right sub-expressions.
-  bool only_numbers = left.is_number() && right.is_number();
-  bool only_smis = left.is_smi() && right.is_smi();
+  NumberInfo::Type operands_type =
+      NumberInfo::Combine(left.number_info(), right.number_info());
 
   Result answer;
   if (left_is_non_smi_constant || right_is_non_smi_constant) {
@@ -1019,7 +1019,7 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
     GenericBinaryOpStub stub(op,
                              overwrite_mode,
                              NO_SMI_CODE_IN_STUB,
-                             only_numbers);
+                             operands_type);
     answer = stub.GenerateCall(masm_, frame_, &left, &right);
   } else if (right_is_smi_constant) {
     answer = ConstantSmiBinaryOperation(op, &left, right.handle(),
@@ -1039,46 +1039,64 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
       GenericBinaryOpStub stub(op,
                                overwrite_mode,
                                NO_GENERIC_BINARY_FLAGS,
-                               only_numbers);
+                               operands_type);
       answer = stub.GenerateCall(masm_, frame_, &left, &right);
     }
   }
 
   // Set NumberInfo of result according to the operation performed.
-  NumberInfo::Type info = NumberInfo::kUnknown;
+  // Rely on the fact that smis have a 31 bit payload on ia32.
+  ASSERT(kSmiValueSize == 31);
+  NumberInfo::Type result_type = NumberInfo::kUnknown;
   switch (op) {
     case Token::COMMA:
-      info = right.number_info();
+      result_type = right.number_info();
+      break;
     case Token::OR:
     case Token::AND:
-      // Could be anything. Check inputs.
-      if (only_numbers)
-        info = NumberInfo::kNumber;
+      // Result type can be either of the two input types.
+      result_type = operands_type;
       break;
     case Token::BIT_OR:
     case Token::BIT_XOR:
     case Token::BIT_AND:
+      // Result is always a number. Smi property of inputs is preserved.
+      result_type = (operands_type == NumberInfo::kSmi)
+          ? NumberInfo::kSmi
+          : NumberInfo::kNumber;
+      break;
     case Token::SAR:
-    case Token::SHR:
-      info = only_smis ? NumberInfo::kSmi : NumberInfo::kNumber;
+      // Result is a smi if we shift by a constant >= 1, otherwise a number.
+      result_type = (right.is_constant() && right.handle()->IsSmi()
+                     && Smi::cast(*right.handle())->value() >= 1)
+          ? NumberInfo::kSmi
+          : NumberInfo::kNumber;
       break;
-    case Token::SHL:
-      info = NumberInfo::kNumber;
+    case Token::SHR:
+      // Result is a smi if we shift by a constant >= 2, otherwise a number.
+      result_type = (right.is_constant() && right.handle()->IsSmi()
+                     && Smi::cast(*right.handle())->value() >= 2)
+          ? NumberInfo::kSmi
+          : NumberInfo::kNumber;
       break;
     case Token::ADD:
-      // Could be strings or numbers. Check types of inputs.
-      if (only_numbers) info = NumberInfo::kNumber;
+      // Result could be a string or a number. Check types of inputs.
+      result_type = NumberInfo::IsNumber(operands_type)
+          ? NumberInfo::kNumber
+          : NumberInfo::kUnknown;
       break;
+    case Token::SHL:
     case Token::SUB:
     case Token::MUL:
     case Token::DIV:
     case Token::MOD:
-      info = NumberInfo::kNumber;
+      // Result is always a number.
+      result_type = NumberInfo::kNumber;
       break;
     default:
       UNREACHABLE();
   }
-  answer.set_number_info(info);
+  answer.set_number_info(result_type);
   frame_->Push(&answer);
 }
 
@@ -7627,7 +7645,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
     case Token::DIV: {
       if (CpuFeatures::IsSupported(SSE2)) {
         CpuFeatures::Scope use_sse2(SSE2);
-        if (only_numbers_in_stub_) {
+        if (NumberInfo::IsNumber(operands_type_)) {
           if (FLAG_debug_code) {
             // Assert at runtime that inputs are only numbers.
             __ AbortIfNotNumber(edx,
@@ -7651,7 +7669,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
         __ movdbl(FieldOperand(eax, HeapNumber::kValueOffset), xmm0);
         GenerateReturn(masm);
       } else {  // SSE2 not available, use FPU.
-        if (only_numbers_in_stub_) {
+        if (NumberInfo::IsNumber(operands_type_)) {
           if (FLAG_debug_code) {
             // Assert at runtime that inputs are only numbers.
             __ AbortIfNotNumber(edx,

src/ia32/codegen-ia32.h

@@ -662,14 +662,14 @@ class GenericBinaryOpStub: public CodeStub {
   GenericBinaryOpStub(Token::Value op,
                       OverwriteMode mode,
                       GenericBinaryFlags flags,
-                      bool only_numbers = false)
+                      NumberInfo::Type operands_type = NumberInfo::kUnknown)
       : op_(op),
         mode_(mode),
         flags_(flags),
         args_in_registers_(false),
         args_reversed_(false),
         name_(NULL),
-        only_numbers_in_stub_(only_numbers) {
+        operands_type_(operands_type) {
     use_sse3_ = CpuFeatures::IsSupported(SSE3);
     ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
   }
@@ -694,32 +694,32 @@ class GenericBinaryOpStub: public CodeStub {
   bool args_reversed_;  // Left and right argument are swapped.
   bool use_sse3_;
   char* name_;
-  bool only_numbers_in_stub_;  // Arguments are only numbers.
+  NumberInfo::Type operands_type_;  // Number type information of operands.
 
   const char* GetName();
 
 #ifdef DEBUG
   void Print() {
     PrintF("GenericBinaryOpStub %d (op %s), "
-           "(mode %d, flags %d, registers %d, reversed %d, only_numbers %d)\n",
+           "(mode %d, flags %d, registers %d, reversed %d, number_info %s)\n",
            MinorKey(),
            Token::String(op_),
            static_cast<int>(mode_),
            static_cast<int>(flags_),
            static_cast<int>(args_in_registers_),
            static_cast<int>(args_reversed_),
-           static_cast<int>(only_numbers_in_stub_));
+           NumberInfo::ToString(operands_type_));
   }
 #endif
 
-  // Minor key encoding in 16 bits NFRASOOOOOOOOOMM.
+  // Minor key encoding in 16 bits NNNFRASOOOOOOOMM.
   class ModeBits: public BitField<OverwriteMode, 0, 2> {};
-  class OpBits: public BitField<Token::Value, 2, 9> {};
-  class SSE3Bits: public BitField<bool, 11, 1> {};
-  class ArgsInRegistersBits: public BitField<bool, 12, 1> {};
-  class ArgsReversedBits: public BitField<bool, 13, 1> {};
-  class FlagBits: public BitField<GenericBinaryFlags, 14, 1> {};
-  class OnlyNumbersBits: public BitField<bool, 15, 1> {};
+  class OpBits: public BitField<Token::Value, 2, 7> {};
+  class SSE3Bits: public BitField<bool, 9, 1> {};
+  class ArgsInRegistersBits: public BitField<bool, 10, 1> {};
+  class ArgsReversedBits: public BitField<bool, 11, 1> {};
+  class FlagBits: public BitField<GenericBinaryFlags, 12, 1> {};
+  class NumberInfoBits: public BitField<NumberInfo::Type, 13, 3> {};
 
   Major MajorKey() { return GenericBinaryOp; }
   int MinorKey() {
@@ -730,7 +730,7 @@ class GenericBinaryOpStub: public CodeStub {
            | SSE3Bits::encode(use_sse3_)
            | ArgsInRegistersBits::encode(args_in_registers_)
            | ArgsReversedBits::encode(args_reversed_)
-           | OnlyNumbersBits::encode(only_numbers_in_stub_);
+           | NumberInfoBits::encode(operands_type_);
   }
 
   void Generate(MacroAssembler* masm);

src/number-info.h

@@ -46,6 +46,26 @@ class NumberInfo : public AllStatic {
     // Make use of the order of enum values.
     return static_cast<Type>(a & b);
   }
+
+  static bool IsNumber(Type a) {
+    ASSERT(a != kUninitialized);
+    return ((a & kNumber) != 0);
+  }
+
+  static const char* ToString(Type a) {
+    switch (a) {
+      case kUnknown: return "UnknownType";
+      case kNumber: return "NumberType";
+      case kSmi: return "SmiType";
+      case kHeapNumber: return "HeapNumberType";
+      case kUninitialized:
+        UNREACHABLE();
+        return "UninitializedType";
+    }
+    UNREACHABLE();
+    return "Unreachable code";
+  }
+
 };
 
 } }  // namespace v8::internal

src/x64/codegen-x64.cc

@@ -5198,15 +5198,15 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
   }
 
   // Get number type of left and right sub-expressions.
-  bool only_numbers = left.is_number() && right.is_number();
-  bool only_smis = left.is_smi() && right.is_smi();
+  NumberInfo::Type operands_type =
+      NumberInfo::Combine(left.number_info(), right.number_info());
 
   Result answer;
   if (left_is_non_smi_constant || right_is_non_smi_constant) {
     GenericBinaryOpStub stub(op,
                              overwrite_mode,
                              NO_SMI_CODE_IN_STUB,
-                             only_numbers);
+                             operands_type);
     answer = stub.GenerateCall(masm_, frame_, &left, &right);
   } else if (right_is_smi_constant) {
     answer = ConstantSmiBinaryOperation(op, &left, right.handle(),
@@ -5226,50 +5226,59 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
       GenericBinaryOpStub stub(op,
                                overwrite_mode,
                                NO_GENERIC_BINARY_FLAGS,
-                               only_numbers);
+                               operands_type);
       answer = stub.GenerateCall(masm_, frame_, &left, &right);
     }
   }
 
   // Set NumberInfo of result according to the operation performed.
-  NumberInfo::Type info = NumberInfo::kUnknown;
+  // We rely on the fact that smis have a 32 bit payload on x64.
+  ASSERT(kSmiValueSize == 32);
+  NumberInfo::Type result_type = NumberInfo::kUnknown;
   switch (op) {
     case Token::COMMA:
-      info = right.number_info();
+      result_type = right.number_info();
       break;
     case Token::OR:
     case Token::AND:
-      // Could be anything. Check inputs.
-      if (only_numbers)
-        info = NumberInfo::kNumber;
+      // Result type can be either of the two input types.
+      result_type = operands_type;
       break;
     case Token::BIT_OR:
     case Token::BIT_XOR:
     case Token::BIT_AND:
-    case Token::SAR:
-    case Token::SHR:
-      // TODO(fsc): Make use of the fact that smis are 32 bits on x64.
-      info = only_smis ? NumberInfo::kSmi : NumberInfo::kNumber;
+      // Result is always a smi.
+      result_type = NumberInfo::kSmi;
       break;
+    case Token::SAR:
     case Token::SHL:
-      info = NumberInfo::kNumber;
+      // Result is always a smi.
+      result_type = NumberInfo::kSmi;
+      break;
+    case Token::SHR:
+      // Result of x >>> y is always a smi if y >= 1, otherwise a number.
+      result_type = (right.is_constant() && right.handle()->IsSmi()
+                     && Smi::cast(*right.handle())->value() >= 1)
+          ? NumberInfo::kSmi
+          : NumberInfo::kNumber;
       break;
     case Token::ADD:
-      // Could be strings or numbers. Check types of inputs.
-      if (only_numbers) {
-        info = NumberInfo::kNumber;
-      }
+      // Result could be a string or a number. Check types of inputs.
+      result_type = NumberInfo::IsNumber(operands_type)
+          ? NumberInfo::kNumber
+          : NumberInfo::kUnknown;
       break;
     case Token::SUB:
     case Token::MUL:
     case Token::DIV:
     case Token::MOD:
-      info = NumberInfo::kNumber;
+      // Result is always a number.
+      result_type = NumberInfo::kNumber;
       break;
     default:
       UNREACHABLE();
   }
-  answer.set_number_info(info);
+  answer.set_number_info(result_type);
   frame_->Push(&answer);
 }
 
@@ -8242,7 +8251,7 @@ const char* GenericBinaryOpStub::GetName() {
                args_in_registers_ ? "RegArgs" : "StackArgs",
                args_reversed_ ? "_R" : "",
                use_sse3_ ? "SSE3" : "SSE2",
-               only_numbers_in_stub_ ? "_OnlyNumbers" : "");
+               NumberInfo::ToString(operands_type_));
   return name_;
 }
 
@@ -8566,7 +8575,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
     case Token::DIV: {
       // rax: y
       // rdx: x
-      if (only_numbers_in_stub_) {
+      if (NumberInfo::IsNumber(operands_type_)) {
         if (FLAG_debug_code) {
           // Assert at runtime that inputs are only numbers.
           __ AbortIfNotNumber(rdx, "GenericBinaryOpStub operand not a number.");

src/x64/codegen-x64.h

@@ -659,14 +659,14 @@ class GenericBinaryOpStub: public CodeStub {
   GenericBinaryOpStub(Token::Value op,
                       OverwriteMode mode,
                       GenericBinaryFlags flags,
-                      bool only_numbers = false)
+                      NumberInfo::Type operands_type = NumberInfo::kUnknown)
       : op_(op),
         mode_(mode),
         flags_(flags),
         args_in_registers_(false),
         args_reversed_(false),
         name_(NULL),
-        only_numbers_in_stub_(only_numbers) {
+        operands_type_(operands_type) {
     use_sse3_ = CpuFeatures::IsSupported(SSE3);
     ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
   }
@@ -691,32 +691,32 @@ class GenericBinaryOpStub: public CodeStub {
   bool args_reversed_;  // Left and right argument are swapped.
   bool use_sse3_;
   char* name_;
-  bool only_numbers_in_stub_;
+  NumberInfo::Type operands_type_;
 
   const char* GetName();
 
 #ifdef DEBUG
   void Print() {
     PrintF("GenericBinaryOpStub %d (op %s), "
-           "(mode %d, flags %d, registers %d, reversed %d, only_numbers %d)\n",
+           "(mode %d, flags %d, registers %d, reversed %d, only_numbers %s)\n",
            MinorKey(),
            Token::String(op_),
            static_cast<int>(mode_),
            static_cast<int>(flags_),
            static_cast<int>(args_in_registers_),
            static_cast<int>(args_reversed_),
-           static_cast<int>(only_numbers_in_stub_));
+           NumberInfo::ToString(operands_type_));
   }
 #endif
 
-  // Minor key encoding in 16 bits NFRASOOOOOOOOOMM.
+  // Minor key encoding in 16 bits NNNFRASOOOOOOOMM.
   class ModeBits: public BitField<OverwriteMode, 0, 2> {};
-  class OpBits: public BitField<Token::Value, 2, 9> {};
-  class SSE3Bits: public BitField<bool, 11, 1> {};
-  class ArgsInRegistersBits: public BitField<bool, 12, 1> {};
-  class ArgsReversedBits: public BitField<bool, 13, 1> {};
-  class FlagBits: public BitField<GenericBinaryFlags, 14, 1> {};
-  class OnlyNumberBits: public BitField<bool, 15, 1> {};
+  class OpBits: public BitField<Token::Value, 2, 7> {};
+  class SSE3Bits: public BitField<bool, 9, 1> {};
+  class ArgsInRegistersBits: public BitField<bool, 10, 1> {};
+  class ArgsReversedBits: public BitField<bool, 11, 1> {};
+  class FlagBits: public BitField<GenericBinaryFlags, 12, 1> {};
+  class NumberInfoBits: public BitField<NumberInfo::Type, 13, 3> {};
 
   Major MajorKey() { return GenericBinaryOp; }
   int MinorKey() {
@@ -727,7 +727,7 @@ class GenericBinaryOpStub: public CodeStub {
            | SSE3Bits::encode(use_sse3_)
            | ArgsInRegistersBits::encode(args_in_registers_)
            | ArgsReversedBits::encode(args_reversed_)
-           | OnlyNumberBits::encode(only_numbers_in_stub_);
+           | NumberInfoBits::encode(operands_type_);
   }
 
   void Generate(MacroAssembler* masm);