Handle argument conversion in StringAddStub.

In case one of the arguments is known to be a string we emit a few
fast conversion attempts for the other.  This allows using the
StringAddStub instead of STRING_ADD_{LEFT,RIGHT} builtins.

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

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

diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index d735419..bfd2650 100644 (file)
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -868,7 +868,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
               FloatingPointHelper::LoadSSE2Operands(masm);
             }
           } else {
-            FloatingPointHelper::LoadSSE2Operands(masm, &call_runtime);
+            FloatingPointHelper::LoadSSE2Operands(masm, &not_floats);
           }
 
           switch (op_) {
@@ -889,7 +889,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
               __ AbortIfNotNumber(eax);
             }
           } else {
-            FloatingPointHelper::CheckFloatOperands(masm, &call_runtime, ebx);
+            FloatingPointHelper::CheckFloatOperands(masm, &not_floats, ebx);
           }
           FloatingPointHelper::LoadFloatOperands(
               masm,
@@ -1001,10 +1001,15 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
     }
   }
 
+  // Avoid hitting the string ADD code below when allocation fails in
+  // the floating point code above.
+  if (op_ != Token::ADD) {
+    __ bind(&call_runtime);
+  }
+
   // If all else fails, use the runtime system to get the correct
   // result. If arguments was passed in registers now place them on the
   // stack in the correct order below the return address.
-  __ bind(&call_runtime);
   if (HasArgsInRegisters()) {
     GenerateRegisterArgsPush(masm);
   }
@@ -1012,7 +1017,6 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
   switch (op_) {
     case Token::ADD: {
       // Test for string arguments before calling runtime.
-      Label not_strings, not_string1, string1, string1_smi2;
 
       // If this stub has already generated FP-specific code then the arguments
       // are already in edx, eax
@@ -1030,49 +1034,31 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
         rhs = eax;
       }
 
-      // Test if first argument is a string.
+      // Test if left operand is a string.
+      Label lhs_not_string;
       __ test(lhs, Immediate(kSmiTagMask));
-      __ j(zero, &not_string1);
+      __ j(zero, &lhs_not_string);
       __ CmpObjectType(lhs, FIRST_NONSTRING_TYPE, ecx);
-      __ j(above_equal, &not_string1);
-
-      // First argument is a string, test second.
-      __ test(rhs, Immediate(kSmiTagMask));
-      __ j(zero, &string1_smi2);
-      __ CmpObjectType(rhs, FIRST_NONSTRING_TYPE, ecx);
-      __ j(above_equal, &string1);
-
-      // First and second argument are strings. Jump to the string add stub.
-      StringAddStub string_add_stub(NO_STRING_CHECK_IN_STUB);
-      __ TailCallStub(&string_add_stub);
+      __ j(above_equal, &lhs_not_string);
 
-      __ bind(&string1_smi2);
-      // First argument is a string, second is a smi. Try to lookup the number
-      // string for the smi in the number string cache.
-      NumberToStringStub::GenerateLookupNumberStringCache(
-          masm, rhs, edi, ebx, ecx, true, &string1);
+      StringAddStub string_add_left_stub(NO_STRING_CHECK_LEFT_IN_STUB);
+      __ TailCallStub(&string_add_left_stub);
 
-      // Replace second argument on stack and tailcall string add stub to make
-      // the result.
-      __ mov(Operand(esp, 1 * kPointerSize), edi);
-      __ TailCallStub(&string_add_stub);
-
-      // Only first argument is a string.
-      __ bind(&string1);
-      __ InvokeBuiltin(Builtins::STRING_ADD_LEFT, JUMP_FUNCTION);
-
-      // First argument was not a string, test second.
-      __ bind(&not_string1);
+      // Left operand is not a string, test right.
+      __ bind(&lhs_not_string);
       __ test(rhs, Immediate(kSmiTagMask));
-      __ j(zero, &not_strings);
+      __ j(zero, &call_runtime);
       __ CmpObjectType(rhs, FIRST_NONSTRING_TYPE, ecx);
-      __ j(above_equal, &not_strings);
+      __ j(above_equal, &call_runtime);
 
-      // Only second argument is a string.
-      __ InvokeBuiltin(Builtins::STRING_ADD_RIGHT, JUMP_FUNCTION);
+      StringAddStub string_add_right_stub(NO_STRING_CHECK_RIGHT_IN_STUB);
+      __ TailCallStub(&string_add_right_stub);
 
-      __ bind(&not_strings);
       // Neither argument is a string.
+      __ bind(&call_runtime);
+      if (HasArgsInRegisters()) {
+        GenerateRegisterArgsPush(masm);
+      }
       __ InvokeBuiltin(Builtins::ADD, JUMP_FUNCTION);
       break;
     }
