Pass the complete number type information into the GenericBinaryOpStub.
authorfschneider@chromium.org <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 16 Feb 2010 13:03:16 +0000 (13:03 +0000)
committerfschneider@chromium.org <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 16 Feb 2010 13:03:16 +0000 (13:03 +0000)
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
src/frame-element.cc
src/frame-element.h
src/ia32/codegen-ia32.cc
src/ia32/codegen-ia32.h
src/number-info.h
src/x64/codegen-x64.cc
src/x64/codegen-x64.h

index 882b4844c5f11f4e83b07273c6bf4aa408bd17bb..5c10cb62cfb1d9ea18fb1badd5ce9f5952ce269a 100644 (file)
@@ -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
index 9b69e63ac29eb0ca9f0350e536bac9090d1c9252..14555596ad7d33982a88f4ef7e33cccc8127506f 100644 (file)
 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;
index 800e78d5911bdad5c7817e66f7607f2fdb809a98..3ae6d303f469fc2736a74dcdda0fb83556237f28 100644 (file)
@@ -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);
   }
index 8dcaeffbcab7e3f1dc1fea65141437d9cae06d49..122a528d55317a095100483969c0e4d6c156a77a 100644 (file)
@@ -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,
index b6e2652568e6c73699c29fa84d194cbe8b64a824..ed4eed36abc0140a13eb799dd356f40659d3a7b9 100644 (file)
@@ -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);
index ae873a31e6af82d16029454cff90d48dc44adb55..6d212bbdf3b3d433548d56a08b229d196835f31b 100644 (file)
@@ -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
index ac443cd520cd3bbb0808b9d3cb85a836630982e1..750c6a1f9e8e4f8c8534333edb2d8df23c169fd4 100644 (file)
@@ -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.");
index 605ee37b9a1d92fd173ac6a39ca5489a8f2a4bc5..345431241c273bdb53df3d72ef42db70444b40a1 100644 (file)
@@ -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);