@@ -3765,14 +3751,15 @@ void StringCharAtGenerator::GenerateSlow(
 
 
 void StringAddStub::Generate(MacroAssembler* masm) {
-  Label string_add_runtime;
+  Label string_add_runtime, call_builtin;
+  Builtins::JavaScript builtin_id = Builtins::ADD;
 
   // Load the two arguments.
   __ mov(eax, Operand(esp, 2 * kPointerSize));  // First argument.
   __ mov(edx, Operand(esp, 1 * kPointerSize));  // Second argument.
 
   // Make sure that both arguments are strings if not known in advance.
-  if (string_check_) {
+  if (flags_ == NO_STRING_ADD_FLAGS) {
     __ test(eax, Immediate(kSmiTagMask));
     __ j(zero, &string_add_runtime);
     __ CmpObjectType(eax, FIRST_NONSTRING_TYPE, ebx);
@@ -3783,6 +3770,20 @@ void StringAddStub::Generate(MacroAssembler* masm) {
     __ j(zero, &string_add_runtime);
     __ CmpObjectType(edx, FIRST_NONSTRING_TYPE, ebx);
     __ j(above_equal, &string_add_runtime);
+  } else {
+    // Here at least one of the arguments is definitely a string.
+    // We convert the one that is not known to be a string.
+    if ((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) == 0) {
+      ASSERT((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) != 0);
+      GenerateConvertArgument(masm, 2 * kPointerSize, eax, ebx, ecx, edi,
+                              &call_builtin);
+      builtin_id = Builtins::STRING_ADD_RIGHT;
+    } else if ((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) == 0) {
+      ASSERT((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) != 0);
+      GenerateConvertArgument(masm, 1 * kPointerSize, edx, ebx, ecx, edi,
+                              &call_builtin);
+      builtin_id = Builtins::STRING_ADD_LEFT;
+    }
   }
 
   // Both arguments are strings.
@@ -4016,6 +4017,56 @@ void StringAddStub::Generate(MacroAssembler* masm) {
   // Just jump to runtime to add the two strings.
   __ bind(&string_add_runtime);
   __ TailCallRuntime(Runtime::kStringAdd, 2, 1);
+
+  if (call_builtin.is_linked()) {
+    __ bind(&call_builtin);
+    __ InvokeBuiltin(builtin_id, JUMP_FUNCTION);
+  }
+}
+
+
+void StringAddStub::GenerateConvertArgument(MacroAssembler* masm,
+                                            int stack_offset,
+                                            Register arg,
+                                            Register scratch1,
+                                            Register scratch2,
+                                            Register scratch3,
+                                            Label* slow) {
+  // First check if the argument is already a string.
+  Label not_string, done;
+  __ test(arg, Immediate(kSmiTagMask));
+  __ j(zero, &not_string);
+  __ CmpObjectType(arg, FIRST_NONSTRING_TYPE, scratch1);
+  __ j(below, &done);
+
+  // Check the number to string cache.
+  Label not_cached;
+  __ bind(&not_string);
+  // Puts the cached result into scratch1.
+  NumberToStringStub::GenerateLookupNumberStringCache(masm,
+                                                      arg,
+                                                      scratch1,
+                                                      scratch2,
+                                                      scratch3,
+                                                      false,
+                                                      &not_cached);
+  __ mov(arg, scratch1);
+  __ mov(Operand(esp, stack_offset), arg);
+  __ jmp(&done);
+
+  // Check if the argument is a safe string wrapper.
+  __ bind(&not_cached);
+  __ test(arg, Immediate(kSmiTagMask));
+  __ j(zero, slow);
+  __ CmpObjectType(arg, JS_VALUE_TYPE, scratch1);  // map -> scratch1.
+  __ j(not_equal, slow);
+  __ test_b(FieldOperand(scratch1, Map::kBitField2Offset),
+            1 << Map::kStringWrapperSafeForDefaultValueOf);
+  __ j(zero, slow);
+  __ mov(arg, FieldOperand(arg, JSValue::kValueOffset));
+  __ mov(Operand(esp, stack_offset), arg);
+
+  __ bind(&done);
 }
 
 

diff --git a/src/ia32/code-stubs-ia32.h b/src/ia32/code-stubs-ia32.h
index 4eac9a9..351636f 100644 (file)
--- a/src/ia32/code-stubs-ia32.h
+++ b/src/ia32/code-stubs-ia32.h
@@ -272,24 +272,35 @@ class StringHelper : public AllStatic {
 // Flag that indicates how to generate code for the stub StringAddStub.
 enum StringAddFlags {
   NO_STRING_ADD_FLAGS = 0,
-  NO_STRING_CHECK_IN_STUB = 1 << 0  // Omit string check in stub.
+  // Omit left string check in stub (left is definitely a string).
+  NO_STRING_CHECK_LEFT_IN_STUB = 1 << 0,
+  // Omit right string check in stub (right is definitely a string).
+  NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 1,
+  // Omit both string checks in stub.
+  NO_STRING_CHECK_IN_STUB =
+      NO_STRING_CHECK_LEFT_IN_STUB | NO_STRING_CHECK_RIGHT_IN_STUB
 };
 
 
 class StringAddStub: public CodeStub {
  public:
-  explicit StringAddStub(StringAddFlags flags) {
-    string_check_ = ((flags & NO_STRING_CHECK_IN_STUB) == 0);
-  }
+  explicit StringAddStub(StringAddFlags flags) : flags_(flags) {}
 
  private:
   Major MajorKey() { return StringAdd; }
-  int MinorKey() { return string_check_ ? 0 : 1; }
+  int MinorKey() { return flags_; }
 
   void Generate(MacroAssembler* masm);
 
-  // Should the stub check whether arguments are strings?
-  bool string_check_;
+  void GenerateConvertArgument(MacroAssembler* masm,
+                               int stack_offset,
+                               Register arg,
+                               Register scratch1,
+                               Register scratch2,
+                               Register scratch3,
+                               Label* slow);
+
+  const StringAddFlags flags_;
 };
 
 

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index d399c35..854052a 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -1411,12 +1411,12 @@ void CodeGenerator::GenericBinaryOperation(BinaryOperation* expr,
           StringAddStub stub(NO_STRING_CHECK_IN_STUB);
           answer = frame_->CallStub(&stub, 2);
         } else {
-          answer =
-            frame_->InvokeBuiltin(Builtins::STRING_ADD_LEFT, CALL_FUNCTION, 2);
+          StringAddStub stub(NO_STRING_CHECK_LEFT_IN_STUB);
+          answer = frame_->CallStub(&stub, 2);
         }
       } else if (right_is_string) {
-        answer =
-          frame_->InvokeBuiltin(Builtins::STRING_ADD_RIGHT, CALL_FUNCTION, 2);
+        StringAddStub stub(NO_STRING_CHECK_RIGHT_IN_STUB);
+        answer = frame_->CallStub(&stub, 2);
       }
       answer.set_type_info(TypeInfo::String());
       frame_->Push(&answer);

diff --git a/test/mjsunit/binary-op-newspace.js b/test/mjsunit/binary-op-newspace.js
index 8034209..40d53b9 100644 (file)
--- a/test/mjsunit/binary-op-newspace.js
+++ b/test/mjsunit/binary-op-newspace.js
@@ -25,21 +25,38 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-/**
- * @fileoverview Check that a mod where the stub code hits a failure
- * in heap number allocation still works.
- */
-
 // Flags: --max-new-space-size=262144
 
+
+// Check that a mod where the stub code hits a failure in heap number
+// allocation still works.
+
 function f(x) {
   return x % 3;
 }
 
-function test() {
+function testMod() {
   for (var i = 0; i < 40000; i++) {
     assertEquals(-1 / 0, 1 / f(-3));
   }
 }
 
-test();
+testMod();
+
+
+// Check that an add where the stub code hits a failure in heap number
+// allocation still works.
+
+function g(x, y) {
+  return x + y;
+}
+
+function testAdd() {
+  var lhs = 17.42;
+  var rhs = 42.17;
+  for (var i = 0; i < 40000; i++) {
+    assertEquals(59.59, g(lhs, rhs));
+  }
+}
+
+testAdd();